New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 2 users
Status: Fixed
Owner:
Email to this user bounced
Closed: Sep 2011
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
4 OOB reads in XMLDocumentParser::doWrite
Project Member Reported by infe...@chromium.org, Sep 6 2011 Back to list
found in my fuzzing + ASAN + ClusterFuzz

Bot CLUSTER_FUZZ_56 on platform LINUX
Chromium Revision : 99672
Webkit Revision : 94518

Testcase:: (run from LayoutTests/fast/xsl)
<?xml version="1.0"?>
<?xml-stylesheet type="text/xsl" href="mozilla-tests.xsl"?>

/mnt/scratch0/chrome/src/out/Release/chrome --allow-file-access-from-files --disable-click-to-play --disable-hang-monitor --disable-metrics --disable-popup-blocking --disable-prompt-on-repost --enable-desktop-notifications --enable-experimental-extension-apis --enable-extension-apps --enable-extension-timeline-api --enable-geolocation --enable-indexed-database --enable-nacl --enable-native-web-workers --enable-search-provider-api-v2 --force-internal-pdf --incognito --js-flags="--expose-gc" --new-window --no-default-browser-check --no-first-run --no-process-singleton-dialog --no-sandbox --single-process --disable-gpu-plugin --disable-gpu-rendering --disable-accelerated-compositing --disable-webgl --disable-accelerated-2d-canvas --user-data-dir=/mnt/scratch0/FuzzTmp/t36 

ASAN:SIGILL
=================================================================
HINT: if your stack trace looks short or garbled, use ASAN_OPTIONS=fast_unwind=0
==17245== ERROR: AddressSanitizer global-buffer-overflow on address 0x00007f009eee8aa1 at pc 0x7f0099dce4d2 bp 0x7f007f4509e0 sp 0x7f007f450560
READ of size 1 at 0x00007f009eee8aa1 thread T12
    #0 0x7f0099dce4d2 in xmlParseTryOrFinish third_party/libxml/src/parser.c:0
    #1 0x7f0099dcc7b9 in xmlParseChunk 
    #2 0x7f009b3bafd0 in WebCore::XMLDocumentParser::doWrite(WTF::String const&) 
    #3 0x7f009b3b44f7 in WebCore::XMLDocumentParser::append(WebCore::SegmentedString const&) 
    #4 0x7f009da5e377 in WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter*, char const*, unsigned long) 
    #5 0x7f009b113944 in WebCore::DocumentLoader::commitData(char const*, unsigned long) 
    #6 0x7f009a19dfa8 in WebKit::FrameLoaderClientImpl::committedLoad(WebCore::DocumentLoader*, char const*, int) 
    #7 0x7f009b113530 in WebCore::DocumentLoader::commitLoad(char const*, int) 
    #8 0x7f009b1b3144 in WebCore::ResourceLoader::didReceiveData(char const*, int, long long, bool) 
    #9 0x7f009b18f63b in WebCore::MainResourceLoader::didReceiveData(char const*, int, long long, bool) 
    #10 0x7f009b1b4c9b in WebCore::ResourceLoader::didReceiveData(WebCore::ResourceHandle*, char const*, int, int) 
    #11 0x7f009a033dc3 in ResourceDispatcher::OnReceivedData(IPC::Message const&, int, base::FileDescriptor, int, int) 
    #12 0x7f009a033403 in ResourceDispatcher::DispatchMessage(IPC::Message const&) 
    #13 0x7f009a0312e7 in ResourceDispatcher::OnMessageReceived(IPC::Message const&) 
    #14 0x7f0099f37660 in ChildThread::OnMessageReceived(IPC::Message const&) 
    #15 0x7f009a08c021 in IPC::ChannelProxy::Context::OnDispatchMessage(IPC::Message const&) 
    #16 0x7f0098982589 in base::subtle::TaskClosureAdapter::Run() 
    #17 0x7f0098910c7e in MessageLoop::RunTask(MessageLoop::PendingTask const&) 
    #18 0x7f0098911272 in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) 
    #19 0x7f0098912461 in MessageLoop::DoWork() 
    #20 0x7f009891b53a in base::MessagePumpDefault::Run(base::MessagePump::Delegate*) 
    #21 0x7f009890fb5a in MessageLoop::RunInternal() 
    #22 0x7f009890dc59 in MessageLoop::Run() 
    #23 0x7f0098985cc8 in base::Thread::ThreadMain() 
    #24 0x7f009898493c in base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:0
    #25 0x7f009df82311 in AsanThread::ThreadStart() /usr/local/google/asan/address-sanitizer/asan/asan_thread.cc:102
    #26 0x7f0092dc59ca in start_thread /build/buildd/eglibc-2.11.1/nptl/pthread_create.c:300
    #27 0x7f0090d2370d in ?? /build/buildd/eglibc-2.11.1/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:114
