CVE-2021-42860: stack-buffer-overflow and heap-buffer-overflow · Issue #286 · michaelrsweet/mxml

Hi,

We have used Mini-xml in our project, so I test v3.2 and master branch and found something:

Fisrt, there are some memory leaks in v3.2 and master:

=================================================================
==111401==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 6 byte(s) in 1 object(s) allocated from:
    #0 in strdup (/opt/mnt/software/mxml32/a.out+0x46f260)
    #1 in mxmlSaveAllocString /opt/mnt/software/mxml32/mxml-file.c:227:13
    #2 in LLVMFuzzerTestOneInput /opt/mnt/software/mxml32/mxml_fuzzer.cpp:23:8
    #3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/opt/mnt/software/mxml32/a     .out+0x42f357)
    #4 in fuzzer::Fuzzer::MutateAndTestOne() (/opt/mnt/software/mxml32/a.out+0x439bc4)
    #5 in fuzzer::Fuzzer::Loop(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::a     llocator<char> >, fuzzer::fuzzer_allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<     char> > > > const&) (/opt/mnt/software/mxml32/a.out+0x43b22f)
    #6 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/opt/mnt/soft     ware/mxml32/a.out+0x42a5ec)
    #7 in main (/opt/mnt/software/mxml32/a.out+0x41d4b2)
    #8 in __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:310

SUMMARY: AddressSanitizer: 6 byte(s) leaked in 1 allocation(s).
INFO: to ignore leaks on libFuzzer side use -detect_leaks=0.

and :
this is your testmxml.c:

...
Creating libmxml.so.1.6...
Creating libmxml.a...
a - mxml-attr.o
a - mxml-entity.o
a - mxml-file.o
a - mxml-get.o
a - mxml-index.o
a - mxml-node.o
a - mxml-search.o
a - mxml-set.o
a - mxml-private.o
a - mxml-string.o
Compiling testmxml.c
Linking testmxml...
Testing library...

=================================================================
==113129==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 88 byte(s) in 1 object(s) allocated from:
    #0 in calloc (/opt/mnt/software/mxml32/testmxml+0x4da178)
    #1 in mxml_new /opt/mnt/software/mxml32/mxml-node.c:841:15
    #2 in mxmlNewElement /opt/mnt/software/mxml32/mxml-node.c:382:15
    #3 in mxml_load_data /opt/mnt/software/mxml32/mxml-file.c:1783:14
    #4 in mxmlSAXLoadFile /opt/mnt/software/mxml32/mxml-file.c:467:11
    #5 in main /opt/mnt/software/mxml32/testmxml.c:676:5
    #6 in __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:310

Indirect leak of 37 byte(s) in 1 object(s) allocated from:
    #0 in __interceptor_strdup (/opt/mnt/software/mxml32/testmxml+0x436770)
    #1 in mxmlNewElement /opt/mnt/software/mxml32/mxml-node.c:383:32
    #2 in mxml_load_data /opt/mnt/software/mxml32/mxml-file.c:1783:14
    #3 in mxmlSAXLoadFile /opt/mnt/software/mxml32/mxml-file.c:467:11
    #4 in main /opt/mnt/software/mxml32/testmxml.c:676:5
    #5 in __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:310

SUMMARY: AddressSanitizer: 125 byte(s) leaked in 2 allocation(s).
Makefile:271: recipe for target 'testmxml' failed
make: *** [testmxml] Error 1

also ,we I input an unformed string to mxmlLoadString, there will be a stack-buffer-overflow and heap-buffer-overflow. I think if you add a longth check in mxml_string_getc when every pointer change(“like (*s)++”), will be better? Of course Maybe I have use it in a wrong . you can check it here:
this is my testcase:

#include <string>
#include <vector>
#include <assert.h>
#include "mxml.h"

extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {

std::string c(reinterpret_cast<const char *>(data), size);

char *ptr;

mxml_node_t *tree;

tree = mxmlLoadString(NULL, c.c_str(), MXML_NO_CALLBACK);

if(tree){
        ptr = mxmlSaveAllocString(tree, MXML_NO_CALLBACK);
        if(!ptr) assert(false);
        mxmlDelete(tree);
}

return 0;
}

you can compile your lib with
CFLAGS =+ "-g -O0 -fno-omit-frame-pointer -gline-tables-only -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link" and
LDFLAGS =+"-fsanitize=fuzzer-no-link -fsanitize=address"
and
clang++ -g -O1 -fno-omit-frame-pointer -gline-tables-only -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link mxml_fuzzer.cpp -I ./ -fsanitize=fuzzer ./libmxml.a

run and these are the backtrace:

=================================================================
==2858==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffed9190220 at pc 0x00000055a6f2 bp 0x7ffed918edc0 sp 0x7ffed918edb8
READ of size 1 at 0x7ffed9190220 thread T0
    #0 in mxml_string_getc /opt/mnt/software/mxml32/mxml-file.c:2611:16
    #1 in mxml_parse_element /opt/mnt/software/mxml32/mxml-file.c:2141:16
    #2 in mxml_load_data /opt/mnt/software/mxml32/mxml-file.c:1977:14
    #3 in mxmlLoadString /opt/mnt/software/mxml32/mxml-file.c:180:11
    #4 in LLVMFuzzerTestOneInput /opt/mnt/software/mxml32/mxml_fuzzer.cpp:12:8
    #5 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/opt/mnt/software/mxml32/a.out+0x42f357)
    #6 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/opt/mnt/software/mxml32/a.out+0x41f7ea)
    #7 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/opt/mnt/software/mxml32/a.out+0x42a7b0)
    #8 in main (/opt/mnt/software/mxml32/a.out+0x41d4b2)
    #9 in __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:310
    #10 in _start (/opt/mnt/software/mxml32/a.out+0x41d529)


=================================================================
==6265==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x612000000a73 at pc 0x000000558e2d bp 0x7ffe13e2caa0 sp 0x7ffe13e2ca98
READ of size 1 at 0x612000000a73 thread T0
    #0 in mxml_string_getc /opt/mnt/software/mxml32/mxml-file.c:2422:13
    #1 in mxml_load_data /opt/mnt/software/mxml32/mxml-file.c:1558:20
    #2 in mxmlLoadString /opt/mnt/software/mxml32/mxml-file.c:180:11
    #3 in LLVMFuzzerTestOneInput /opt/mnt/software/mxml32/mxml_fuzzer.cpp:12:8
    #4 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/opt/mnt/software/mxml32/a.out+0x42f357)
    #5 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/opt/mnt/software/mxml32/a.out+0x41f7ea)
    #6 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/opt/mnt/software/mxml32/a.out+0x42a7b0)
    #7 in main (/opt/mnt/software/mxml32/a.out+0x41d4b2)
    #8 in __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:310
    #9 in _start (/opt/mnt/software/mxml32/a.out+0x41d529)