invalid read in utf8EscapedDump

[zaverucha]

When dumping cbor the function utf8EscapedDump can over read the input buffer for some inputs. To reproduce the issue, find the attached zip file, unzip it, then run
valgrind bin/cbordump repro.cbor
which should produce something like:

==30879== Invalid read of size 1
==30879== at 0x402B20: utf8EscapedDump (in bin/cbordump)
==30879== by 0x4032C4: value_to_pretty (in bin/cbordump)
==30879== by 0x402E58: container_to_pretty (in bin/cbordump)
==30879== by 0x403068: value_to_pretty (in bin/cbordump)
==30879== by 0x403744: cbor_value_to_pretty_advance (in bin/cbordump)
==30879== by 0x400FC9: dumpFile (in /bin/cbordump)
==30879== by 0x40116F: main (in bin/cbordump)

I tested this on Ubuntu 16.04.1 LTS, HEAD was at 863a480dc4e61ce35371c4d0db17be14c9a68125

I think the reason is that on lines 195, 203 and 211 of src/cborpretty.c a character of input is consumed, but the count of characters (the variable n) is not decremented.

repro.zip

[thiagomacieira]

Confirmed. Full valgrind output:

==31962== Invalid read of size 1
==31962==    at 0x4028F6: utf8EscapedDump (cborpretty.c:129)
==31962==    by 0x4030DC: value_to_pretty (cborpretty.c:347)
==31962==    by 0x402C2E: container_to_pretty (cborpretty.c:249)
==31962==    by 0x402E4D: value_to_pretty (cborpretty.c:288)
==31962==    by 0x403596: cbor_value_to_pretty_advance (cborpretty.c:467)
==31962==    by 0x400F4A: dumpFile (in /home/tjmaciei/src/tinycbor/bin/cbordump)
==31962==    by 0x4010DC: main (in /home/tjmaciei/src/tinycbor/bin/cbordump)
==31962==  Address 0x54df875 is 0 bytes after a block of size 21 alloc'd
==31962==    at 0x4C2B0FF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==31962==    by 0x40509F: _cbor_value_dup_string (cborparser_dup_string.c:102)
==31962==    by 0x40261A: cbor_value_dup_text_string (cbor.h:394)
==31962==    by 0x40308D: value_to_pretty (cborpretty.c:341)
==31962==    by 0x402C2E: container_to_pretty (cborpretty.c:249)
==31962==    by 0x402E4D: value_to_pretty (cborpretty.c:288)
==31962==    by 0x403596: cbor_value_to_pretty_advance (cborpretty.c:467)
==31962==    by 0x400F4A: dumpFile (in /home/tjmaciei/src/tinycbor/bin/cbordump)
==31962==    by 0x4010DC: main (in /home/tjmaciei/src/tinycbor/bin/cbordump)

[thiagomacieira]

You're right, the decrementing was of n missed.

Reduced test:

$ xxd -ps -r <<<'64 c2 a0 c2 a0' | valgrind bin/cbordump 
==32197== Memcheck, a memory error detector
==32197== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==32197== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==32197== Command: bin/cbordump
==32197== 
==32197== Invalid read of size 1
==32197==    at 0x4028F6: utf8EscapedDump (cborpretty.c:129)
==32197==    by 0x4030DC: value_to_pretty (cborpretty.c:347)
==32197==    by 0x403596: cbor_value_to_pretty_advance (cborpretty.c:467)
==32197==    by 0x400F4A: dumpFile (in /home/tjmaciei/src/tinycbor/bin/cbordump)
==32197==    by 0x401091: main (in /home/tjmaciei/src/tinycbor/bin/cbordump)
==32197==  Address 0x54df0c5 is 0 bytes after a block of size 5 alloc'd
==32197==    at 0x4C2B0FF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==32197==    by 0x40509F: _cbor_value_dup_string (cborparser_dup_string.c:102)
==32197==    by 0x40261A: cbor_value_dup_text_string (cbor.h:394)
==32197==    by 0x40308D: value_to_pretty (cborpretty.c:341)
==32197==    by 0x403596: cbor_value_to_pretty_advance (cborpretty.c:467)
==32197==    by 0x400F4A: dumpFile (in /home/tjmaciei/src/tinycbor/bin/cbordump)
==32197==    by 0x401091: main (in /home/tjmaciei/src/tinycbor/bin/cbordump)
==32197== 
"\u00A0\u00A0\u0000\u0000"

[thiagomacieira]

Is this repro.zip file supposed to be valid contents? I'm still getting "repro.cbor: unexpected end of data"

Update: right, the data is incomplete. The first byte is 0xa5, which indicates there should have been 5 items in the map, but there are only 2.

[zaverucha]

Thanks Thiago! Yes, the data in repro.zip is invalid cbor. Something afl-fuzz came up with. "unexpected end of data" is the expected output
Greg

[thiagomacieira]

Thanks. Has there been anything else found in the fuzzer? I'm pretty confident that the encoder and decoder are fine, this one was in the pretty-printer.

[zaverucha]

Nothing else, I focused on the decoder and I'm pretty confident in it as well. I'm assuming that the parsing done by cbordump is a good proxy for the parsing IoTivity would do when receiving data over the network (i.e., the same APIs/codepaths). Would you agree?

[thiagomacieira]

Correct, but that doesn't mean IoTivity uses it properly.