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 1 user
Status: Fixed
Owner:
Email to this user bounced
Closed: May 2012
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
Security: libxml2 1-byte heap-buffer-overflow in xmlXPtrEvalXPtrPart
Reported by a...@ut.ee, Apr 28 2012 Back to list
VULNERABILITY DETAILS

Here is some code from xmlXPtrEvalXPtrPart at xpointer.c:

    len = xmlStrlen(ctxt->cur);
    len++;
    buffer = (xmlChar *) xmlMallocAtomic(len * sizeof (xmlChar));
    if (buffer == NULL) {
        xmlXPtrErrMemory("allocating buffer");
	return;
    }

    cur = buffer;
    while (CUR != 0) {
	if (CUR == ')') {
	    level--;
	    if (level == 0) {
		NEXT;
		break;
	    }
	    *cur++ = CUR;
	} else if (CUR == '(') {
	    level++;
	    *cur++ = CUR;
	} else if (CUR == '^') {
	    NEXT;
	    if ((CUR == ')') || (CUR == '(') || (CUR == '^')) {
		*cur++ = CUR;
	    } else {
		*cur++ = '^';
		*cur++ = CUR;
	    }
	} else {
	    *cur++ = CUR;
	}
	NEXT;
    }
    printf("before=%08x\n", *(unsigned *)cur);
    *cur = 0;
    printf("after=%08x\n", *(unsigned *)cur);

(I added the two printf's for debugging.)

Overflow occurs when ctxt->cur string ends with '^'. For example if ctxt->cur is "aaaaaaaaaaaaaa^", then:
 1. len = 15 + 1 = 16
 2. 16 byte buffer is allocated
 3. 14 'a' chars are copied
 4. '^' and '\x00' are copied together
 5. *cur = 0 will overflow

With tcmalloc, if the 16-byte block after buffer is not in use, its first bytes are a pointer to the next free block.
Low byte of this pointer is overwritten to 0x00.

VERSION
Chrome Version: 18.0.969.0 (Developer Build 113953 Linux)
Operating System: Ubuntu 10.04.3 LTS, i686

REPRODUCTION CASE
Open http://www.ut.ee/~asd/xslt/bad.xml

I usually also have to force chrome to use more heap by moving mouse or clicking. It might take some tries.

It prints out:

before=b95bee20
after=b95bee00
third_party/tcmalloc/chromium/src/free_list.cc:115] Memory corruption detected.

So a pointer to next free block is corrupted and tcmalloc detects the corruption. I don't think it would be too difficult to corrupt arbitrary memory, but I'm not sure, haven't tried.

 
Thanks for the report. And thanks for the code analysis -- seems like you're 90% of the way to a fix?

Do you fancy taking a go at providing a Chromium patch? It would require learning how to contribute patches to Chromium (e.g. http://dev.chromium.org/developers/contributing-code), but could be worthwhile: for simple fixes to security bugs, we typically top-up any reward with a +$500 bonus.

An example of a previous Chromium patch to libxml, including link to codereview, is here:
http://src.chromium.org/viewvc/chrome?view=rev&revision=95382
Comment 2 by a...@ut.ee, Apr 29 2012
Sure, I'll give it a try.
Comment 3 by a...@ut.ee, Apr 30 2012
Ok, I created a patch and uploaded it for review. Chris, I currently set you as a reviewer, should I add someone else as a reviewer instead?

I may not get to it immediately but I'd be delighted to be the reviewer.
Labels: SecSeverity-High reward-topanel SecImpacts-Stable SecImpacts-Beta Mstone-19
Owner: cevans@chromium.org
Status: Started
Confirmed on 20.0.1115.1 dev

(crash ID af7f42845f5ec1ae)

https://chromiumcodereview.appspot.com/10263014/ from Juri.
Cc: veill...@gmail.com
Comment 7 by cdn@chromium.org, May 2 2012
Labels: OS-All
Comment 8 by veill...@gmail.com, May 3 2012
To be honnest I looked at the patch but didn't took yet the time to investigate.
A priori XPointer is not hooked to any default handling and that wasn't looking
urgent (that module is fairly rought for historical reasons). Is there
a reproducer
by chance ? since it seems a clang report I'm afraid there isn't one,
but I'm asking
anyway :-)

