stack-buffer-overflow and heap-buffer-overflow
NotmebutWind opened this issue · 7 comments
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)
@liweiii Note that I do not consider leaks at exit a bug - unfortunately AddressSanitizer on Linux defaults to performing leak checks at exit which often yields false positives. When a process goes away, so does any memory it allocated...
Anyways, if you will kindly provide the input that causes the stack/heap overflow issues I will look into this further.
Here is the input . I think the reason I have found. leak is because you didn‘’t free the memory. I think if this lib is used in an server or parse a lot of XML ,maybe OOM and crash will occur. overflow is because my input string is not a well-formed XML string. so it's your deal if it's necessery to change the code or tell that only formed strings to use mxmlLoadStrin
crash-71c1bb2443bbb3249e11999ba2d8178a52a7166c.zip
crash-3d7f02c2a2b7910101ebf16005e97a8f0bfebb06.zip
g.
@liweiii OK, after testing the current master code, I can't get this to fail. Can you re-test and let me know if the other changes I've made have corrected the issue?
Closing as fixed in current master; if you can reproduce on master, please let me know the details!
Hi,
I'm trying to triage CVE-2021-42859 on behalf of Debian. I have been unable to reproduce using the PoC ZIP attached to this issue using version 3.2 as packaged in Debian. (https://tracker.debian.org/pkg/mxml)
@michaelrsweet - if the issue cannot be reproduced, can I ask that you, as upstream, request that the CVE is rejected at https://cveform.mitre.org/ please? (using the requesting an update of the CVE option)
@codehelp Done!