stack-buffer-overflow and heap-buffer-overflow

[NotmebutWind]

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)

[michaelrsweet]

@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.

[NotmebutWind]

Hi, I argee with your idea that ASan is check leak that way, but that not mean that leaks occurred when exit! Besides, I didn't use ASan to check ,but LSan, it can report leak when it occurs. Second, I couldn't upload the crash case until Monday, but you can quickly reproduce the crash used my teatcase and compile lines provided in issue. 

Best Wishes

------------------ 原始邮件 ------------------ 发件人: "michaelrsweet/mxml" @.>; 发送时间: 2021年10月15日(星期五) 晚上9:46 @.>; 抄送: "COME WITH @.**@.>; 主题: Re: [michaelrsweet/mxml] stack-buffer-overflow and heap-buffer-overflow (#286)

@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.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[NotmebutWind]

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.

[michaelrsweet]

@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?

[michaelrsweet]

Closing as fixed in current master; if you can reproduce on master, please let me know the details!

[codehelp]

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)

[michaelrsweet]

@codehelp Done!