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

 
 

You are not allowed to post comments on this tracker with your current authentication level.

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

Submitter:  None
Submitted:  Sun 15 Jan 2017 05:53:12 PM UTC
   
 
Severity:  3 - Normal Status:  Fixed
Assigned to:  bens Open/Closed:  Closed

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>
Group administrator
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>
Group administrator
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>
Group administrator
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>
Group administrator
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>
Group administrator
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>
Group administrator
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.)

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
  • -email is unavailable- added by bens (Posted a comment)
  • -email is unavailable- added by aliverix (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-02-24 bens Open/ClosedOpen Closed
    2017-01-20 bens StatusReady For Test Fixed
        SummaryCrash with empty begin and end regex for &quot;color&quot; in .nanorc crash with empty start and end regex for "color" in .nanorc
    2017-01-18 bens Attached File- Added 0001-painting-stay-within-the-line-when-skipping-zero-len.patch, #39501
        StatusIn Progress Ready For Test
    2017-01-17 bens Attached File- Added crude.patch, #39499
    2017-01-17 aliverix Attached File- Added nanorc_crash_new, #39497
    2017-01-16 bens Attached File- Added 0001-rcfile-don-t-accept-empty-regexes-for-syntax-colorin.patch, #39490
        StatusNone In Progress
    2017-01-15 bens Assigned toNone bens
    2017-01-15 None Attached File- Added nanorc_crash, #39485

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code