bugGNU nano - Bugs: bug #50056, crash with empty start and end...

 
 

bug #50056: crash with empty start and end regex for "color" in .nanorc

Submitted by:  None
Submitted on:  Sun 15 Jan 2017 05:53:12 PM UTC  
 
Severity: 3 - NormalStatus: Fixed
Assigned to: Benno Schulenberg <bens>Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Fri 24 Feb 2017 10:21:24 AM UTC, comment #10:

Released in 2.7.5. Thanks again for reporting.

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Fri 20 Jan 2017 04:41:11 PM UTC, comment #9:

Fixed in git, 9de376de.

Thanks for reporting. I didn't wait for your confirmation because by now I am pretty confident that this is a good fix. (It is not entirely complete yet, because it shouldn't simply advance a byte with index++ but advance a character with move_mbright(). But that will follow later.)

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Wed 18 Jan 2017 06:59:12 PM UTC, comment #8:

Thanks for testing. Here is a slightly different patch. If you could test this one too...

And if you would like to see anything in the patch itself or its commit message changed, let me know.

(file #39501)

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Wed 18 Jan 2017 12:24:31 AM UTC, comment #7:

I fuzzed for three hours with the new patch and everything looks fine. (The other crashes were generated after 10 minutes or so)

Elia Geretto <aliverix>
Tue 17 Jan 2017 08:52:22 PM UTC, comment #6:

Thanks. Problem is reproducible. But trying to run valgrind on it just hangs. Checking with older versions, it appears that the segfault happens since nano-2.5.1 -- before that, nano just hung with a blank screen.

It is clear that it has something to do with an empty (sub)expression, but I'm a bit surprised that regcomp doesn't qualify such a thing as invalid.

After killing valgrind, there is some useful information, intermixed with the debug info, though:

-----
Entering precalculation of multiline color info
Starting work on color id 0
working on lineno 1... start found at pos 0... end found on this line
==11554== Invalid read of size 1
==11554== at 0x4026058: __GI_strlen (mc_replace_strmem.c:284)
==11554== by 0x4142073: regexec@@GLIBC_2.3.4 (regexec.c:245)
==11554== by 0x804E8E3: precalc_multicolorinfo (color.c:425)
==11554== by 0x8050200: display_buffer (files.c:567)
==11554== by 0x8060220: main (nano.c:2644)
==11554== Address 0x4819de1 is 0 bytes after a block of size 1 alloc'd
==11554== at 0x4024F20: malloc (vg_replace_malloc.c:236)
==11554== by 0x806DF1D: nmalloc (utils.c:382)
==11554== by 0x806DFB6: mallocstrncpy (utils.c:412)
==11554== by 0x806E00E: mallocstrcpy (utils.c:422)
==11554== by 0x804F301: initialize_buffer_text (files.c:126)
==11554== by 0x804F1DC: make_new_buffer (files.c:92)
==11554== by 0x804FF9C: open_buffer (files.c:468)
==11554== by 0x8060124: main (nano.c:2619)
==11554==
start found at pos 1... ==11554== Invalid read of size 1
==11554== at 0x4026058: __GI_strlen (mc_replace_strmem.c:284)
==11554== by 0x4142073: regexec@@GLIBC_2.3.4 (regexec.c:245)
==11554== by 0x804E676: precalc_multicolorinfo (color.c:434)
==11554== by 0x8050200: display_buffer (files.c:567)
==11554== by 0x8060220: main (nano.c:2644)
==11554== Address 0x4819de1 is 0 bytes after a block of size 1 alloc'd
==11554== at 0x4024F20: malloc (vg_replace_malloc.c:236)
==11554== by 0x806DF1D: nmalloc (utils.c:382)
==11554== by 0x806DFB6: mallocstrncpy (utils.c:412)
==11554== by 0x806E00E: mallocstrcpy (utils.c:422)
==11554== by 0x804F301: initialize_buffer_text (files.c:126)
==11554== by 0x804F1DC: make_new_buffer (files.c:92)
==11554== by 0x804FF9C: open_buffer (files.c:468)
==11554== by 0x8060124: main (nano.c:2619)
==11554==
end found on this line
start found at pos 2... end found on this line
start found at pos 3... end found on this line
start found at pos 4... end found on this line
start found at pos 5... end found on this line
-----

And then it just goes on and on and on. But I've started nano with an empty file! There are zero positions on that line.

Something goes wrong in the precalculation... Ehm, no, also in the actual painting. Pfff...! It never checks whether the index is maybe beyond end-of-line!

Attached is a crude patch that avoids the crashes now for me. If you can, please run further tests.

(file #39499)

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Tue 17 Jan 2017 05:40:05 PM UTC, comment #5:

From what I see in your patch, you are checking whether the regular expressions for start and end are empty. This is actually not sufficient because it is possible to obtain the same effect, which is what the fuzzer did, with other expressions, as for example "|" or "|abc", or even "a*". I am uploading another file that makes the program crash at the same point.

(file #39497)

Elia Geretto <aliverix>
Mon 16 Jan 2017 03:54:41 PM UTC, comment #4:

Here is a patch that should avoid the problem. If you can run that through AFL for a while, that would be great.

Yes, if you can, please upload your project stuff.

(file #39490)

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Sun 15 Jan 2017 08:03:01 PM UTC, comment #3:

I ran it just for a couple of hours, I actually found this bug as part of a university assignment. Regarding the set up part, it is quite easy if you just want to fake user input, given that it's taken from stdin. It is a little more difficult if you want to fuzz the configuration files because there is no easy way (that I know of) to make nano parse an arbitrary file. I had to make some changes in the code, a dirty fix that adds a command line argument that allows to pass an arbitrary configuration file.

It cannot be integrated in the code like this (I used a git branch on purpose for that), but I will upload my project here if you want to check it out. (There is a README inside)

Elia Geretto <aliverix>
Sun 15 Jan 2017 06:54:14 PM UTC, comment #2:

Wow. Thanks for the report. I've already looked how this can be avoided, but don't have the time right now to make a patch.

Have you been running AFL for long on nano? Is it easy enough to set up? So that I could install it and run it too now and then?

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Sun 15 Jan 2017 05:57:30 PM UTC, comment #1:

I didn't realize I was posting as "Anonymous", I am willing to provide additional details if necessary.

Elia Geretto <aliverix>
Sun 15 Jan 2017 05:53:12 PM UTC, original submission:

A segmentation fault occurs when the following rule is inserted in a syntax specification inside a configuration file:

color green start="" end=""

In order to reproduce the bug, it is sufficient to substitute the ~/.nanorc file with the one attached to this bug report and then start nano with:

$ nano -q

(Bug found with AFL)

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #39499:  crude.patch added by bens (2KiB - text/x-diff)
file #39497:  nanorc_crash_new added by aliverix (55B - application/octet-stream)
file #39485:  nanorc_crash added by None (49B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by bens (Posted a comment)
  • -unavailable- added by aliverix (Posted a comment)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 11 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 24 Feb 2017 10:21:24 AM UTCbensOpen/ClosedOpen=>Closed
    Fri 20 Jan 2017 04:41:11 PM UTCbensStatusReady For Test=>Fixed
      SummaryCrash with empty begin and end regex for "color" in .nanorc=>crash with empty start and end regex for "color" in .nanorc
    Wed 18 Jan 2017 06:59:12 PM UTCbensAttached File-=>Added 0001-painting-stay-within-the-line-when-skipping-zero-len.patch, #39501
      StatusIn Progress=>Ready For Test
    Tue 17 Jan 2017 08:52:22 PM UTCbensAttached File-=>Added crude.patch, #39499
    Tue 17 Jan 2017 05:40:05 PM UTCaliverixAttached File-=>Added nanorc_crash_new, #39497
    Mon 16 Jan 2017 03:54:41 PM UTCbensAttached File-=>Added 0001-rcfile-don-t-accept-empty-regexes-for-syntax-colorin.patch, #39490
      StatusNone=>In Progress
    Sun 15 Jan 2017 06:54:14 PM UTCbensAssigned toNone=>bens
    Sun 15 Jan 2017 05:53:12 PM UTCNoneAttached File-=>Added nanorc_crash, #39485

    Back to the top


    Powered by Savane 3.1-cleanup1