0x00007f009eee8aa1 is located 0 bytes to the right of global variable '.str11' (0x7f009eee8aa0) of size 1
==17245== ABORTING
Shadow byte and word:
  0x00001fe013ddd154: 1
  0x00001fe013ddd150: fc fc fc fc 01 f9 f9 f9
More shadow bytes:
  0x00001fe013ddd130: 00 00 00 00 00 01 f9 f9
  0x00001fe013ddd138: fc fc fc fc 00 00 00 00
  0x00001fe013ddd140: 02 f9 f9 f9 fc fc fc fc
  0x00001fe013ddd148: 00 00 00 00 05 f9 f9 f9
=>0x00001fe013ddd150: fc fc fc fc 01 f9 f9 f9
  0x00001fe013ddd158: fc fc fc fc 00 00 00 00
  0x00001fe013ddd160: 00 f9 f9 f9 fc fc fc fc
  0x00001fe013ddd168: 00 00 00 00 00 00 00 04
  0x00001fe013ddd170: fc fc fc fc 00 00 00 00

 
Miaubiz found it too, but not faster than ClusterFuzz :):)
Labels: Stability-AddressSanitizer
Comment 3 by mal@google.com, Sep 8 2011
Labels: Stability-CodeYellow
Cc: thestig@chromium.org agl@chromium.org evan@chromium.org
Summary: 4 OOB reads in XMLDocumentParser::doWrite (was: NULL)
Testcase:: [put it in LayoutTests/http/tests/security/min--fuzz-lyt-cross-origin-xsl-BLOCKED1315383275.27.html]
<iframe src="resources/cross-origin-xsl.xml">

xmlParseTryOrFinish /src/third_party/libxml/src/parser.c:11123
xmlParseChunk /src/third_party/libxml/src/parser.c:11617
WebCore::XMLDocumentParser::doWrite(WTF::String const&) 
WebCore::XMLDocumentParser::append(WebCore::SegmentedString const&) 
WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter*, char const*, unsigned long) 
WebCore::DocumentLoader::commitData(char const*, unsigned long) 

Testcase 2: put as LayoutTests/fast/frames/min--fuzz-lyt-set-parent-src-synchronously1315494415.68.xhtml
<html xmlns="http://www.w3.org/1999/xhtml">
<iframe id="parent" src="resources/set-src-to-javascript-url.xhtml"><script> if (window.layoutTestController) { layoutTestController.waitUntilDone(); }</script>

Testcase3 : put as LayoutTests/fast/frames/min--fuzz-lyt-set-parent-src-synchronously1315494415.68.xhtml
<html xmlns="http://www.w3.org/1999/xhtml">
<iframe id="parent" src="resources/set-src-to-javascript-url.xhtml"><script> if (window.layoutTestController) { layoutTestController.waitUntilDone(); }</script>

xmlParseTryOrFinish /src/third_party/libxml/src/parser.c:10921
xmlParseChunk /src/third_party/libxml/src/parser.c:11617
WebCore::XMLDocumentParser::doWrite(WTF::String const&) 
WebCore::XMLDocumentParser::append(WebCore::SegmentedString const&) 
WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter*, char const*, unsigned long) 
WebCore::DocumentLoader::commitData(char const*, unsigned long) 

