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

Issue metadata

Status: Fixed
Owner:
User never visited
Closed: Dec 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Security



Sign in to add a comment
link

Issue 158249: Security: Heap-buffer-underflow in xmlParseAttValueComplex

Reported by aedla@chromium.org, Oct 29 2012 Project Member

Issue description

Code in third_party/libxml/src/parser.c:

    if ((in_space) && (normalize)) {
        while (buf[len - 1] == 0x20) len--;
    }
    buf[len] = 0;

There is no check for len going negative.

VERSION
Chrome Version: 24.0.1311.0 (Developer Build 164569)
Operating System: Ubuntu 12.04 x64

REPRODUCTION CASE
<!DOCTYPE e [<!ATTLIST e a ID #REQUIRED>]>
<e a="&#32; "/>

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: tab
==14082== ERROR: AddressSanitizer heap-buffer-overflow on address 0x7f749f965e3f at pc 0x7f74d1ce857a bp 0x7fffb399b290 sp 0x7fffb399b288
READ of size 1 at 0x7f749f965e3f thread T0
    #0 0x7f74d1ce8579 in xmlParseAttValueComplex /b/build/slave/ASAN_Release__symbolized_/build/third_party/libxml/src/parser.c:3803
    #1 0x7f74d1ce660f in xmlParseAttribute2 /b/build/slave/ASAN_Release__symbolized_/build/third_party/libxml/src/parser.c:8636
    #2 0x7f74d1cd00f2 in xmlParseStartTag2 /b/build/slave/ASAN_Release__symbolized_/build/third_party/libxml/src/parser.c:8794
 
bad.xml
59 bytes View Download

Comment 1 by infe...@chromium.org, Oct 29 2012

Cc: veill...@gmail.com

Comment 2 by veill...@gmail.com, Oct 29 2012

Grumpf ... yes definitely a bug. Doesn't seems to lead to a crash under normal
circumstances, 

  patch is trivial, attached !

What is your schedule w.r.t. making it public, I have a set of parser patches
on top of 2.9.0 and tempted to make a new release soon,

Daniel
158249.patch
407 bytes View Download

Comment 3 by infe...@chromium.org, Oct 29 2012

Labels: reward-topanel
Status: Available
So, we have m23 stable release coming up in a week or two. I do have a merge window to merge this. I can try doing it tmrw or letting Chris do it. You can commit to libxml trunk anytime, it will be great if you can provide me the changeset link here. Can you wait a week or two before formal announcement ?

Comment 4 by infe...@chromium.org, Oct 29 2012

Cc: kareng@google.com

Comment 5 by veill...@gmail.com, Oct 29 2012

Okidoc, here is the public commit:

http://git.gnome.org/browse/libxml2/commit/?id=6a36fbe3b3e001a8a840b5c1fdd81cefc9947f0d

no announcement of any sort expected from my side, at least until 2.9.1 release,

Daniel

Comment 6 by scarybea...@gmail.com, Oct 29 2012

Labels: -reward-topanel
Removing reward-topanel :P

Comment 7 by scarybea...@gmail.com, Oct 29 2012

Owner: aedla@chromium.org
Status: Assigned
@aedla: couple of things to look at!

1) Learning about security bug labels. Have a look at:
http://dev.chromium.org/Home/chromium-security/security-bug-lifecycle
And then try and add some labels. Come over if you want help with any.
When we file bugs ourselves, we can add labels right away.

2) Fixing this! If you concur with Daniel's git commit then it's time for you to apply it (and adjust the README) as a Chromium patch! I will review it. Let me know if you want help with the process.

Comment 8 by aedla@chromium.org, Oct 29 2012

Labels: -Area-Undefined Area-Internals Mstone-23 SecSeverity-High
@scarybeasts: thanks for the tips!

Comment 9 by aedla@chromium.org, Oct 30 2012

Comment 10 by bugdroid1@chromium.org, Oct 30 2012

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=164867

------------------------------------------------------------------------
r164867 | aedla@chromium.org | 2012-10-30T06:37:56.085053Z

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

Add a check to prevent len from going negative in xmlParseAttValueComplex.

BUG= 158249 


Review URL: https://chromiumcodereview.appspot.com/11343029
------------------------------------------------------------------------

Comment 11 by aedla@chromium.org, Oct 30 2012

Labels: Merge-Approved SecImpacts-Stable SecImpacts-Beta
Status: FixUnreleased

Comment 12 by scarybea...@gmail.com, Nov 12 2012

Labels: -Restrict-View-SecurityTeam -Merge-Approved Restrict-View-SecurityNotify Merge-Merged

Comment 13 by bugdroid1@chromium.org, Nov 12 2012

Project Member
Labels: merge-merged-1271
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=167215

------------------------------------------------------------------------
r167215 | cevans@chromium.org | 2012-11-12T18:41:52.958049Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1271/src/third_party/libxml/README.chromium?r1=167215&r2=167214&pathrev=167215
   M http://src.chromium.org/viewvc/chrome/branches/1271/src/third_party/libxml/src/parser.c?r1=167215&r2=167214&pathrev=167215

Merge 164867 - Add a check to prevent len from going negative in xmlParseAttValueComplex.

BUG= 158249 


Review URL: https://chromiumcodereview.appspot.com/11343029

TBR=aedla@chromium.org
Review URL: https://codereview.chromium.org/11410039
------------------------------------------------------------------------

Comment 14 by bugdroid1@chromium.org, Nov 12 2012

Project Member
Labels: merge-merged-1312
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=167219

------------------------------------------------------------------------
r167219 | cevans@chromium.org | 2012-11-12T19:13:53.001408Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/third_party/libxml/README.chromium?r1=167219&r2=167218&pathrev=167219
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/third_party/libxml/src/parser.c?r1=167219&r2=167218&pathrev=167219

Merge 164867 - Add a check to prevent len from going negative in xmlParseAttValueComplex.

BUG= 158249 


Review URL: https://chromiumcodereview.appspot.com/11343029

TBR=aedla@chromium.org
Review URL: https://codereview.chromium.org/11365207
------------------------------------------------------------------------

Comment 15 by jsc...@chromium.org, Dec 20 2012

Status: Fixed

Comment 16 by bugdroid1@chromium.org, Mar 10 2013

Project Member
Labels: -Type-Security -Area-Internals -Mstone-23 -SecSeverity-High -SecImpacts-Stable -SecImpacts-Beta Security-Impact-Stable Security-Impact-Beta Cr-Internals M-23 Security-Severity-High Type-Bug-Security

Comment 17 by scarybea...@gmail.com, Mar 21 2013

Labels: -Restrict-View-SecurityNotify

Comment 18 by bugdroid1@chromium.org, Mar 21 2013

Project Member
Labels: -Security-Severity-High Security_Severity-High

Comment 19 by bugdroid1@chromium.org, Mar 21 2013

Project Member
Labels: -Security-Impact-Stable Security_Impact-Stable

Comment 20 by bugdroid1@chromium.org, Mar 21 2013

Project Member
Labels: -Security-Impact-Beta Security_Impact-Beta

Comment 21 by sheriffbot@chromium.org, Jun 14 2016

Project Member
Labels: -security_impact-beta

Comment 22 by sheriffbot@chromium.org, Oct 1 2016

Project Member
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

Comment 23 by sheriffbot@chromium.org, Oct 2 2016

Project Member
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

Comment 24 by mbarbe...@chromium.org, Oct 2 2016

Labels: allpublic

Sign in to add a comment