Memory errors uncovered by fuzzing

[erkyrath]

Lionel Debroux did a round of testing glulxe with deliberately corrupted game files and uncovered a variety of memory handling errors.

I'm not attaching all the data files from the run, for they are large. But here is the uniqued list of crash points:

   2     #0 0x437a90 in fread (.../glulxe/glulxe/glulxe+0x437a90)
  64     #0 0x4971cd in free (.../glulxe/glulxe/glulxe+0x4971cd)
   1     #0 0x4973e9 in malloc (.../glulxe/glulxe/glulxe+0x4973e9)
 265     #0 0x49744d in malloc (.../glulxe/glulxe/glulxe+0x49744d)
   2     #0 0x49fbf0 in __sanitizer::BufferedStackTrace::UnwindImpl(unsigned long, unsigned long, void*, bool, unsigned int) (.../glulxe/glulxe/glulxe+0x49fbf0)
   1     #0 0x4a2e00 in __asan::GetCurrentThread() (.../glulxe/glulxe/glulxe+0x4a2e00)
   1     #0 0x4b7584 in __sanitizer::StackDepotBase<__sanitizer::StackDepotNode, 1, 20>::Put(__sanitizer::StackTrace, bool*) (.../glulxe/glulxe/glulxe+0x4b7584)
   1     #0 0x4b7590 in __sanitizer::StackDepotBase<__sanitizer::StackDepotNode, 1, 20>::Put(__sanitizer::StackTrace, bool*) (.../glulxe/glulxe/glulxe+0x4b7590)
  16     #0 0x4c9a3c in pop_arguments .../glulxe/glulxe/vm.c:310:19
   8     #0 0x4c9a94 in pop_arguments .../glulxe/glulxe/vm.c:310:19
  39     #0 0x4c9b69 in pop_arguments .../glulxe/glulxe/vm.c:310:17
  16     #0 0x4c9d5f in pop_arguments .../glulxe/glulxe/vm.c:310:17
   1     #0 0x4d2dee in execute_loop .../glulxe/glulxe/exec.c:524:19
   1     #0 0x4d3b00 in enter_function .../glulxe/glulxe/funcs.c:64:5
   6     #0 0x4d588f in enter_function .../glulxe/glulxe/funcs.c:63:5
   9     #0 0x4d5c82 in pop_callstub .../glulxe/glulxe/funcs.c:231:17
  17     #0 0x4d5d31 in pop_callstub .../glulxe/glulxe/funcs.c:240:29
   2     #0 0x4d8065  (.../glulxe/glulxe/glulxe+0x4d8065)
 398     #0 0x4d8065 in parse_operands .../glulxe/glulxe/operand.c:427:19
   1     #0 0x4d8ed8 in parse_operands .../glulxe/glulxe/operand.c:433:19
 207     #0 0x4d939c in parse_operands .../glulxe/glulxe/operand.c:427:19
   1     #0 0x4d93c7 in parse_operands .../glulxe/glulxe/operand.c:430:19
 120     #0 0x4d95b0 in store_operand .../glulxe/glulxe/operand.c:555:5
  94     #0 0x4d9769 in store_operand .../glulxe/glulxe/operand.c:555:5
   2     #0 0x4d9c12 in store_operand_b .../glulxe/glulxe/operand.c:619:5
   1     #0 0x5099e7  (.../glulxe/glulxe/glulxe+0x5099e7)
  64     #0 0x5099e7 in glk_stream_open_file_uni .../glulxe/cheapglk/cgstream.c:317:18
   1     #0 0x50b15d in glk_stream_set_position .../glulxe/cheapglk/cgstream.c:520:18
   1     #0 0x516cde in gli_buffer_change_case .../glulxe/cheapglk/cgunicod.c:234:21
  10     #0 0x517c69 in gli_buffer_change_case .../glulxe/cheapglk/cgunicod.c:234:21
  79     #0 0x519dac in gli_buffer_canon_decompose_uni .../glulxe/cheapglk/cgunicod.c:368:21
   2     #0 0x52e30d in giblorb_initialize_map .../glulxe/cheapglk/gi_blorb.c:291:38
   1     #0 0x52e382 in giblorb_initialize_map .../glulxe/cheapglk/gi_blorb.c:264:26

[erkyrath]

glulxe_054_cheapglk_106_crashes_output.zip

Full stack traces for each crash are listed in this file (compressed).

The game files that generated these crashes are a large collection (55Mb compressed); I'll keep them on hand until I've looked at the errors.

[erkyrath]

After poking through that a bit, all the crashes in glulxe code fall into two categories:

• The StkN() / StkWN() macros;
• Infinite recursion in buildcache() in string.c

The Stk macros can be tricked up with verify code (at the cost of some performance).

The buildcache code can have a depth limit. I'll have to see what the typical depth is in "normal" games, but the reported crashes are at about 240.

There are a few more crashes in cheapglk that I have not yet looked at. I suspect they result from passing a block of VM memory through the dispatch layer without checking its bounds. (I.e., passing VM memory through to gli_buffer_change_case().) This should be handleable by calling verify_address() in glkop somewhere.

[erkyrath]

Fixed a bunch of holes. Remaining in group 1:

1/crashes/id:000296,sig:11,src:004679+003734,time:513078710,op:splice,rep:4 -- glk_buffer_to_title_case_uni() gets ptr=null, len=0, numchars=1. Is this a glkop error? Whose job is it to check numchars<len?

MORE INFO:

glk_buffer_to_title_case_uni(1, 0, 1, 0);

That is, the argument is not a null reference, but an array of length zero. This should be valid, but CaptureIArray() / grab_temp_i_array() returns NULL. This gets into cgunicode.c and crashes.

Conclusion: I think grab_temp_i_array() should malloc and return an array, of which zero bytes will be used.

(But also have the glk_buffer_xxx funcs check that numchars<len?)

[erkyrath]

Group 2:

2/crashes/id:000358,sig:06,src:004409,time:395055860,op:havoc,rep:8 -- gli_buffer_canon_decompose_uni() -- this is not a null pointer, it's something else.
2/crashes/id:000376,sig:06,src:004753,time:442584451,op:havoc,rep:2 -- gli_buffer_canon_decompose_uni()
2/crashes/id:000399,sig:06,src:002594,time:506480881,op:havoc,rep:4 -- glk_stream_set_position() in giblorb_load_chunk_by_number()

[erkyrath]

Group 3:

3/crashes/id:000262,sig:06,src:002830+000794,time:56824584,op:splice,rep:2 -- gli_buffer_canon_decompose_uni()
3/crashes/id:000289,sig:06,src:003088,time:69244735,op:havoc,rep:64 -- giblorb_initialize_map()

By the way, I'm retesting these crashes with libgmalloc.dylib, which is not necessarily going to pick up the same failures as Lionel found. Hopefully they're on the same page, though, no pun intended.

[erkyrath]

stillcrash.zip

These are the six Glulx game files which are still crashy. (Tested under glulxe with cheapglk and libgmalloc.dylib.) I don't have time to investigate this further right now, but I'll keep the issue open.

[curiousdannii]

Would it be worth uploading the test files somewhere so that Git and Quixe could be tested too?

[erkyrath]

Ah, that is a good point. Hadn't thought of that.

It's 55MB and Github doesn't like attachments over 10MB. Let me see...

[erkyrath]

I guess what I should do is boil it down to one test case per crash point. That will only be about thirty rather than 1150 of them.

[erkyrath]

crash-gamefiles.zip

Here's a selection. This is a subset of the crashes listed in glulxe_054_cheapglk_106_crashes_output.zip (uploaded above).

[debrouxl]

Years ago, I had bad experiences with the crash minimizers, which is why I usually provide unfiltered sets of samples to maintainers, even if they can be 95+% redundant, and minimizers have probably improved since then. If needed, I can privately provide the link to the full tarball, and/or upload the queue of samples produced by afl-fuzz (should be 10K+ files), so that you don't have to restart from scratch :)

With AFL (nowadays, AFL++), different implementations which consume the same input file formats can be fuzzed cooperatively, which can help increase the coverage faster in some cases. I used that approach when fuzzing the module / track loaders several years ago ( https://www.openwall.com/lists/oss-security/2017/11/02/8 ).
Fuzzing detects more issues with AddressSanitizer implementation (AFL_USE_ASAN=1 when building using CC=afl-clang-fast / CXX=afl-clang-fast++).
Honggfuzz is another good fuzzer, traditionally less oriented towards cooperative fuzzing. In my runs, it found several issues AFL didn't.

The input corpus I used was fairly small: it consisted of several dozen .ulx, .blb, .glblorb, .z5 files which do not exercise every possible language feature supported by glulxe. Fuzzing speed is higher on Linux - fast fork()+exec() and optimizations of that sequence - and higher with smaller input files, which would mean short texts and commands, tiny media files, etc. The games exercising the various file formats and language features don't have to make any sense to humans, they just need to provide good initial test coverage in order to reduce CPU time waste, and they need not to crash the interpreter.

Fuzzing an Inform compiler would probably be best done using a dictionary: flat plain text file of keyword_name="keyword" lines.