Testcase 4:  [LayoutTests/fast/xsl/min--fuzz-lyt-dtd-in-source-document1315497357.28.xml]
<?xml version="1.0"?>
<!DOCTYPE xml SYSTEM "resources/dtd-in-source-document.dtd">
<?xml-stylesheet type="text/xsl" href="resources/dtd-in-source-document.xsl"?>

xmlParseTryOrFinish /src/third_party/libxml/src/parser.c:11210
xmlParseChunk /src/third_party/libxml/src/parser.c:11617
WebCore::XMLDocumentParser::doWrite(WTF::String const&) 
WebCore::XMLDocumentParser::append(WebCore::SegmentedString const&) 
WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter*, char const*, unsigned long) 
WebCore::DocumentLoader::commitData(char const*, unsigned long) 

These easily reproduce in ASANified DumpRenderTree too.
Is this  bug 83012 ?
No, this is definitely new. This is OOB read and not a use after free.
Cc: simonjam@chromium.org
Is this still happening? I'm not able to reproduce it with any of these test cases.

Chrome: 100214
WebKit: 94748

Valgrind doesn't see it either.
Are you running under ASAN ? Did you try with ASANified chrome or DumpRenderTree ?
Yeah. I tried both chrome and DumpRenderTree. I built both with ASAN at the same time as base_unittests. When I run the intentionally broken test in base_unittests, ASAN detects it. So I'm pretty sure it's all built correctly.

I copy & pasted your test cases into the correct directories too. I didn't try the one http test case just out of laziness.
Even I cannot reproduce it now. Kostya, Alex was fixing some ASAN bug recently, that might be the reason for this. ClusterFuzz should update itself soon. Lets see if it still reproduces.
Owner: kcc@chromium.org
Ok, so this is reproducing every time on ClusterFuzz, checked manually. ld version there is 2.20.1 vs 2.21.1 on my local ubuntu box. Attaching /proc/cpuinfo for machine spec on ClusterFuzz. Kostya, does this ring any bells ? can this be a tools issues?

Testcase.html (can put anywehre)
<script>
function done() {
}

var x = '';
for (i=0; i<50000; ++i)
  x += '<a>';
var uri = 'data:image/svg+xml,' + x;
var i = new Image();
i.src = uri;
</script> 
tmp
796 bytes View Download
Comment 14 by kcc@chromium.org, Sep 11 2011
This stack is quite popular in crash, so I doubt that asan is being completely  insane. 
See e.g. this one: http://crash/reportdetail?reportid=2a670aa978ae1f5a

I am a bit puzzled by this part: 
  0 bytes to the right of global variable '.str11' (0x7f009eee8aa0) of size 1
Will look on Monday.
Comment 15 by kcc@chromium.org, Sep 11 2011
Note: asan claims this is an OOB in a global object '.str11' of size 1. 
The previous asan versions did not handle constant global objects. 
Valgrind/Memcheck does not handle global objects at all. 
Comment 16 by kcc@chromium.org, Sep 11 2011
I can not reproduce it either. 
inferno@: can you reproduce it on a local machine using a binary built on bot? 
Comment 17 by kcc@chromium.org, Sep 11 2011
No, wait, I do see it with chrome on my local machine using test from #c13
Cc: kcc@chromium.org
Owner: ----
Nice Kostya, so testcase from c#13 reproduces on your z600. for me, it only reproduced consistenly on ClusterFuzz, but not on my z600. Was it consistently crashing for you ? Also, i am thinking if there are multiple OOB reads here because i see different line numbers in different repros of c#4 (although they don't reproduce on my z600 either ) ?
Comment 19 by kcc@chromium.org, Sep 11 2011
This is on my T3500, not z600. 
reproduces reliably on r100473 with chrome, but not with DumpRenderTree
There is a giant switch in the code, all line numbers correspond to different reads from the same pointer (next = ctxt->input->cur[1];)
Thanks a lot Kostya for verifying. Good to finally have a reliable testcase from ClusterFuzz and right now, this is the top crasher on ClusterFuzz, so will see if someone can knock this one out soon.
Comment 21 by kcc@chromium.org, Sep 11 2011
At some point the call stack looks like this (lines in parser.c might be slightly wrong due to my edits): 
#0  0x00007ffff1f63381 in xmlStopParser  at third_party/libxml/src/parser.c:11820
#1  0x00007ffff3591d13 in WebCore::XMLDocumentParser::startElementNs  at third_party/WebKit/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp:827
#2  0x00007ffff1f542c0 in xmlParseStartTag2  at third_party/libxml/src/parser.c:9120
#3  0x00007ffff1f608e2 in xmlParseTryOrFinish at third_party/libxml/src/parser.c:10849
#4  0x00007ffff1f5b449 in xmlParseChunk at third_party/libxml/src/parser.c:11624
#5  0x00007ffff358e922 in WebCore::XMLDocumentParser::doWrite at third_party/WebKit/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp:665
#6  0x00007ffff3587c47 in WebCore::XMLDocumentParser::append at third_party/WebKit/Source/WebCore/xml/parser/XMLDocumentParser.cpp:130
#7  0x00007ffff5cbce37 in WebCore::DecodedDataDocumentParser::appendBytes 

