Project: chromium Issues People Development process History Sign in
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 4 users
Status: Fixed
Owner:
Email to this user bounced
Closed: Nov 2010
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Security
M-8

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
Security: possible memory corruption (double-free) in XPath processing code
Reported by yangding...@gmail.com, Nov 17 2010 Back to list
STEPS TO REPRODUCE:

1. Put the attached files (demo.xml and demo.xsl) in the same directory of the web server.
2. Load demo.xml in the browser via an URL like http://localhost/evil/demo.xml.
3. If the renderer doesn't crash immediately, reload the page a few times.

This will result in a sad tab. The crash has been reproduced with the following combinations:

Chrome 7.0.517.44 / Windows XP SP3
Chromium 9.0.584.0 (66236) / Windows XP SP3
Chromium 9.0.584.0 (66239) / Ubuntu 10.04 LTS

VULNERABILITY DETAILS

(Note:
Due to network traffic limitations imposed by my university, it's impractical for me to check out the entire Chromium source tree and debug with it. All of my debugging has been done with libxml2 (2.7.8), so the following analysis should only be regarded as a guidance for locating the actual bug in the version of libxml2 as used by Chrome. However, it's quite possible that the description applies to both versions without much difference.)

It seems the issue exists in function xmlXPathCompOpEvalPositionalPredicate() of xpath.c. Upon encountering input expression '//book[evil()][0]', this function is called to evaluate predicate expression 'evil()'. Since evil() is an illegal XPath function, a further call to xmlXPathCompOpEvalToBoolean() would return with an error, and the engine moves into error handling code. Then comes the interesting part:

(Starting from line 11764 of xpath.c)
    if ((ctxt->error != XPATH_EXPRESSION_OK) || (res == -1)) {
        xmlXPathObjectPtr tmp;
        /* pop the result */
        tmp = valuePop(ctxt);
        xmlXPathReleaseObject(xpctxt, tmp);
        /* then pop off contextObj, which will be freed later */
        valuePop(ctxt);
        goto evaluation_error;
    }

Here an XPath object is popped from the value stack and released by xmlXPathReleaseObject(). Then the control moves to evaluation_error:

(Starting from line 11835 of xpath.c)
evaluation_error:
    xmlXPathNodeSetClear(set, hasNsNodes);
    newContextSize = 0;

evaluation_exit:
    if (contextObj != NULL) {
        if (ctxt->value == contextObj)
        valuePop(ctxt);
        xmlXPathReleaseObject(xpctxt, contextObj);
    }

Here contextObj is not NULL, so xmlXPathReleaseObject() is called again on contextObj, which refers to the same object as the aforementioned block variable tmp in this particular scenario. This will cause the call to xmlFree() by xmlXPathReleaseObject() to operate on an area of already freed memory, leading to memory corruption.

VERSION

Chrome 7.0.517.44 / Windows XP SP3
Chromium 9.0.584.0 (66236) / Windows XP SP3
Chromium 9.0.584.0 (66239) / Ubuntu 10.04 LTS

REPRODUCTION CASE

Please see the attached files (demo.xml and demo.xsl).

 
demo.xml
86 bytes Download
demo.xsl
314 bytes Download
Labels: -Pri-0 Pri-1 SecSeverity-High Mstone-7
Status: Available
This can be easily automated using a wrapper html that loads the xml and reloads the page (attached). It does indeed cause memory corruption and affects all versions from table to latest dev.
repro.html
77 bytes View Download
Labels: -Mstone-7 Mstone-8 ReleaseBlock-Stable
Status: Assigned
I've got this.
Comment 3 by k...@google.com, Nov 17 2010
Build kicks off at 7pm, for stable, FYI.
Comment 5 by bugdro...@gmail.com, Nov 18 2010
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=66567

------------------------------------------------------------------------
r66567 | cevans@chromium.org | Wed Nov 17 17:24:47 PST 2010

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/libxml/src/xpath.c?r1=66567&r2=66566&pathrev=66567
 M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/libxml/README.chromium?r1=66567&r2=66566&pathrev=66567

Fix XPath bug from upstream.

BUG= 63444 
TEST=See bug

