marcIhm / yabasic

Yabasic - A simple Basic interpreter for Unix and Windows
http://www.yabasic.de
MIT License
90 stars 17 forks source link

Heap-based buffer overflow in the seekback() function #35

Closed fcambus closed 4 years ago

fcambus commented 4 years ago

Hi,

When building and running yabasic 2.86.0 with Clang and AddressSanitizer enabled, I found a heap-based buffer overflow in the seekback() function, in main.c.

The issue can be triggered as follow:

./configure CC=clang CFLAGS="-fsanitize=address -g"
make
./yabasic
=================================================================
==22032==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6260000000ff at pc 0x0000004dafef bp 0x7ffffda34780 sp 0x7ffffda34778
WRITE of size 1 at 0x6260000000ff thread T0
    #0 0x4dafee in seekback /home/fcambus/yabasic-2.86.0/unix/lang/main.c:2454:41
    #1 0x4c696e in isbound /home/fcambus/yabasic-2.86.0/unix/lang/main.c:2354:10
    #2 0x4c52c7 in main /home/fcambus/yabasic-2.86.0/unix/lang/main.c:203:16
    #3 0x7f009e87f1e2 in __libc_start_main /build/glibc-4WA41p/glibc-2.30/csu/../csu/libc-start.c:308:16
    #4 0x41da2d in _start (/home/fcambus/yabasic-2.86.0/unix/lang/yabasic+0x41da2d)

0x6260000000ff is located 1 bytes to the left of 10004-byte region [0x626000000100,0x626000002814)
allocated by thread T0 here:
    #0 0x49592d in malloc (/home/fcambus/yabasic-2.86.0/unix/lang/yabasic+0x49592d)
    #1 0x4c6314 in my_malloc /home/fcambus/yabasic-2.86.0/unix/lang/main.c:2099:12
    #2 0x4c5012 in main /home/fcambus/yabasic-2.86.0/unix/lang/main.c:122:23
    #3 0x7f009e87f1e2 in __libc_start_main /build/glibc-4WA41p/glibc-2.30/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/fcambus/yabasic-2.86.0/unix/lang/main.c:2454:41 in seekback
Shadow bytes around the buggy address:
  0x0c4c7fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4c7fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4c7fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4c7fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4c7fff8000: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c4c7fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]
  0x0c4c7fff8020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4c7fff8030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4c7fff8040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4c7fff8050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4c7fff8060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
==22032==ABORTING
marcIhm commented 4 years ago

Hi Frederic, this is fixed in Version 2.86.1. Thanx for reporting ! regards Marc On 06.12.2019 16:42:54, Frederic Cambus notifications@github.com wrote: Hi, When building and running yabasic 2.86.0 with Clang and AddressSanitizer enabled, I found a heap-based buffer overflow in the seekback() function, in main.c. The issue can be triggered as follow: ./configure CC=clang CFLAGS="-fsanitize=address -g" make ./yabasic ================================================================= ==22032==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6260000000ff at pc 0x0000004dafef bp 0x7ffffda34780 sp 0x7ffffda34778 WRITE of size 1 at 0x6260000000ff thread T0 #0 0x4dafee in seekback /home/fcambus/yabasic-2.86.0/unix/lang/main.c:2454:41 #1 0x4c696e in isbound /home/fcambus/yabasic-2.86.0/unix/lang/main.c:2354:10 #2 0x4c52c7 in main /home/fcambus/yabasic-2.86.0/unix/lang/main.c:203:16 #3 0x7f009e87f1e2 in libc_start_main /build/glibc-4WA41p/glibc-2.30/csu/../csu/libc-start.c:308:16 #4 0x41da2d in _start (/home/fcambus/yabasic-2.86.0/unix/lang/yabasic+0x41da2d) 0x6260000000ff is located 1 bytes to the left of 10004-byte region [0x626000000100,0x626000002814) allocated by thread T0 here: #0 0x49592d in malloc (/home/fcambus/yabasic-2.86.0/unix/lang/yabasic+0x49592d) #1 0x4c6314 in my_malloc /home/fcambus/yabasic-2.86.0/unix/lang/main.c:2099:12 #2 0x4c5012 in main /home/fcambus/yabasic-2.86.0/unix/lang/main.c:122:23 #3 0x7f009e87f1e2 in libc_start_main /build/glibc-4WA41p/glibc-2.30/csu/../csu/libc-start.c:308:16 SUMMARY: AddressSanitizer: heap-buffer-overflow /home/fcambus/yabasic-2.86.0/unix/lang/main.c:2454:41 in seekback Shadow bytes around the buggy address: 0x0c4c7fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c4c7fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c4c7fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c4c7fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c4c7fff8000: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa =>0x0c4c7fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa] 0x0c4c7fff8020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c4c7fff8030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c4c7fff8040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c4c7fff8050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c4c7fff8060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 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 ==22032==ABORTING — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub [https://github.com/marcIhm/yabasic/issues/35?email_source=notifications&email_token=AC5EZHQTJ3I75DQ6M3Z3Y73QXJXHZA5CNFSM4JW2ZQ2KYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4H6VQORA], or unsubscribe [https://github.com/notifications/unsubscribe-auth/AC5EZHTNSIG27Q7LP3QXJMLQXJXHZANCNFSM4JW2ZQ2A].

fcambus commented 4 years ago

Thanks for the fix. It seems a similar issue also exists in flex.c L3100:

*(yylval.string+yyleng-2)='\0';
marcIhm commented 4 years ago

Hi, well I do not think so; having a look at the full flex-rule: ` \"[^\"](\"|\n) { int cnt; if (yytext[yyleng-1]=='\n') yycolumn=1; if (yytext[yyleng-1]=='\n' && in_short_if) {in_short_if--;yyless(0);return tIMPLICITENDIF;} if (yytext[yyleng-1]=='\n') { yylval.string=NULL; return tSTRING; } for(cnt=0;yytext[yyleng-cnt-2]=='\';cnt++) ; if (cnt%2) { yyless(yyleng-1); yymore(); } else { yylval.string=(char )my_strdup(yytext+1); *(yylval.string+yyleng-2)='\0'; replace(yylval.string); return tSTRING; } }

` shows that yyleng is 2 at minimum. So I think this statement is safe, although it looks a bit fishy. regards Marc So unless you have written testimony from valgrind or others, I am inclined to close this :-)

fcambus commented 4 years ago

Please see issue #36 for details and for a reproducer.