The code in xmlStopParser  at third_party/libxml/src/parser.c:11820 looks like this: 
        ctxt->input->cur = BAD_CAST"";
So, it actually assigns an empty string (i.e. a pointer to a global object of size 1) to ctxt->input->cur

Later on, the parser code reads from this global with offset 1, which is clearly illegal.
10927                   cur = ctxt->input->cur[0];
10928                   next = ctxt->input->cur[1]; <<<<<<<<<<<<

I conclude that asan is sane :) 
Let someone with strong libxml-foo debug this further. 
@inferno: is this a regression? I fixed a very similar issue a long time ago.
@scarybeasts: cant say, because the previous asan versions did not handle constant global objects. With this new functionality, it is hitting instantly on ClusterFuzz.
Owner: cevans@chromium.org
I'll have an initial look...
I think http://trac.webkit.org/changeset/56420 is the fix I did that I was thinking of 18 months ago. Possibly another missing check for a stopped parser.
Status: Started
I'm pretty sure this is a libxml bug. Sounds like it's causing us clusterfuzz pain? I'll crank something out tomorrow.
Yes right, ClusterFuzz is wasting time, hitting this again and again.
Comment 28 by kcc@chromium.org, Sep 12 2011
You can temporary add xmlParseTryOrFinish to the blacklist (may need few other functions too if they access the same pointer)
FWIW: ASAN for the win! valgrind definitely can't touch this.

The following Linux patch will arrange for the OOB read to crash every time (even outside of memory tools):

Index: parser.c
===================================================================
--- parser.c	(revision 100650)
+++ parser.c	(working copy)
@@ -80,6 +80,8 @@
 #include <zlib.h>
 #endif
 
+#include <sys/mman.h>
+
 static void
 xmlFatalErr(xmlParserCtxtPtr ctxt, xmlParserErrors error, const char *info);
 
@@ -11810,7 +11812,11 @@
     ctxt->instate = XML_PARSER_EOF;
     ctxt->disableSAX = 1;
     if (ctxt->input != NULL) {
-	ctxt->input->cur = BAD_CAST"";
+void* ptr = mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
+char* cptr = (char*)ptr;
+mprotect(cptr + 4096, 4096, PROT_NONE);
+cptr[4095] = '\0';
+	ctxt->input->cur = cptr + 4095;
 	ctxt->input->base = ctxt->input->cur;
     }
 }

Project Member Comment 30 by bugdroid1@chromium.org, Sep 13 2011
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=100883

------------------------------------------------------------------------
r100883 | cevans@chromium.org | Tue Sep 13 01:24:32 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/libxml/src/parser.c?r1=100883&r2=100882&pathrev=100883
 M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/libxml/README.chromium?r1=100883&r2=100882&pathrev=100883

Desist libxml from continuing the parse after a SAX callback has stopped the
parse.