Daniel
@veillard: the test case hosted at http://www.ut.ee/~asd/xslt/bad.xml seems to crash Chrome for me.

The magic piece in the linked XSL file is:

<xsl:value-of select="document('doc.xml#name(aaaaaaaaaaaaaa%5E')"/>

When you say that xpointer is "not hooked to any default handling", perhaps there's an unexpected entry patch via the document() function?

Ah, I had forgotten we hooked to XPointer in libxslt for document() ...

Okay, I see, I could reproduce this with valgrind using xsltproc. Weirdly
I can't reproduce it directly at the libxml2 level with
  valgrind ./testXPath -i doc.xml --xptr "name(aaaaaaaaaaaaaa^')"
though it really goes through the same code path.

Patch looks correct to me, might be a good idea to send it upstream
once you feel it is safe to send out,

Daniel
Feel free to commit upstream, I'll commit to Chromium today.
Project Member Comment 12 by bugdroid1@chromium.org, May 3 2012
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=135174

------------------------------------------------------------------------
r135174 | cevans@chromium.org | Thu May 03 10:32:37 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/libxml/src/xpointer.c?r1=135174&r2=135173&pathrev=135174
 M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/libxml/README.chromium?r1=135174&r2=135173&pathrev=135174

Fix XPointer bug.

BUG= 125462 
AUTHOR=asd@ut.ee
R=cevans@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10344022
------------------------------------------------------------------------
Labels: -Restrict-View-SecurityTeam -Pri-0 -Area-Undefined Restrict-View-SecurityNotify Pri-1 Area-Internals Merge-Approved
Status: FixUnreleased
Labels: -Merge-Approved Merge-Merged
M19: r135380
Project Member Comment 15 by bugdroid1@chromium.org, May 4 2012
Labels: merge-merged-1084
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=135380

------------------------------------------------------------------------
r135380 | cevans@chromium.org | Fri May 04 11:33:54 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1084/src/third_party/libxml/src/xpointer.c?r1=135380&r2=135379&pathrev=135380
 M http://src.chromium.org/viewvc/chrome/branches/1084/src/third_party/libxml/README.chromium?r1=135380&r2=135379&pathrev=135380

Merge 135174 - Fix XPointer bug.

BUG= 125462 
AUTHOR=asd@ut.ee
R=cevans@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10344022

TBR=cevans@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10383012
------------------------------------------------------------------------
Labels: -reward-topanel reward-1500 reward-unpaid
Hi Juri!

Definitely reward-worthy here!
$1000 for the great find and high quality report. With $500 bonus for an accepted fix.

$1500 total
Comment 17 by a...@ut.ee, May 5 2012
Thank you :)
Labels: -reward-unpaid
Payment in system.
Labels: CVE-2011-3102
Comment 21 by cdn@chromium.org, May 15 2012
Status: Fixed
Updating status to Fixed on security bugs which were fixed when m19 went to stable.
Cc: happyaro...@gmail.com
CC'ing Debian libxml maintainer.
Project Member Comment 23 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 24 by bugdroid1@chromium.org, Mar 10 2013
Labels: -Type-Security -Area-Internals -SecSeverity-High -SecImpacts-Stable -SecImpacts-Beta -Mstone-19 M-19 Security-Impact-Stable Security-Impact-Beta Cr-Internals Security-Severity-High Type-Bug-Security
Project Member Comment 25 by bugdroid1@chromium.org, Mar 13 2013
Labels: Restrict-View-EditIssue
Project Member Comment 26 by bugdroid1@chromium.org, Mar 13 2013
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Labels: -Restrict-View-SecurityNotify -Restrict-View-EditIssue
Project Member Comment 28 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Severity-High Security_Severity-High
Project Member Comment 29 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member Comment 30 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Impact-Beta Security_Impact-Beta
Project Member Comment 31 by sheriffbot@chromium.org, Jun 14 2016
Labels: -security_impact-beta
Project Member Comment 32 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 33 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