Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heap-based buffer overflow in the canonpath() function #29

Closed
fcambus opened this issue Dec 13, 2019 · 5 comments
Closed

Heap-based buffer overflow in the canonpath() function #29

fcambus opened this issue Dec 13, 2019 · 5 comments

Comments

@fcambus
Copy link

@fcambus fcambus commented Dec 13, 2019

Hi,

While fuzzing samurai 0.7 with American Fuzzy Lop, I found a heap-based buffer overflow in the canonpath() function, in util.c.

Attaching a reproducer (gzipped so GitHub accepts it): test01.gz

Issue can be reproduced by running:

samu -f test01
=================================================================
==17675==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000001719 at pc 0x0000004dd5a3 bp 0x7fff02eaec90 sp 0x7fff02eaec88
WRITE of size 1 at 0x602000001719 thread T0
    #0 0x4dd5a2 in canonpath /home/fcambus/samurai-0.7/util.c:204:6
    #1 0x4d3ed3 in parseedge /home/fcambus/samurai-0.7/parse.c:131:3
    #2 0x4d28a2 in parse /home/fcambus/samurai-0.7/parse.c:231:4
    #3 0x4d6252 in main /home/fcambus/samurai-0.7/samu.c:221:2
    #4 0x7efd0e3321e2 in __libc_start_main /build/glibc-4WA41p/glibc-2.30/csu/../csu/libc-start.c:308:16
    #5 0x41b45d in _start (/home/fcambus/samurai-0.7/samu+0x41b45d)

0x602000001719 is located 0 bytes to the right of 9-byte region [0x602000001710,0x602000001719)
allocated by thread T0 here:
    #0 0x49335d in malloc (/home/fcambus/samurai-0.7/samu+0x49335d)
    #1 0x4dbf34 in xmalloc /home/fcambus/samurai-0.7/util.c:55:6
    #2 0x4dc803 in mkstr /home/fcambus/samurai-0.7/util.c:133:8
    #3 0x4c857b in merge /home/fcambus/samurai-0.7/env.c:84:11
    #4 0x4c8538 in enveval /home/fcambus/samurai-0.7/env.c:115:8
    #5 0x4d3ec0 in parseedge /home/fcambus/samurai-0.7/parse.c:130:9
    #6 0x4d28a2 in parse /home/fcambus/samurai-0.7/parse.c:231:4
    #7 0x4d6252 in main /home/fcambus/samurai-0.7/samu.c:221:2
    #8 0x7efd0e3321e2 in __libc_start_main /build/glibc-4WA41p/glibc-2.30/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/fcambus/samurai-0.7/util.c:204:6 in canonpath
Shadow bytes around the buggy address:
  0x0c047fff8290: fa fa fd fa fa fa fd fd fa fa 03 fa fa fa 00 00
  0x0c047fff82a0: fa fa 00 fa fa fa fd fd fa fa fd fd fa fa 00 05
  0x0c047fff82b0: fa fa 00 fa fa fa 00 05 fa fa fd fd fa fa fd fd
  0x0c047fff82c0: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c047fff82d0: fa fa fd fd fa fa fd fd fa fa 00 03 fa fa 00 03
=>0x0c047fff82e0: fa fa 00[01]fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff82f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8310: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8320: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8330: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==17675==ABORTING
@michaelforney

This comment has been minimized.

Copy link
Owner

@michaelforney michaelforney commented Dec 13, 2019

Thanks for fuzzing samurai!

I believe the buffer overflow is due to failing to reject empty paths, which it tries to canonicalize the same way as foo/.. -> .. For an empty string, the output (.), is longer than the input (``), and this operation done in-place. I need to decide whether to make canonpath able to return failure, or have it print an error and exit itself, or just have the parser fail on an empty string.

The test case also revealed another issue, which is infinite recursion caused by a recursive rule variable definition. This still needs more investigation.

michaelforney added a commit that referenced this issue Dec 13, 2019
Reported by Frederic Cambus in #29.
michaelforney added a commit that referenced this issue Dec 13, 2019
Reported by Frederic Cambus in #29.
@fcambus

This comment has been minimized.

Copy link
Author

@fcambus fcambus commented Dec 13, 2019

This issue has been assigned CVE-2019-19795.

@michaelforney

This comment has been minimized.

Copy link
Owner

@michaelforney michaelforney commented Dec 13, 2019

Both issues are now fixed. I'm planning to make a new release shortly, so please let me know if you find any other issues.

@michaelforney

This comment has been minimized.

Copy link
Owner

@michaelforney michaelforney commented Dec 13, 2019

Also, note that the entire point of the tool is to run arbitrary commands from the build file, so it should be considered a trusted input.

@fcambus

This comment has been minimized.

Copy link
Author

@fcambus fcambus commented Dec 14, 2019

Thanks for your fixes! I've let fuzzers run overnight on the latest master branch and they did not find anything, good job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.