SCUMM: INDY3: heap buffer-overflow in Scumm::MacGuiImpl::readPascalString()

[dwatteau]

Current Git HEAD, built with --enable-asan, and starting the Macintosh release of Indy3 gives this:

User picked target 'indy3-ega-mac' (engine ID 'scumm', game ID 'indy3')...
=================================================================
==8371==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61f0000bea06 at pc 0x000109af2224 bp 0x7ff7b6775ac0 sp 0x7ff7b6775ab8
READ of size 1 at 0x61f0000bea06 thread T0
    #0 0x109af2223 in Scumm::MacGuiImpl::readPascalString(unsigned char*&) macgui_impl.cpp:83
    #1 0x109b123b4 in Scumm::MacIndy3Gui::readStrings() macgui_indy3.cpp:1032
    #2 0x109b0ff4e in Scumm::MacIndy3Gui::MacIndy3Gui(Scumm::ScummEngine*, Common::Path const&) macgui_indy3.cpp:931
    #3 0x109b10c34 in Scumm::MacIndy3Gui::MacIndy3Gui(Scumm::ScummEngine*, Common::Path const&) macgui_indy3.cpp:880
    #4 0x109adb930 in Scumm::MacGui::MacGui(Scumm::ScummEngine*, Common::Path const&) macgui.cpp:35
    #5 0x109adba64 in Scumm::MacGui::MacGui(Scumm::ScummEngine*, Common::Path const&) macgui.cpp:32
    #6 0x109dd5b17 in Scumm::ScummEngine::init() scumm.cpp:1206
    #7 0x1099004fe in Scumm::ScummEngine::run() scumm.h:581
    #8 0x1097cd2df in runGame(Plugin const*, OSystem&, DetectedGame const&, void const*) main.cpp:311
    #9 0x1097c6d60 in scummvm_main main.cpp:796
    #10 0x1097b4b52 in main macosx-main.cpp:44
    #11 0x7ff800920417 in start+0x767 (dyld:x86_64+0xfffffffffff6e417)

0x61f0000bea06 is located 0 bytes after 2950-byte region [0x61f0000bde80,0x61f0000bea06)
allocated by thread T0 here:
    #0 0x10cc5be40 in wrap_malloc+0xa0 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0xdde40)
    #1 0x109b11622 in Scumm::MacIndy3Gui::readStrings() macgui_indy3.cpp:946
    #2 0x109b0ff4e in Scumm::MacIndy3Gui::MacIndy3Gui(Scumm::ScummEngine*, Common::Path const&) macgui_indy3.cpp:931
    #3 0x109b10c34 in Scumm::MacIndy3Gui::MacIndy3Gui(Scumm::ScummEngine*, Common::Path const&) macgui_indy3.cpp:880
    #4 0x109adb930 in Scumm::MacGui::MacGui(Scumm::ScummEngine*, Common::Path const&) macgui.cpp:35
    #5 0x109adba64 in Scumm::MacGui::MacGui(Scumm::ScummEngine*, Common::Path const&) macgui.cpp:32
    #6 0x109dd5b17 in Scumm::ScummEngine::init() scumm.cpp:1206
    #7 0x1099004fe in Scumm::ScummEngine::run() scumm.h:581
    #8 0x1097cd2df in runGame(Plugin const*, OSystem&, DetectedGame const&, void const*) main.cpp:311
    #9 0x1097c6d60 in scummvm_main main.cpp:796
    #10 0x1097b4b52 in main macosx-main.cpp:44
    #11 0x7ff800920417 in start+0x767 (dyld:x86_64+0xfffffffffff6e417)

SUMMARY: AddressSanitizer: heap-buffer-overflow macgui_impl.cpp:83 in Scumm::MacGuiImpl::readPascalString(unsigned char*&)
Shadow bytes around the buggy address:
  0x61f0000be780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x61f0000be800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x61f0000be880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x61f0000be900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x61f0000be980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x61f0000bea00:[06]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x61f0000bea80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x61f0000beb00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x61f0000beb80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x61f0000bec00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x61f0000bec80: 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
==8371==ABORTING
Abort trap: 6

[dwatteau]

erik told me that he couldn't replicate this issue.

Looking a bit at what I was doing, I was building from an older Git tree from October, by mistake.

So, false alert; this issue doesn't happen with the current Git HEAD anymore, it seems.

Sorry for the noise!

[eriktorbjorn]

Ah. Yeah, I seem to remember fixing a bug where if it extracted the last string in the resource it would read beyond the end of the buffer. Perhaps that's what you saw.