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 seekback() function #35

Closed
fcambus opened this issue Dec 6, 2019 · 4 comments
Closed

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

fcambus opened this issue Dec 6, 2019 · 4 comments

Comments

@fcambus
Copy link

fcambus commented Dec 6, 2019

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
Copy link
Owner

marcIhm commented Dec 8, 2019 via email

@fcambus
Copy link
Author

fcambus commented Dec 8, 2019

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

*(yylval.string+yyleng-2)='\0';

@marcIhm
Copy link
Owner

marcIhm commented Dec 9, 2019

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
Copy link
Author

fcambus commented Dec 10, 2019

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

@fcambus fcambus closed this as completed Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants