PerlGameDev / SDL

Rehashing the old perl SDL binding on cpan.org
http://search.cpan.org/dist/SDL
GNU General Public License v2.0
81 stars 29 forks source link

out-of-bounds read in t/image_xpm_array.t #307

Open smcv opened 1 year ago

smcv commented 1 year ago

While testing #305 I found an unrelated memory error in t/image_xpm_array.t. If I'm reading correctly, it's continuing to copy bytes beyond the \0 termination of each string in the array. This is undefined behaviour and can lead to crashes, although in practice it doesn't seem to crash on any of Debian's autobuilders.

t/image_xpm_array.t ............. =================================================================
==523369==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200014495b at pc 0x7f762086de02 bp 0x7ffe69d5b790 sp 0x7ffe69d5af50
READ of size 30 at 0x60200014495b thread T0
    #0 0x7f762086de01 in __interceptor_memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:899
    #1 0x7f761f98f5c7 in memcpy /usr/include/x86_64-linux-gnu/bits/string_fortified.h:29
    #2 0x7f761f98f5c7 in XS_SDL__Image_read_XPM_from_array lib/SDL/Image.xs:381
    #3 0x55a593831f17 in Perl_pp_entersub (/usr/bin/perl+0x123f17) (BuildId: 42daa0cc03328cecf85c1f8589ec1619f547c3a5)
    #4 0x55a593827ef5 in Perl_runops_standard (/usr/bin/perl+0x119ef5) (BuildId: 42daa0cc03328cecf85c1f8589ec1619f547c3a5)
    #5 0x55a593786778 in perl_run (/usr/bin/perl+0x78778) (BuildId: 42daa0cc03328cecf85c1f8589ec1619f547c3a5)
    #6 0x55a5937584b1 in main (/usr/bin/perl+0x4a4b1) (BuildId: 42daa0cc03328cecf85c1f8589ec1619f547c3a5)
    #7 0x7f76205666c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #8 0x7f7620566784 in __libc_start_main_impl ../csu/libc-start.c:360
    #9 0x55a5937584f0 in _start (/usr/bin/perl+0x4a4f0) (BuildId: 42daa0cc03328cecf85c1f8589ec1619f547c3a5)

0x60200014495b is located 0 bytes after 11-byte region [0x602000144950,0x60200014495b)
allocated by thread T0 here:
    #0 0x7f76208d85bf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x55a593804f55 in Perl_safesysmalloc (/usr/bin/perl+0xf6f55) (BuildId: 42daa0cc03328cecf85c1f8589ec1619f547c3a5)

SUMMARY: AddressSanitizer: heap-buffer-overflow ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:899 in __interceptor_memcpy
Shadow bytes around the buggy address:
  0x602000144680: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x602000144700: fa fa fd fd fa fa fd fd fa fa fd fa fa fa 00 02
  0x602000144780: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x602000144800: fa fa fd fd fa fa 00 02 fa fa fd fd fa fa fd fd
  0x602000144880: fa fa fd fd fa fa 00 03 fa fa fd fd fa fa 00 02
=>0x602000144900: fa fa fd fd fa fa fd fd fa fa 00[03]fa fa fd fd
  0x602000144980: fa fa 00 05 fa fa fd fd fa fa 00 05 fa fa fd fd
  0x602000144a00: fa fa 00 05 fa fa fd fd fa fa 00 05 fa fa fd fd
  0x602000144a80: fa fa 00 05 fa fa fd fd fa fa 00 05 fa fa fd fd
  0x602000144b00: fa fa 00 05 fa fa fd fd fa fa 00 05 fa fa fd fd
  0x602000144b80: fa fa 00 05 fa fa fd fd fa fa fd fd fa fa fd fd
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
==523369==ABORTING
Dubious, test returned 1 (wstat 256, 0x100)
No subtests run 
smcv commented 1 year ago

The solution would probably involve using strncpy instead of memcpy to copy into each item src_x[x], and then setting the last byte src[x][w - 1] to \0 (because strncpy can leave a string unterminated, if truncation occurs).