New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 125462 link

Starred by 1 user

Issue metadata

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

Issue description

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
Labels: CVE_description-submitted

Sign in to add a comment