Review URL: http://codereview.chromium.org/5196003
------------------------------------------------------------------------
Status: FixUnreleased
Merged to M8.
Labels: reward-topanel
@yangdingning: thanks for letting us know about this libxml bug!

Q1) With what name, if any, would like us to credit you?

Q2) 
Oops! Chopped off the second question:

Q2) This bug report may qualify under the Chromium Security Reward program. Can you confirm who else you shared the details of the bug with?
Comment 9 by bugdro...@gmail.com, Nov 18 2010
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=66581

------------------------------------------------------------------------
r66581 | cevans@chromium.org | Wed Nov 17 18:23:22 PST 2010

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/552/src/third_party/libxml/src/xpath.c?r1=66581&r2=66580&pathrev=66581
 M http://src.chromium.org/viewvc/chrome/branches/552/src/third_party/libxml/README.chromium?r1=66581&r2=66580&pathrev=66581

Merge 66567 - Fix XPath bug from upstream.

BUG= 63444 
TEST=See bug

Review URL: http://codereview.chromium.org/5196003

TBR=cdn@chromium.org
Review URL: http://codereview.chromium.org/5216001
------------------------------------------------------------------------
Quick reaction, really!

> Q1) With what name, if any, would like us to credit you?
Please credit Yang Dingning from NCNIPC, Graduate University of Chinese Academy of Sciences. Thanks!

> Q2) This bug report may qualify under the Chromium Security Reward program. Can you confirm who else you shared the details of the bug with?

This bug is also reported to Gnome's bug tracking database for module libxml2, and Apple via their Apple Bug Reporter, since Safari also has dependency on libxml2. No other party has been contacted.
Just a reminder, Daniel has made a second patch for this issue correcting a tiny problem in the first one. The patch can be found at
http://git.gnome.org/browse/libxml2/commit/?id=fec31bcd452e77c10579467ca87a785b41115de6
Thank you for your kind words and the follow-up comment. I'll pull in the follow-up fix shortly, although the worst that might happen here seems to be a leak in an error path, so it's not urgent :)
Labels: -reward-topanel reward-1000 reward-unpaid
Congratulations! This bug report qualifies for a $1000 Chromium Security Reward
We are rewarding at the higher $1000 level due to various factors:
- The helpfulness of testing on multiple operating systems and versions.
- Nice simple reduced test case.
- Excellent explanation of the bug at a code level.

----
Boilerplate text:
Please do NOT publicly disclose details until a fix has been released to all our
users. Early public disclosure may cancel the provisional reward.
Also, please be considerate about disclosure when the bug affects a core library
that may be used by other products.
Please do NOT share this information with third parties who are not directly
involved in fixing the bug. Doing so may cancel the provisional reward.
Please be honest if you have already disclosed anything publicly or to third parties.
----
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=66868

------------------------------------------------------------------------
r66868 | mal@chromium.org | Fri Nov 19 19:11:40 PST 2010

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/cocoa/content_settings_dialog_controller_unittest.mm?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/host_content_settings_map_unittest.cc?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/host_content_settings_map.cc?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/cocoa/content_settings_dialog_controller.mm?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/about_flags.cc?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/third_party/libxml/src/xpath.c?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/cocoa/content_exceptions_window_controller.h?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/common/chrome_switches.cc?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/renderer/render_view.cc?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/browser_navigator.cc?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/views/options/content_filter_page_view.cc?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/common/chrome_switches.h?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/cocoa/content_setting_bubble_cocoa.mm?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/browser_navigator.h?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/gtk/options/content_filter_page_gtk.cc?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/content_setting_bubble_model_unittest.cc?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/content_setting_bubble_model.cc?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/cocoa/content_settings_dialog_controller.h?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/content_setting_combo_model.cc?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/third_party/libxml/README.chromium?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/app/generated_resources.grd?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/cocoa/content_exceptions_window_controller.mm?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/webkit/glue/plugins/webplugin_delegate_impl_mac.mm?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/app/nibs/ContentSettings.xib?r1=66868&r2=66867&pathrev=66868
 M http://src.chromium.org/viewvc/chrome/branches/552d/src/chrome/browser/renderer_host/browser_render_process_host.cc?r1=66868&r2=66867&pathrev=66868

