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: Verified
Owner:
Last visit > 30 days ago
Closed: Jun 2010
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Security
M-5

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

Browser crash after AppModalDialogQueue::ShowNextDialog

Reported by aohe...@gmail.com, Jun 21 2010

Issue description

Chromium crashes when several pages simultaneously open a dialog window with javascript alert(). 

To reproduce:
  $ echo > a.html
  $ echo '<script>alert("click");</script>' > b.html
  $ chromium-browser -g *.html *.html *.html *.html *.html *.html

This happens at least in x86 Ubuntu 10.04 with Chromium	6.0.438.0 (Developer Build 49884) Ubuntu and Google Chrome 5.0.375.70 (Official Build 48679). Adding more files increases the reproducibility.

I have not yet tested whether this happens in other platforms, or can be triggered with less files. There is a message about failed GTK assertion before the segfault, so this is probably a Linux-specific issue.

Backtrace from chromium-browser -g 
[...]
[10374:10374:2812236104:ERROR:chrome/app/chrome_dll_main.cc(247)] Gtk: gtk_widget_destroy: assertion `GTK_IS_WIDGET (widget)' failed

Program received signal SIGSEGV, Segmentation fault.
0x000000f9 in ?? ()
(gdb) bt
#0  0x000000f9 in ?? ()
#1  0x082c09a2 in AppModalDialogQueue::ShowNextDialog (this=0xb8c8b70)
    at chrome/browser/app_modal_dialog_queue.cc:18
#2  0x082c039c in AppModalDialog::CompleteDialog (this=0xaaccf40)
    at chrome/browser/app_modal_dialog.cc:44
#3  0x0854b821 in JavaScriptAppModalDialog::OnAccept (this=0xaaccf40, 
    prompt_text=..., suppress_js_messages=false)
    at chrome/browser/js_modal_dialog.cc:106
#4  0x0854bfc5 in JavaScriptAppModalDialog::HandleDialogResponse (
    this=0xaaccf40, dialog=0xb133800, response_id=-5)
    at chrome/browser/js_modal_dialog_gtk.cc:61
#5  0x00963b18 in g_cclosure_marshal_VOID(int0_t) ()
   from /usr/lib/libgobject-2.0.so.0
#6  0x00956252 in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0
#7  0x0096a99d in ?? () from /usr/lib/libgobject-2.0.so.0
#8  0x0096bdb4 in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0
#9  0x0096c256 in g_signal_emit () from /usr/lib/libgobject-2.0.so.0
#10 0x00326b41 in gtk_dialog_response () from /usr/lib/libgtk-x11-2.0.so.0
#11 0x00963dcc in g_cclosure_marshal_VOID__VOID ()
   from /usr/lib/libgobject-2.0.so.0
#12 0x00956252 in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0
#13 0x0096a99d in ?? () from /usr/lib/libgobject-2.0.so.0
#14 0x0096bdb4 in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0
[...]

 

Comment 2 by evan@chromium.org, Jun 21 2010

I think Elliot is the AppModalDialogQueue person, or maybe Tony, or maybe they can help direct further.  :)
Does not crash for me on ubuntu 10.04 (lucid) with chromium 6.0.444.0 (50328), but does crash for v5.0.375.70 beta.

Elliot, Tony, can you please take a look.

Comment 4 by tony@chromium.org, Jun 22 2010

I sent a patch out to fix.  I was able to repro on ToT.  It also seems like it should repro in Win or Mac as well.

Comment 5 by tony@chromium.org, Jun 22 2010

There are lots of crashers on all OSes with AppModalDialogQueue in the stack.  It's not entirely clear to me if they're the same crash or not.
Tony, thanks for fixing this so quickly! It's exciting that the fix may improve stability as well as security.
Can you post links to the code review so we can assess severity?
Also, some links to http://crash/ samples might be interesting.
Labels: -Pri-0 -Area-Undefined Pri-1 Area-UI
Status: Started
Marking Tony as owner since he is fixing it.

Comment 8 by bugdro...@gmail.com, Jun 23 2010

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

------------------------------------------------------------------------
r50561 | tony@chromium.org | 2010-06-22 18:17:35 -0700 (Tue, 22 Jun 2010) | 10 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/app_modal_dialog_gtk.cc?r1=50561&r2=50560
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/app_modal_dialog_win.cc?r1=50561&r2=50560
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/js_modal_dialog.cc?r1=50561&r2=50560

Fix a crash when we try to close a js dialog that wasn't shown.

The dialog has been queued, but it hasn't been shown because a
different dialog is already showing.  We try to close the dialog
because a page navigation has occurred.

BUG= 47056 
TEST=None

Review URL: http://codereview.chromium.org/2803017
------------------------------------------------------------------------

Comment 9 by tony@chromium.org, Jun 23 2010

Status: Fixed
See the codereview link above.

Here are some crash stacks:
http://crash/search?query=product:%22Chrome_Linux%22+AppModalDialogQueue
http://crash/search?query=product:%22Chrome_Mac%22+AppModalDialogQueue
http://crash/search?query=product:%22Chrome%22+AppModalDialogQueue

There are lots of crashes with cookie prompt dialog, which is relatively new, at the top of the stack.  I'm not sure those are the same crash.  I would think we would crash sooner in those cases.

There are also lots of crashes with ExtensionHost near the top of the stack (also looks like a null pointer deref). That also seems unrelated to this fix.

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Status: WillMerge
Given the volume of crashes this is likely a merge candidate.
Labels: SecSeverity-Low
I looked at the fix; it adds a NULL guard. Hence, SecSeverity-Low instead of SecSeverity-Critical had it been a memory corruption :) Phew!
Labels: -Pri-1 Pri-2
Labels: Mstone-5
Status: FixUnreleased
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=51022 

------------------------------------------------------------------------
r51022 | inferno@chromium.org | 2010-06-28 12:41:46 -0700 (Mon, 28 Jun 2010) | 13 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/375/src/chrome/browser/app_modal_dialog_gtk.cc?r1=51022&r2=51021
   M http://src.chromium.org/viewvc/chrome/branches/375/src/chrome/browser/app_modal_dialog_win.cc?r1=51022&r2=51021
   M http://src.chromium.org/viewvc/chrome/branches/375/src/chrome/browser/js_modal_dialog.cc?r1=51022&r2=51021

Merge 50561 - Fix a crash when we try to close a js dialog that wasn't shown.

The dialog has been queued, but it hasn't been shown because a
different dialog is already showing.  We try to close the dialog
because a page navigation has occurred.

BUG= 47056 
TEST=None

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

TBR=tony@chromium.org
Review URL: http://codereview.chromium.org/2827029
------------------------------------------------------------------------

Works fine with Google Chrome 5.0.375.99 (Official Build 51029)

Comment 17 by tony@chromium.org, Jul 1 2010

Status: Verified
Labels: -Restrict-View-SecurityNotify
Was fixed in 5.0.375.99; releasing.
Labels: Type-Security
Labels: SecImpacts-Stable
Batch update.
Project Member

Comment 21 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 22 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Area-UI -SecSeverity-Low -Mstone-5 -Type-Security -SecImpacts-Stable Security-Severity-Low M-5 Security-Impact-Stable Cr-UI Type-Bug-Security
Project Member

Comment 23 by bugdroid1@chromium.org, Mar 13 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Project Member

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

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

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

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

Comment 26 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 27 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