search

michaelrsweet / codedoc

Documentation generator for C/C++ code
https://www.msweet.org/codedoc
Apache License 2.0
48 stars 6 forks source link

FIx heap use after free and double free in codedoc.c #17

Closed ashamedbit closed 9 months ago

ashamedbit commented 9 months ago

Hello, I am a new contributor to the repository :). I was going through the issues and was able to create a patch for issue #16 . Additionally, I also found the following Heap-Use-After-Free (testcase attached below poc188.txt)

================================================================= ==80285==ERROR: AddressSanitizer: heap-use-after-free on address 0x6080000014b8 at pc 0x00000054f2c5 bp 0x7fff77a0a130 sp 0x7fff77a0a128 READ of size 8 at 0x6080000014b8 thread T0

0 0x54f2c4 in mxmlRemove /home/aniruddhan/mxml/mxml-node.c:646:23

#1 0x54f44a in mxmlDelete /home/aniruddhan/mxml/mxml-node.c:195:3
#2 0x4ff706 in scan_file /home/aniruddhan/mxml/codedoc/codedoc.c:3671:9
#3 0x4f85e7 in main /home/aniruddhan/mxml/codedoc/codedoc.c:532:12
#4 0x7f9782378082 in __libc_start_main /build/glibc-wuryBv/glibc-2.31/csu/../csu/libc-start.c:308:16
#5 0x41c56d in _start (/home/aniruddhan/mxml/codedoc/a.out+0x41c56d)

0x6080000014b8 is located 24 bytes inside of 88-byte region [0x6080000014a0,0x6080000014f8) freed by thread T0 here:

0 0x4c1387 in free /home/aniruddhan/aflgo/instrument/llvm_tools/compiler-rt/lib/asan/asan_malloc_linux.cpp:123:3

#1 0x54f521 in mxmlDelete /home/aniruddhan/mxml/mxml-node.c:231:5
#2 0x4fbd4c in scan_file /home/aniruddhan/mxml/codedoc/codedoc.c:3138:7
#3 0x4f85e7 in main /home/aniruddhan/mxml/codedoc/codedoc.c:532:12
#4 0x7f9782378082 in __libc_start_main /build/glibc-wuryBv/glibc-2.31/csu/../csu/libc-start.c:308:16

previously allocated by thread T0 here:

0 0x4c1847 in calloc /home/aniruddhan/aflgo/instrument/llvm_tools/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3

#1 0x54f969 in mxml_new /home/aniruddhan/mxml/mxml-node.c:836:15

The double free or the HUAF occurs because temp (an mxml_node) returned from mxmlFindElement is freed within function sort_node. But the definition of mxmlFindElement shows that the returned node is not a node that is allocated with mxmlNewElement and therefore should not be freed. This leads to both heap-use-after-free and double free later on in the code.

After removing the free statement I was able to verify that both heap use-after-free and double-free were gone.

Let me know if the patch is helpful :)

michaelrsweet commented 9 months ago

Nothing says that the pointer returned by mxmlFindElement can't be deleted - the ownership of nodes is managed outside of Mini-XML...

This is more an issue of a current constant being overridden by a new one - not something that will normally occur (you'd get a compile error) but something that we need to track somehow. Without the delete you'll end up with duplicate nodes, so this fix isn't correct.

Given that you have the poc188.txt in the other PR, I'll close this one out and track any changes over there.