GNOME Bugzilla – Bug 759671
Heap-based buffer overread in xmlNextChar (from parser.c:8472)
Last modified: 2017-02-08 21:49:55 UTC
Hi, The following heap overread can be observed on an ASAN build of the latest Libxml2 stable release version 2.9.3: $ ./xmllint c01_id0051.min ================================================================= ==10002== ERROR: AddressSanitizer: heap-buffer-overflow on address 0xb5800597 at pc 0x809cef0 bp 0xbfadf238 sp 0xbfadf22c READ of size 1 at 0xb5800597 thread T0 #0 0x809ceef in xmlNextChar ~/libxml2-2.9.3/parserInternals.c:535 #1 0x814cfd7 in xmlParseInternalSubset ~/libxml2-2.9.3/parser.c:8472 #2 0x817b450 in xmlParseDocument ~/libxml2-2.9.3/parser.c:10880 #3 0x81b5589 in xmlDoRead ~/libxml2-2.9.3/parser.c:15390 #4 0x81b5589 in xmlReadFile ~/libxml2-2.9.3/parser.c:15452 #5 0x805d5a9 in parseAndPrintFile ~/libxml2-2.9.3/xmllint.c:2401 #6 0x8051352 in main ~/libxml2-2.9.3/xmllint.c:3759 #7 0xb6006a82 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287 #8 0x80555c0 in _start (~/libxml2-2.9.3/libxml293/bin/xmllint+0x80555c0) 0xb5800597 is located 0 bytes to the right of 7-byte region [0xb5800590,0xb5800597) allocated by thread T0 here: #0 0xb61f8854 in malloc (/usr/lib/i386-linux-gnu/libasan.so.0+0x16854) #1 0x8507dff in xmlStrndup ~/libxml2-2.9.3/xmlstring.c:45 SUMMARY: AddressSanitizer: heap-buffer-overflow ~/libxml2-2.9.3/parserInternals.c:529 xmlNextChar Shadow bytes around the buggy address: 0x36b00060: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x36b00070: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x36b00080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x36b00090: fa fa fa fa fa fa 07 fa fa fa fd fd fa fa fd fd 0x36b000a0: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd =>0x36b000b0: fa fa[07]fa fa fa 02 fa fa fa 02 fa fa fa 00 07 0x36b000c0: fa fa fd fd fa fa fd fa fa fa 04 fa fa fa 04 fa 0x36b000d0: fa fa 00 07 fa fa fd fd fa fa fd fa fa fa fd fd 0x36b000e0: fa fa 05 fa fa fa 00 01 fa fa 06 fa fa fa 00 03 0x36b000f0: fa fa 07 fa fa fa 00 01 fa fa 00 01 fa fa 06 fa 0x36b00100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap righ redzone: fb Freed Heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 ASan internal: fe ==10002== ABORTING Aborted $ xxd -g 1 c01_id0051.min 0000000: 3c 3f 68 3f 3e 3c 21 44 4f 43 54 59 50 45 74 5b <?h?><!DOCTYPEt[ 0000010: 3c 21 45 4c 45 4d 45 4e 54 20 74 20 28 41 29 3e <!ELEMENT t (A)> 0000020: 3c 21 45 4e 54 49 54 59 20 25 20 78 78 20 27 26 <!ENTITY % xx '& 0000030: 23 33 37 3b 5d 26 23 33 37 3b 4d 26 23 33 37 3b #37;]%M% 0000040: 7a 27 3e 3c 21 45 4e 54 49 54 59 7a 27 ff 27 3e z'><!ENTITYz'.'> 0000050: 25 78 78 3b %xx;
Ah, I've been hitting it constantly with libFuzzer in a slightly different form: ==11961==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000b60fa1 at pc 0x00000099b4f1 bp 0x7ffde259e3d0 sp 0x7ffde259e3c8 READ of size 1 at 0x000000b60fa1 thread T0 #0 0x99b4f0 in xmlNextChar parserInternals.c:535:13 #1 0x5769e1 in xmlParseDocument parser.c:10880:6 #2 0x5a0575 in xmlDoRead parser.c:15390:5 #3 0x5a0a50 in xmlReadMemory parser.c:15476:13 #4 0x4df7bb in LLVMFuzzerTestOneInput (/usr/local/google/home/kcc/fuzz/libxml/libxml_fuzzer+0x4df7bb) #5 0xb264c5 in fuzzer::SimpleUserSuppliedFuzzer::TargetFunction(unsigned char const*, unsigned long) (/usr/local/google/home/kcc/fuzz/libxml/libxml_fuzzer+0xb264c5) #6 0xb336bf in fuzzer::Fuzzer::ExecuteCallback(std::vector<unsigned char, std::allocator<unsigned char> > const&) (/usr/local/google/home/kcc/fuzz/libxml/libxml_fuzzer+0xb336bf) #7 0xb23788 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*) (/usr/local/google/home/kcc/fuzz/libxml/libxml_fuzzer+0xb23788) #8 0xb2410c in fuzzer::FuzzerDriver(std::vector<std::string, std::allocator<std::string> > const&, fuzzer::UserSuppliedFuzzer&) (/usr/local/google/home/kcc/fuzz/libxml/libxml_fuzzer+0xb2410c) #9 0xb238d6 in fuzzer::FuzzerDriver(int, char**, fuzzer::UserSuppliedFuzzer&) (/usr/local/google/home/kcc/fuzz/libxml/libxml_fuzzer+0xb238d6) #10 0xb2381a in fuzzer::FuzzerDriver(int, char**, int (*)(unsigned char const*, unsigned long)) (/usr/local/google/home/kcc/fuzz/libxml/libxml_fuzzer+0xb2381a) #11 0xb3f450 in main (/usr/local/google/home/kcc/fuzz/libxml/libxml_fuzzer+0xb3f450) #12 0x7f76d7127ec4 in __libc_start_main /tmp/tmp.Htw1L27e9P/csu/libc-start.c:287 #13 0x41be75 in _start (/usr/local/google/home/kcc/fuzz/libxml/libxml_fuzzer+0x41be75) 0x000000b60fa1 is located 63 bytes to the left of global variable '<string literal>' defined in 'parser.c:3131:8' (0xb60fe0) of size 40 '<string literal>' is ascii string 'Name %s is not XML Namespace compliant ' 0x000000b60fa1 is located 0 bytes to the right of global variable '<string literal>' defined in 'parser.c:3115:33' (0xb60fa0) of size 1 '<string literal>' is ascii string '' SUMMARY: AddressSanitizer: global-buffer-overflow parserInternals.c:535:13 in xmlNextChar
Created attachment 319996 [details] repro
[moved from bug 756525, where I originally put this comment in error] I could still reproduce both errors, so I bodged a couple of fixes to unblock Kostya's fuzzer (although I don't think the parserInternals.c change is correct): diff --git a/parser.c b/parser.c index c5741e3..117e9c5 100644 --- a/parser.c +++ b/parser.c @@ -12672,6 +12672,7 @@ xmlHaltParser(xmlParserCtxtPtr ctxt) { } ctxt->input->cur = BAD_CAST""; ctxt->input->base = ctxt->input->cur; + ctxt->input->end = ctxt->input->cur + 1; } } diff --git a/parserInternals.c b/parserInternals.c index 2b8646c..e18c76c 100644 --- a/parserInternals.c +++ b/parserInternals.c @@ -532,8 +532,13 @@ xmlNextChar(xmlParserCtxtPtr ctxt) ctxt->input->col++; ctxt->input->cur++; ctxt->nbChars++; - if (*ctxt->input->cur == 0) + if ((ctxt->input->cur >= ctxt->input->end) || + (*ctxt->input->cur == 0)) xmlParserInputGrow(ctxt->input, INPUT_CHUNK); + if (ctxt->input->cur >= ctxt->input->end) { + xmlPopInput(ctxt); + return; + } } if ((*ctxt->input->cur == '%') && (!ctxt->html)) xmlParserHandlePEReference(ctxt);
Created attachment 320476 [details] c01_id0051.min Test case c01_id0051.min from Comment #0.
Thanks David, but the proper fix is not not skip the undetected '>' at the end of the internal subset and return directly in case of error. Patch commited: https://git.gnome.org/browse/libxml2/commit/?id=a7a94612aa3b16779e2c74e1fa353b5d9786c602 Daniel
Hi Daniel Has a CVE been requested for this issue? (or not worth of?). The commit a7a94612aa3b16779e2c74e1fa353b5d9786c602 references #759671. Regards, Salvatore
Hi To avoid confusions I suggest to open/unembargo this one. The reason is that the commit references to 759671. But already distributions have CVE-2016-1762 marked as "Apple specific", e.g. https://people.canonical.com/~ubuntu-security/cve/2016/CVE-2016-1762.html Regards, Salvatore
(In reply to Salvatore Bonaccorso from comment #7) > Hi > > To avoid confusions I suggest to open/unembargo this one. The reason is that > the commit references to 759671. But already distributions have > CVE-2016-1762 marked as "Apple specific", e.g. > https://people.canonical.com/~ubuntu-security/cve/2016/CVE-2016-1762.html > > Regards, > Salvatore This issue is not Apple-specific; the CVE-ID was just assigned from Apple's pool because you requested one.
*** Bug 751643 has been marked as a duplicate of this bug. ***
Removing security group as commit is public.
*** Bug 758549 has been marked as a duplicate of this bug. ***
*** Bug 756525 has been marked as a duplicate of this bug. ***