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

Issue 595262 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Security
Nag



Sign in to add a comment

Heap-buffer-overflow in xmlParseEndTag2

Project Member Reported by ClusterFuzz, Mar 16 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5502218635051008

Fuzzer: libfuzzer_libxml_xml_read_memory_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: Heap-buffer-overflow READ 1
Crash Address: 0x6110000099b1
Crash State:
  xmlParseEndTag2
  xmlParseElement
  xmlParseContent
  
Recommended Security Severity: Medium

Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=378984:379001

Minimized Testcase (0.31 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv96pdRS8EGdZDC-KDwgT10LZ0fqmwBJMUIL1WsG9-wzmYHFk-qq-uPA4TrBiOfk_X4zfBummeKBQvbnyTYs-L5HB5pFaYNUmbCr_22ze216JVkOf-47JgNKfSFLzMs-1gH5XQQ1gy-hZBMu9zxMpsgBwtFD5sA
<!DOCTYPE root [
<!ENTITY % defroot '&#60;!ELEMENT <?xml version="1.0"?>
<xsd:schema  
        elementFormDefault="qualified" >  
                <xsd:complexType>  
                        <xsd:sequence>  
--                      </xsd:sequeNce>  
                </complexType' >
%defroot; %defmiddle;%deftest;


Filer: mmoroz

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 

Comment 1 by mmoroz@chromium.org, Mar 16 2016

Cc: dominicc@chromium.org infe...@chromium.org krasin@chromium.org kcc@chromium.org aizatsky@chromium.org ddkil...@apple.com
Components: Blink>XML
Owner: scottmg@chromium.org
fuzz-2-libxml_xml_read_memory_fuzzer (1)
316 bytes View Download
Project Member

Comment 2 by ClusterFuzz, Mar 16 2016

Labels: Pri-1
Status: Assigned (was: Available)

Comment 3 by mea...@chromium.org, Mar 21 2016

Labels: Security_Impact-Head M-50
Project Member

Comment 4 by sheriffbot@chromium.org, Mar 22 2016

Labels: -Security_Impact-Head Security_Impact-Beta
Project Member

Comment 5 by ClusterFuzz, Mar 24 2016

ClusterFuzz has detected this testcase as flaky and is unable to reproduce it in the original crash revision. Skipping fixed testing check and marking it as potentially fixed.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5502218635051008

Fuzzer: libfuzzer_libxml_xml_read_memory_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: Heap-buffer-overflow READ 1
Crash Address: 0x6110000099b1
Crash State:
  xmlParseEndTag2
  xmlParseElement
  xmlParseContent
  
Recommended Security Severity: Medium

Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=378984:379001

Minimized Testcase (0.31 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv96pdRS8EGdZDC-KDwgT10LZ0fqmwBJMUIL1WsG9-wzmYHFk-qq-uPA4TrBiOfk_X4zfBummeKBQvbnyTYs-L5HB5pFaYNUmbCr_22ze216JVkOf-47JgNKfSFLzMs-1gH5XQQ1gy-hZBMu9zxMpsgBwtFD5sA
<!DOCTYPE root [
<!ENTITY % defroot '&#60;!ELEMENT <?xml version="1.0"?>
<xsd:schema  
        elementFormDefault="qualified" >  
                <xsd:complexType>  
                        <xsd:sequence>  
--                      </xsd:sequeNce>  
                </complexType' >
%defroot; %defmiddle;%deftest;


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 6 by ClusterFuzz, Apr 6 2016

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6163240853176320

Fuzzer: libfuzzer_libxml_xml_read_memory_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: Heap-buffer-overflow READ 1
Crash Address: 0x61600000fbe2
Crash State:
  xmlParseEndTag2
  xmlParseElement
  xmlParseContent
  
Recommended Security Severity: Medium

Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=378984:379001

Minimized Testcase (0.74 Kb): https://cluster-fuzz.appspot.com/download/AMIfv95mNuvdxTmAaTZDl2jnUeYD6fpZBFnzgDoUBFBS6S9z4t1hQFwPpKENou1ZfOWQ6XY1H3pYogIAvtqVYU-rvnCIb8wNjfonhKdB21DcfZxso8PzYeyx4P8YC7cIZpW_bhqfrLyslX19LVjKJpd4sI6iRd3Ddw

Filer: mmoroz

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
Project Member

Comment 7 by ClusterFuzz, Apr 7 2016

Labels: Nag
scottmg@: Uh oh! This issue is still open and hasn't been updated in the last 21 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Owner: dominicc@chromium.org
Status: Started (was: Assigned)
Let me take this.
Disclaimer: libxml newbie here.

When looking for end tag names, this does xmlStrncmp(ctxt->input->cur, ctxt->name, tlen) and then dereferences ctxt->input->cur[tlen]. tlen is a tag name length or something.

The assumption that because the strings are equal up to a maximum length of tlen, that the targets string is actually that long is wrong. xmlStrncmp knows to terminate searching the string when it encounters null. So there's an assumption that tlen and strlen(ctxt->name) are in sync or something.

With this repro, the input is "...</result"; cur is pointing at 'r'; name is "result"; tlen is 10 (?) and so cur[tlen] is out of bounds.

I think this tlen comes from bsk:DocPart; need to do some more spelunking as to why this length is getting mixed up with "result".
Cc: pranjal....@gmail.com
Should be reproducible. Please note this is ASan build, not MSan:

$ libfuzzer-linux-release-385134/libxml_xml_read_memory_fuzzer ./595262 
INFO: Seed: 3051653423
libfuzzer-linux-release-385134/libxml_xml_read_memory_fuzzer: Running 1 inputs 1 time(s) each.
=================================================================
==38191==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61600000fbe2 at pc 0x0000007bb6cd bp 0x7ffd2d670d90 sp 0x7ffd2d670d88
READ of size 1 at 0x61600000fbe2 thread T0
    #0 0x7bb6cc in xmlParseEndTag2 third_party/libxml/src/parser.c:9829:13
    #1 0x7acb60 in xmlParseElement third_party/libxml/src/parser.c:10239:2
    #2 0x7a9fef in xmlParseContent third_party/libxml/src/parser.c:10043:6
    #3 0x7ac40e in xmlParseElement third_party/libxml/src/parser.c:10216:5
    #4 0x7c1973 in xmlParseDocument third_party/libxml/src/parser.c:10913:2
    #5 0x7c8937 in xmlDoRead third_party/libxml/src/parser.c:15391:5
    #6 0x4dd281 in LLVMFuzzerTestOneInput testing/libfuzzer/fuzzers/libxml_xml_read_memory_fuzzer.cc:17:18
    #7 0x4ee7c8 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) third_party/libFuzzer/src/FuzzerLoop.cpp:322:13
    #8 0x4ddb06 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*) third_party/libFuzzer/src/FuzzerDriver.cpp:240:3
    #9 0x4e119c in FuzzerDriver third_party/libFuzzer/src/FuzzerDriver.cpp:351:9
    #10 0x4e119c in fuzzer::FuzzerDriver(int, char**, int (*)(unsigned char const*, unsigned long)) third_party/libFuzzer/src/FuzzerDriver.cpp:396
    #11 0x4fdcc0 in main third_party/libFuzzer/src/FuzzerMain.cpp:25:10
    #12 0x7fd47b39cec4 in __libc_start_main /build/eglibc-3GlaMS/eglibc-2.19/csu/libc-start.c:287

<...>
595262
760 bytes View Download
Thanks. I had no problem reproducing this today per comment 9.
Here's a patch, although there may be a better one. It still seems to me that ctxt->name and tlen are getting out of sync.
0001-Fix-memory-access-out-of-bounds-error.patch
840 bytes Download
There is an upstream bug in bugzilla.gnome.org that includes a fix for this.  Let me try to find it.
I think this was reported upstream here:
https://bugzilla.gnome.org/show_bug.cgi?id=758549

Specifically, see this comment:
https://bugzilla.gnome.org/show_bug.cgi?id=758549#c8

And the patch stashed to this bug is similar to yours:
https://bugzilla.gnome.org/show_bug.cgi?id=758588

Send me your bugzilla.gnome.org account and I'll cc you, or Max or Kostya can help.
Thanks ddkilzer! I have made a dominicc@chromium.org Gnome Bugzilla account.
Cc: dan...@veillard.com
Project Member

Comment 19 by sheriffbot@chromium.org, Apr 14 2016

Labels: -Security_Impact-Beta Security_Impact-Stable
Project Member

Comment 20 by bugdroid1@chromium.org, Apr 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/27a27edb43e42023db7d154dcd9f66a500296cc1

commit 27a27edb43e42023db7d154dcd9f66a500296cc1
Author: dominicc <dominicc@chromium.org>
Date: Tue Apr 19 04:46:41 2016

Do not advance beyond the closing tag length in xmlParseEndTag2

BUG= 593690 , 595262 
TEST=libxml_xml_read_memory_fuzzer

Review URL: https://codereview.chromium.org/1873693002

Cr-Commit-Position: refs/heads/master@{#388137}

[modify] https://crrev.com/27a27edb43e42023db7d154dcd9f66a500296cc1/third_party/libxml/README.chromium
[modify] https://crrev.com/27a27edb43e42023db7d154dcd9f66a500296cc1/third_party/libxml/src/parser.c

Status: Fixed (was: Started)
Project Member

Comment 22 by ClusterFuzz, Apr 19 2016

Labels: -Restrict-View-SecurityTeam Merge-Triage M-51 Restrict-View-SecurityNotify
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz
Project Member

Comment 23 by ClusterFuzz, Apr 20 2016

ClusterFuzz has detected this issue as fixed in range 387062:388346.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6163240853176320

Fuzzer: libfuzzer_libxml_xml_read_memory_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: Heap-buffer-overflow READ 1
Crash Address: 0x61600000fbe2
Crash State:
  xmlParseEndTag2
  xmlParseElement
  xmlParseContent
  
Recommended Security Severity: Medium

Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=378984:379001
Fixed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=387062:388346

Minimized Testcase (0.74 Kb): https://cluster-fuzz.appspot.com/download/AMIfv95mNuvdxTmAaTZDl2jnUeYD6fpZBFnzgDoUBFBS6S9z4t1hQFwPpKENou1ZfOWQ6XY1H3pYogIAvtqVYU-rvnCIb8wNjfonhKdB21DcfZxso8PzYeyx4P8YC7cIZpW_bhqfrLyslX19LVjKJpd4sI6iRd3Ddw

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Labels: -Merge-Triage Merge-Request-51

Comment 25 by tin...@google.com, May 9 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)
Please merge your change to M51 branch 2704 ASAP (before 5:00 PM PST today). So we can take it for this week beta release tomorrow. Thank you.
Project Member

Comment 27 by bugdroid1@chromium.org, May 12 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6efc7f88ba163d2a6dcc1cb50fc2a34f42584bd4

commit 6efc7f88ba163d2a6dcc1cb50fc2a34f42584bd4
Author: Dominic Cooney <dominicc@chromium.org>
Date: Thu May 12 00:13:41 2016

Do not advance beyond the closing tag length in xmlParseEndTag2

BUG= 593690 , 595262 
TEST=libxml_xml_read_memory_fuzzer

Review URL: https://codereview.chromium.org/1873693002

Cr-Commit-Position: refs/heads/master@{#388137}
(cherry picked from commit 27a27edb43e42023db7d154dcd9f66a500296cc1)

Review URL: https://codereview.chromium.org/1975593002 .

Cr-Commit-Position: refs/branch-heads/2704@{#511}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/6efc7f88ba163d2a6dcc1cb50fc2a34f42584bd4/third_party/libxml/README.chromium
[modify] https://crrev.com/6efc7f88ba163d2a6dcc1cb50fc2a34f42584bd4/third_party/libxml/src/parser.c

Labels: Release-0-M51
This is now upstream in db07dd61.
Project Member

Comment 30 by sheriffbot@chromium.org, Jul 26 2016

Labels: -Restrict-View-SecurityNotify
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 31 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 32 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