BUG= 95465 
TEST=covered by existing tests under ASAN
Review URL: http://codereview.chromium.org/7875008
------------------------------------------------------------------------
Project Member Comment 31 by bugdroid1@chromium.org, Sep 13 2011
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=100884

------------------------------------------------------------------------
r100884 | ukai@chromium.org | Tue Sep 13 01:47:51 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/libxml/src/parser.c?r1=100884&r2=100883&pathrev=100884
 M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/libxml/README.chromium?r1=100884&r2=100883&pathrev=100884

Revert 100883 - Desist libxml from continuing the parse after a SAX callback has stopped the
parse.

BUG= 95465 
TEST=covered by existing tests under ASAN
Review URL: http://codereview.chromium.org/7875008

TBR=cevans@chromium.org
Review URL: http://codereview.chromium.org/7886003
------------------------------------------------------------------------
Project Member Comment 32 by bugdroid1@chromium.org, Sep 13 2011
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=100953

------------------------------------------------------------------------
r100953 | cevans@chromium.org | Tue Sep 13 12:35:03 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/libxml/src/parser.c?r1=100953&r2=100952&pathrev=100953
 M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/libxml/README.chromium?r1=100953&r2=100952&pathrev=100953

Desist libxml from continuing the parse after a SAX callback has stopped the
parse.
Attempt 2 -- now with less compile fail on Mac / Clang.

BUG= 95465 
TBR=cdn
TEST=covered by existing tests under ASAN
Review URL: http://codereview.chromium.org/7892003
------------------------------------------------------------------------
Cc: veill...@gmail.com
Labels: -Restrict-View-SecurityTeam -SecSeverity-Medium -Mstone-14 -Stability-CodeYellow Restrict-View-SecurityNotify SecSeverity-Low Mstone-16
Status: FixUnreleased
I don't think this bug is particularly nasty at all, so we can just let it roll into M16.
Approximately, the cause is:
- libxml calls a SAX handler
- WebKit code calls back into libxml to kill the parse for whatever reason, via xmlStopParser() -- I have looked around and this is documented as legal. xmlStopParser() sets the libxml state to EOF and sets the input buffer to the empty string.
- libxml code, just after the SAX handler exits, tramples over the state machine's "next state" in a couple of places, and also has a precomputed view over the number of input bytes available (which was now just set to 1 by xmlStopParser())

I fixed a few places in libxml to check for EOF state after the SAX handlers exit. I think it's the correct fix, but I'll check with upstream and offer the patch.
Labels: SecImpacts-Stable
Batch update.
Comment 35 by cdn@chromium.org, May 15 2012
Status: Fixed
Marking old security bugs Fixed..
Project Member Comment 36 by bugdroid1@chromium.org, Oct 13 2012
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member Comment 37 by bugdroid1@chromium.org, Mar 10 2013
Labels: -Type-Security -Area-WebKit -SecSeverity-Low -Mstone-16 -Stability-AddressSanitizer -SecImpacts-Stable Cr-Content Security-Severity-Low Performance-Memory-AddressSanitizer Security-Impact-Stable Type-Bug-Security M-16
Project Member Comment 38 by bugdroid1@chromium.org, Mar 13 2013
Labels: Restrict-View-EditIssue
Project Member Comment 39 by bugdroid1@chromium.org, Mar 13 2013
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Labels: -Restrict-View-SecurityNotify -Restrict-View-EditIssue
Project Member Comment 41 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Severity-Low Security_Severity-Low
Project Member Comment 42 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member Comment 43 by bugdroid1@chromium.org, Apr 1 2013
Labels: -Performance-Memory-AddressSanitizer Stability-Memory-AddressSanitizer
Project Member Comment 44 by bugdroid1@chromium.org, Apr 6 2013
Labels: -Cr-Content Cr-Blink
Labels: ClusterFuzz
Project Member Comment 46 by sheriffbot@chromium.org, Oct 1 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 47 by sheriffbot@chromium.org, Oct 2 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: allpublic
Sign in to add a comment