GNOME Bugzilla – Bug 756456
heap-buffer-overflow in xmlParseConditionalSections
Last modified: 2016-01-15 00:13:23 UTC
The bug is found with libFuzzer (http://llvm.org/docs/LibFuzzer.html) on the fresh git. The 2.9.2 is not affected (at least with this input). A case with a similar stack traces was reported as https://bugzilla.gnome.org/show_bug.cgi?id=744980 but is apparently fixed in the git head. ==28890==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000e8b7 at pc 0x7f19d729a322 bp 0x7ffc649f5c60 sp 0x7ffc649f5c58 READ of size 1 at 0x60200000e8b7 thread T0 #0 0x7f19d729a321 in xmlParseConditionalSections parser.c:6918:9 #1 0x7f19d7297bf1 in xmlParseMarkupDecl__internal_alias parser.c:6986:6 #2 0x7f19d72b38ff in xmlParseInternalSubset parser.c:8420:6 #3 0x7f19d72b28d5 in xmlParseDocument__internal_alias parser.c:10836:6 #4 0x7f19d72c6b48 in xmlDoRead parser.c:15324:5 #5 0x4da50a in parseAndPrintFile xmllint.c:2401:9 #6 0x4d6331 in main xmllint.c:3759:7 0x60200000e8b7 is located 1 bytes to the right of 6-byte region [0x60200000e8b0,0x60200000e8b6) allocated by thread T0 here: #0 0x4ab58b in malloc #1 0x7f19d72779a0 in xmlNewBlanksWrapperInputStream parser.c:2458:14 #2 0x7f19d7298000 in xmlParsePEReference__internal_alias parser.c:8069:14 #3 0x7f19d7297b27 in xmlParseMarkupDecl__internal_alias parser.c:6978:2 #4 0x7f19d72b38ff in xmlParseInternalSubset parser.c:8420:6 #5 0x7f19d72b28d5 in xmlParseDocument__internal_alias parser.c:10836:6 #6 0x7f19d72c6b48 in xmlDoRead parser.c:15324:5 #7 0x4da50a in parseAndPrintFile xmllint.c:2401:9 #8 0x4d6331 in main xmllint.c:3759:7 Reproduce: git clone git://git.gnome.org/libxml2 ./autogen.sh CC="clang -fsanitize=address" ./configure make -j echo PCFET0NUWVBFIGRvYyBbCjwhRU5USVRZICUgZSAiPCFbSU5DTFVERVs8IVtJTkNMVVRFWzxoRSBNWVNTVGNvPXBsZXguZXRkIj4KJWU7Cl0+Ojxkb210bCB4bWxucz0iaHR0cDovL193d2M+PCAuKE1vZzw8 | base64 --decode | ASAN_OPTIONS=strip_path_prefix=`pwd`/:fast_unwind_on_malloc=0 ./xmllint - The target function used with libFuzzer was: extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { if (auto doc = xmlReadMemory(reinterpret_cast<const char *>(data), size, "noname.xml", NULL, 0)) { xmlFreeDoc(doc); } return 0; }
could you run again after following change: 6910 if (RAW == 0) { 6911 xmlFatalErr(ctxt, XML_ERR_CONDSEC_NOT_FINISHED, NULL); 6912 xmlStopParser(ctxt); 6913 return; 6914 } else {
Nope, the bug is still happening....
I could bypass this with a check that the SKIP(3) at the end of xmlParseConditionalSections() was in range, but I don't know what the complete/correct fix is. diff --git a/parser.c b/parser.c index 81620acf416d..6326e9880a61 100644 --- a/parser.c +++ b/parser.c @@ -6918,7 +6921,9 @@ xmlParseConditionalSections(xmlParserCtxtPtr ctxt) { "All markup of the conditional section is not in the same entity\n", NULL, NULL); } - SKIP(3); + if ((ctxt->input->cur + 3) < ctxt->input->end) { + SKIP(3); + } } }
Yeah, I think we need first to check against xmlStopParser having been called, but that extra check is not a bad idea anyway. So I ended up with a slightly different patch: https://git.gnome.org/browse/libxml2/commit/?id=bd0526e66a56e75a18da8c15c4750db8f801c52d thanks ! Daniel
Now part of 2.9.3 upstream release, Daniel
(In reply to Daniel Veillard from comment #4) > Yeah, I think we need first to check against xmlStopParser having been > called, but that extra check is not a bad idea anyway. So I ended up with a > slightly different patch: > > https://git.gnome.org/browse/libxml2/commit/ > ?id=bd0526e66a56e75a18da8c15c4750db8f801c52d There was one additional follow-up fix for this issue here (for anyone looking to merge the complete fix): https://git.gnome.org/browse/libxml2/commit/?id=41ac9049a27f52e7a1f3b341f8714149fc88d450