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 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment
link

Issue 137707: Security: Chrome extensions bug cause crash in all Chrome processes

Reported by nir.mosh...@gmail.com, Jul 17 2012

Issue description

VULNERABILITY DETAILS

Hi,

I found an interesting crash in chrome's extension mechanism - it's related to the XMLHttpRequest object.

This crash is interesting becuase it carshes all chrome processes (!)
This crash its most likely a security-related issue, as it is most likely caused by jumping to an illegal address (SEGFAULT).

You can reproduce the crash by installing the attached chrome extension and execute it.

You can see in the example (attached) chrome crash after:
xhr.open("GET", "http://api.duckduckgo.com/?q=" + search_value + "&format=json", true);

I also added a crash dump.

for more information you can contact me via email:
nmoshe@nds.com
0xlaser@walla.com

Nir.


VERSION
Chrome Version: [20.0.1132.57] + [stable]
Operating System: Windows 7

REPRODUCTION CASE
Please include a demonstration of the security bug, such as an attached.
I added a simple demo (chrome extension) that reproduce the crash: manifest.json + popup.html

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: browser, all chrome processes
Crash State: 
Client ID (if relevant):
 
chrome.dmp
173 KB Download
popup.html
777 bytes View Download
manifest.json
363 bytes View Download

Comment 1 by jsc...@chromium.org, Jul 17 2012

Cc: pkasting@chromium.org sky@chromium.org ben@chromium.org
Labels: -Area-Undefined OS-All Area-UI SecSeverity-Critical SecImpacts-Stable SecImpacts-Beta Mstone-20
Status: Available
This certainly looks ugly. Here's a search for similar crashes:
http://crash.corp.google.com/search?query=crashed_thread_function_name.contains%3A%22ChromeJavaScriptDialogCreator%3A%3ACancelPendingDialogs%22+-crash_address%3A%220%22

Anyone on the CC know where I should direct this for a quick turnaround?

Comment 2 by pkasting@chromium.org, Jul 17 2012

Cc: -pkasting@chromium.org aa@chromium.org jam@chromium.org a...@chromium.org
I looked at one crash stack and it had Extensions on the list, as well as TabContents teardown stuff, so I'm going to replace myself with some people who might know more about those.

Comment 3 by cdn@chromium.org, Jul 18 2012

Owner: cdn@chromium.org
Status: Started

Comment 4 by cdn@chromium.org, Jul 18 2012

Owner: ----
Status: Available
Opps wrong bug

Comment 5 by jsc...@chromium.org, Jul 18 2012

Cc: -ben@chromium.org -sky@chromium.org
Labels: -SecSeverity-Critical SecSeverity-Medium Feature-Extensions
Yep, there are some reports with WebHost, but none of them are exploitable crashes and the stack is different. So, it looks like the security bug is triggered only by ExtensionHost.

@aa - Could you direct this one to someone appropriate?

Comment 6 by aa@chromium.org, Jul 19 2012

Owner: mpcomplete@chromium.org
Status: Assigned
Probably some expectation that WebContents has that we aren't fulfilling.

Comment 8 by a...@chromium.org, Jul 19 2012

This isn't crashing on my Mac, so I can't comment from there, but looking at the backtrace from a sample crash pointed to by Justin:

ChromeJavaScriptDialogCreator::CancelPendingDialogs(content::WebContents *)
ChromeJavaScriptDialogCreator::ResetJavaScriptState(content::WebContents *)
WebContentsImpl::~WebContentsImpl()
WebContentsImpl::`vector deleting destructor'(unsigned int)
ExtensionHost::~ExtensionHost()

ExtensionHost gets destructed, so it kills the WebContents that it owns. The WebContents, in its destructor, calls back to the module providing Javascript dialogs, asking it to clear state. Boom.

I was working with an external contributor in this code, since I did the last big work on Javascript dialogs, and in ExtensionHost I made sure that he made the dialog creator precede the WebContents so it would outlive it. So it's not immediately obvious to me what's wrong.

I'm going to build Chrome on Windows to track this down, though it's been a while and I suspect I won't have answers for a while. If anyone wants to also poke at this, fire any questions about this area of the code to me; it's mine.

Comment 9 by a...@chromium.org, Jul 23 2012

ChromeJavaScriptDialogCreator::CancelPendingDialogs(content::WebContents *) kills open dialogs. Further down on the stack is:

views::View::DoRemoveChildView(views::View *,bool,bool,bool)
views::View::RemoveAllChildViews(bool)
views::internal::RootView::~RootView()

How are dialogs implemented on Views? If they depend on the RootView, then it might make sense that trying to manipulate the currently-open dialog might crash.

I'm failing to reproduce on my Windows build, but I'm still happy to help.

Comment 10 by mpcomplete@chromium.org, Jul 26 2012

Based on my rudimentary reading of the disassembly[1] in WinDbg, it's crashing because the active_dialog of AppModalDialogQueue is non-NULL but garbage. Maybe there is a javascript alert open when the popup is dismissed, and the alert dialog is deleted just before the popup is deleted. I don't know much about AppModalDialogs, so I don't understand why the dialog could be deleted without notifying the queue and removing itself as active.

[1] chrome_1c30000!ChromeJavaScriptDialogCreator::CancelPendingDialogs:
02aeecb6 55              push    ebp
02aeecb7 8bec            mov     ebp,esp
02aeecb9 83ec10          sub     esp,10h
02aeecbc 56              push    esi
02aeecbd 57              push    edi
02aeecbe e8985b52ff      call    chrome_1c30000!AppModalDialogQueue::GetInstance (0201485b)
02aeecc3 8b7d08          mov     edi,dword ptr [ebp+8]
02aeecc6 8bf0            mov     esi,eax
02aeecc8 8b4e20          mov     ecx,dword ptr [esi+20h]
02aeeccb 85c9            test    ecx,ecx
02aeeccd 740a            je      chrome_1c30000!ChromeJavaScriptDialogCreator::CancelPendingDialogs+0x23 (02aeecd9)
02aeeccf 397928          cmp     dword ptr [ecx+28h],edi
02aeecd2 7505            jne     chrome_1c30000!ChromeJavaScriptDialogCreator::CancelPendingDialogs+0x23 (02aeecd9)
02aeecd4 8b01            mov     eax,dword ptr [ecx]
02aeecd6 ff5004          call    dword ptr [eax+4]
02aeecd9 56              push    esi  <<<=== crashes here, or maybe 1 line up

Comment 11 by a...@chromium.org, Jul 26 2012

> Maybe there is a javascript alert open when the popup is dismissed

I'm pretty certain there is.

> I don't understand why the dialog could be deleted without notifying the queue and removing itself as active.

That's why I was asking about the views::View calls on the stack. Are the AppModalDialogs under views implemented in such a way that they would get torn down but leave the pointer dangling?

Comment 12 by infe...@chromium.org, Aug 1 2012

Labels: -Mstone-20 Mstone-21

Comment 13 by infe...@chromium.org, Aug 2 2012

Labels: Security-CodeYellow
Please do read Mark's email titled "Code Yellow: Security Bug Backlog" on chrome-team mailing list.

Comment 14 by infe...@chromium.org, Aug 16 2012

This bug is part of security bug yellow, do you think we will have a fix in time for m22 ?

Comment 15 by mpcomplete@chromium.org, Aug 16 2012

I don't think this will be fixed by M22. But I'll take another look. Last time didn't turn up much insight.

Comment 16 by mpcomplete@chromium.org, Aug 21 2012

OK, I was able to reproduce this on Windows. What's happening is that ExtensionPopup is closing itself directly when it loses focus. This seems to close the alert dialog as well (maybe because it is in the view hierarchy?). The AppModalDialog is deleted as the widget's delegate.

The ExtensionHost still isn't deleted in all this time, and won't be deleted until the ExtensionPopup is deleted. The dialog is cancelled from within ~ExtensionHost, and that's where the boom happens.

Not sure what the right fix is. Maybe we should have ~AppModalDialog cancel itself if it hasn't been already. Or maybe we should delete ExtensionHost earlier. I'll keep looking.

Comment 17 by a...@chromium.org, Aug 21 2012

> What's happening is that ExtensionPopup is closing itself directly when it loses focus. This seems to close the alert dialog as well (maybe because it is in the view hierarchy?).

BTW, this is the key difference between this on the Mac and on Windows. On the Mac, the alert box is independent so it outlives the ExtensionPopup, and then it's around to be torn down when ~ExtensionHost is hit. Your suspicions seem reasonable.

> Maybe we should have ~AppModalDialog cancel itself if it hasn't been already.

That seems like a pretty good idea anyway.

Comment 18 by bugdroid1@chromium.org, Aug 22 2012

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

------------------------------------------------------------------------
r152716 | mpcomplete@chromium.org | 2012-08-22T03:15:56.168890Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/app_modal_dialogs/app_modal_dialog.cc?r1=152716&r2=152715&pathrev=152716
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/app_modal_dialogs/app_modal_dialog.h?r1=152716&r2=152715&pathrev=152716

Fix a Windows crash bug with javascript alerts from extension popups.

BUG= 137707 


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

Comment 19 by infe...@chromium.org, Aug 22 2012

Cc: erikkay@chromium.org
 Issue 144105  has been merged into this issue.

Comment 20 by infe...@chromium.org, Aug 22 2012

Cc: kuz...@gmail.com
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify Merge-Approved
Status: FixUnreleased
Matt, 144105 definitely looks like dup (exactly same stack), but also had a good test. if you get a chance, mind confirming that it is dup.

Comment 21 by mpcomplete@chromium.org, Aug 22 2012

 issue 144105  does seem like a dupe. I can only repro it if I skip step 3, but then it is the same crash.

Comment 22 by infe...@chromium.org, Aug 22 2012

Thanks a lot Matt for confirming. Really appreciate it.

Comment 23 by scarybea...@gmail.com, Sep 5 2012

Labels: reward-topanel

Comment 24 by scarybea...@gmail.com, Sep 5 2012

Labels: -Mstone-21 -Merge-Approved Mstone-22 Merge-Merged

Comment 25 by bugdroid1@chromium.org, Sep 5 2012

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

------------------------------------------------------------------------
r155002 | cevans@chromium.org | 2012-09-05T20:07:18.195781Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1229/src/chrome/browser/ui/app_modal_dialogs/app_modal_dialog.cc?r1=155002&r2=155001&pathrev=155002
   M http://src.chromium.org/viewvc/chrome/branches/1229/src/chrome/browser/ui/app_modal_dialogs/app_modal_dialog.h?r1=155002&r2=155001&pathrev=155002

Merge 152716 - Fix a Windows crash bug with javascript alerts from extension popups.

BUG= 137707 


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

TBR=mpcomplete@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10928004
------------------------------------------------------------------------

Comment 26 by scarybea...@gmail.com, Sep 22 2012

Thank you Nir. With what name would you like to be credited in our release notes? So far I have "Nir Moshe"

Comment 27 by nir.mosh...@gmail.com, Sep 23 2012

"Nir Moshe" is ok :)

Comment 28 by scarybea...@gmail.com, Sep 25 2012

Labels: -reward-topanel reward-500 reward-unpaid
@nmoshe@nds.com: thanks for the credit details!

This also qualifies for a $500 Chromium Security Reward :D Congrats.

----
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.
----

Comment 29 by nir.mosh...@gmail.com, Sep 27 2012

Thanks :)
Will you publish my name in the "Security Hall of Fame"?

Comment 30 by scarybea...@gmail.com, Sep 27 2012

Sure thing! I'll try and do it later.

Comment 31 by scarybea...@gmail.com, Dec 14 2012

Labels: -reward-unpaid
Payment in system.

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

Status: Fixed

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

Project Member
Labels: -Type-Security -Area-UI -SecSeverity-Medium -SecImpacts-Stable -SecImpacts-Beta -Feature-Extensions -Mstone-22 Cr-Platform-Extensions Security-Impact-Stable Security-Impact-Beta Security-Severity-Medium Cr-UI M-22 Type-Bug-Security

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

Labels: -Restrict-View-SecurityNotify

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

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

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

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

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

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

Comment 38 by srsridhar@chromium.org, Jun 20 2014

Labels: hasTestcase

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

Project Member
Labels: -security_impact-beta

Comment 40 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 41 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 42 by mbarbe...@chromium.org, Oct 2 2016

Labels: allpublic

Comment 43 by sheriffbot@chromium.org, Jul 29 2018

Project Member
Labels: -Pri-0 Pri-1

Sign in to add a comment