Reintegrate 552 r66225-r66645.

------
66466 17.11.2010 19:09:22, by bauerb@chromium.org
Tentative compile fix after merge error.

TBR=kerz
-----
66453 17.11.2010 18:17:09, by bauerb@chromium.org
Merge 65953 -- Move click-to-play to about:flags.

XIB changes: Add an outlet |pluginDefaultSettingMatrix_| to ContentSettingsDialogController, hooked up to the associated matrix, to remove the click-to-play radio button.

While I'm at it, clean up a bit:
* Remove the old --disable-click-to-play flag that reverted to the M6 behavior for blocked plugins
* Make ContentExceptionsWindowController use ContentSettingComboModel for the action popup.
* Make HostContentSettingsMapTest use AutoReset to reset command line switches.

BUG= 62091 
TEST=unit_tests

Review URL: http://codereview.chromium.org/4643007
-----
66581 18.11.2010 03:23:22, by cevans@chromium.org
Merge 66567 - Fix XPath bug from upstream.

BUG= 63444 
TEST=See bug

Review URL: http://codereview.chromium.org/5196003

TBR=cdn@chromium.org
Review URL: http://codereview.chromium.org/5216001
-----
66642 18.11.2010 19:14:57, by thakis@chromium.org
Merge 66631 - Mac: Only clear the background of CoreAnimation plugins if we're going to draw into them.

Previously, the logic was:

1.) Clear plugin background
2.) If the plugin didn't update, return early
3.) Paint plugin
4.) "Swap buffers"

But the "Swap buffers" step only unbound and rebound an FBO object If the plugin didn't change, its backing store would contain transparent black, and if the graphics driver decided to flush the FBO for another reason than a "swap buffers" call, the blackness would show up in the browser.

This CL swaps steps 1 and 2, so even if the FBO is flushed for some unrelated reason, we display something valid.

BUG= 60341 
TEST=Open the file attached to the bug. Resize the window for a few minutes, put the computer to sleep and back on, resize window for a few more minutes. The plugin area (the 191x60px rect in the middle) shouldn't become black. YouTube should still work. CPU usage shouldn't be worse than it was before for the browser, plugin, and renderer processes.

Review URL: http://codereview.chromium.org/5220002

TBR=thakis@chromium.org
Review URL: http://codereview.chromium.org/5167003
-----
66645 18.11.2010 19:30:53, by willchan@chromium.org
Merge 66630 - Fix SPDY crash on race when canceling a stream that just got created.

When I fixed the code not to be re-entrant (since that caused crashes) in r61880, I created a window when the pending create callback was posted to the MessageLoop to be run on the next iteration. In this window before it actually gets invoked, if the pending stream creation got cancelled, then the callback wasn't cancelled, so we would execute a callback on a cancelled stream creation, which can cause crashes.

The fix is to keep track of the pending callbacks. Cancellation of pending stream creations check this pending callback map first.

BUG= 63532 
TEST=See bug thread for repro steps. New unit test added as well.

Review URL: http://codereview.chromium.org/5174005

TBR=willchan@chromium.org
Review URL: http://codereview.chromium.org/5216002
-----


Review URL: http://codereview.chromium.org/5238002
------------------------------------------------------------------------
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
@yangdingning: e-mail cevans@chromium.org to get set up for collecting your reward! :)
Labels: -Restrict-View-SecurityNotify
Status: Fixed
Labels: -reward-unpaid
Payment is in the electronic system.
Labels: Type-Security
Labels: SecImpacts-Stable
Batch update.
Cc: happyaro...@gmail.com
CC'ing Debian libxml maintainer.
Project Member Comment 22 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 23 by bugdroid1@chromium.org, Mar 10 2013
Labels: -SecSeverity-High -Mstone-8 -Type-Security -SecImpacts-Stable M-8 Security-Impact-Stable Security-Severity-High Type-Bug-Security
Project Member Comment 24 by bugdroid1@chromium.org, Mar 11 2013
Labels: -Area-Undefined
Project Member Comment 25 by bugdroid1@chromium.org, Mar 13 2013
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Project Member Comment 26 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Severity-High Security_Severity-High
Project Member Comment 27 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member Comment 28 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 29 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