GNU bug report logs - #53333
Fix for crash in ebrowse

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 53333 in the body.
You can then email your comments to 53333 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jan Stranik <jan <at> stranik.org>
To: bug-gnu-emacs <at> gnu.org
Subject: Fix for crash in ebrowse
Date: Mon, 17 Jan 2022 17:35:36 -0500

[Message part 1 (text/plain, inline)]

Hello --

attached is a patch to ebrowse. Noticed a one-off write error in case of
identifiers that are too long and need escaping. The patch prevents the
write to memory outside of allocated range which on my platform caused
segfault.

Best,

[ebrowse-emacs-27.2-fix.diff (text/x-diff, attachment)]

[Message part 3 (text/plain, inline)]

-- 

Jan Stranik

Message #8 received at 53333 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jan Stranik <jan <at> stranik.org>
Cc: 53333 <at> debbugs.gnu.org
Subject: Re: bug#53333: Fix for crash in ebrowse
Date: Tue, 18 Jan 2022 20:09:18 +0200

> Date: Mon, 17 Jan 2022 17:35:36 -0500
> From:  Jan Stranik via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> attached is a patch to ebrowse. Noticed a one-off write error in case of
> identifiers that are too long and need escaping. The patch prevents the
> write to memory outside of allocated range which on my platform caused
> segfault.

Thanks, but can you explain the need for this part:

> !           else {
> !               s++;
> !               break;
> !           }
> !       }

Why do we need to advance the pointer 's' in the 'else' clause? why
not leave it alone?

Or maybe I will understand the reason if you show some simple code
that hits this problem (which would be a good thing of its own, as
we'd then have a test to add to our test suite)?

Message #11 received at 53333 <at> debbugs.gnu.org (full text, mbox):

From: Jan Stranik <jan <at> stranik.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 53333 <at> debbugs.gnu.org
Subject: Re: bug#53333: Fix for crash in ebrowse
Date: Tue, 18 Jan 2022 20:32:55 -0500

>
> Thanks, but can you explain the need for this part:
>
>> !           else {
>> !               s++;
>> !               break;
>> !           }
>> !       }
>
> Why do we need to advance the pointer 's' in the 'else' clause? why
> not leave it alone?

The identifier is copied from end to the buffer. As we are copying, we
want to escape quote and backslash characters. Normally if we encounter
any of these characters we just prepend \ to in front. If there is not
enough space in the buffer to insert the \, we should increase the s, to
back-out the character that we wanted to escape.

If we would not do that, the first character might not be escaped. If
that character were a quote, it would break the lisp expressions written
later to the BROWSE file.

> Or maybe I will understand the reason if you show some simple code
> that hits this problem (which would be a good thing of its own, as
> we'd then have a test to add to our test suite)?

I encountered this in a c++ header file with very long identifiers that
just filled the buffer but the first character had to be escaped.

-- 

Jan Stranik

Message #16 received at 53333-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jan Stranik <jan <at> stranik.org>
Cc: 53333-done <at> debbugs.gnu.org
Subject: Re: bug#53333: Fix for crash in ebrowse
Date: Thu, 20 Jan 2022 13:45:31 +0200

> From: Jan Stranik <jan <at> stranik.org>
> Cc: 53333 <at> debbugs.gnu.org
> Date: Tue, 18 Jan 2022 20:32:55 -0500
> 
> >
> > Thanks, but can you explain the need for this part:
> >
> >> !           else {
> >> !               s++;
> >> !               break;
> >> !           }
> >> !       }
> >
> > Why do we need to advance the pointer 's' in the 'else' clause? why
> > not leave it alone?
> 
> The identifier is copied from end to the buffer. As we are copying, we
> want to escape quote and backslash characters. Normally if we encounter
> any of these characters we just prepend \ to in front. If there is not
> enough space in the buffer to insert the \, we should increase the s, to
> back-out the character that we wanted to escape.
> 
> If we would not do that, the first character might not be escaped. If
> that character were a quote, it would break the lisp expressions written
> later to the BROWSE file.

Thanks, I installed the change on the emacs-28 branch, and I'm marking
this bug done.

Message #19 received at 53333-done <at> debbugs.gnu.org (full text, mbox):

From: Jan Stranik <jan <at> stranik.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 53333-done <at> debbugs.gnu.org
Subject: Re: bug#53333: Fix for crash in ebrowse
Date: Thu, 27 Jan 2022 16:20:37 -0500

>
> Thanks, I installed the change on the emacs-28 branch, and I'm marking
> this bug done.

Thank you for pushing the change.

--

Jan Stranik

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.