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.
Issue 137541 Reproduceable crash. Changing tabs while a specific text field has focus.
Starred by 2 users Reported by jwroberts@google.com, Jul 16 2012 Back to list
Status: Fixed
Owner:
Email to this user bounced
Closed: Dec 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 0
Type: Bug-Security

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
Chrome Version: 20.0.1132.57
Operating System:
Google Chrome	20.0.1132.57 (Official Build 145807)
OS	Linux
WebKit	536.11 (@122148)
JavaScript	V8 3.10.8.20
Flash	11.3.31.115
User Agent	Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/536.11 (KHTML, like Gecko) Chrome/20.0.1132.57 Safari/536.11

URL (if applicable) where crash occurred:
http://order.comcastteleworker.com/google/

Can you reproduce this crash?
Yes

What steps will reproduce this crash? (or if it's not reproducible, what were you doing just before the crash)?

1. Open two tabs in Chrome. In one tab go to the site listed above.
2. Enter 94089 in the 'Existing ... move to new address form'. Click Check.
3. After the page redirect, give the first textfield on the page the focus (i.e. Current Acct #)
4. Attempt to change tabs by clicking on the 2nd tab header.
5. crash...

Errors from debug log at the time of the crash:

Crash dump id: a145c97649b7be32

     0K                                                       [6520:6520:4982425032:ERROR:gl_context_glx.cc(167)] Couldn't make context current with X drawable.
[6520:6520:4982426192:ERROR:gl_context_glx.cc(167)] Couldn't make context current with X drawable.
[6520:6520:4982432404:ERROR:x11_util.cc(1221)] X Error detected: serial 226, error_code 130 (GLXBadDrawable), request_code 128, minor_code 5 (X_GLXMakeCurrent)
[6520:6520:4982432503:ERROR:x11_util.cc(1221)] X Error detected: serial 228, error_code 8 (BadMatch (invalid parameter attributes)), request_code 128, minor_code 5 (X_GLXMakeCurrent)
[6520:6520:4983705513:ERROR:gl_context_glx.cc(167)] Couldn't make context current with X drawable.
[6520:6520:4983707862:ERROR:gl_context_glx.cc(167)] Couldn't make context current with X drawable.


*Please note that issues filed with no information filled in above will be marked as WontFix*


****DO NOT CHANGE BELOW THIS LINE****
report_id:a145c97649b7be32
 
Comment 1 by cdn@chromium.org, Jul 16 2012
Cc: cdn@chromium.org
Labels: -Type-Bug -Pri-2 -Restrict-View-EditIssue Type-Security Pri-0 Restrict-View-SecurityTeam OS-Linux SecSeverity-Critical Mstone-20 SecImpacts-Stable Area-Internals Feature-TabContents
Status: Available
Comment 2 by cdn@chromium.org, Jul 16 2012
Cc: cevans@chromium.org
Comment 3 by cdn@chromium.org, Jul 16 2012
Can anyone actually debug through this without hitting the gdb hardlock of doom? It reproduces 100% for me. It crashes derefing the vtable of drag_data_ so probably a stale WebContentsView. 
Comment 4 by tsepez@chromium.org, Jul 16 2012
In the historical past, I recall dealing with this nuisance by removing the MODAL option from the code that creates the dialog and recompiling.
Comment 5 by tsepez@chromium.org, Jul 16 2012
Heh.  I just tried removing the modal option in javascript_app_modal_dialog_gtk.cc and your crash goes away.
Comment 6 by cdn@chromium.org, Jul 17 2012
Cc: dpa...@chromium.org
I take back what I said. The actual issue seems to be that the source_tab_index_ of DragData is not set correctly (In this case it is 1 when the vector of DragTabData only has one entry. This triggers an out of bounds read off the end of the vector.

dpapad, it looks like you worked on this code, any chance you can take a look at this. I'll poke at it more this afternoon and might be able to fix it if I can wrap my head around the flow. I suspect the number of tabs is out of sync with this vector somehow but sure sure exactly how this happens.
Comment 7 by cdn@chromium.org, Jul 18 2012
Owner: cdn@chromium.org
Status: Started
Comment 8 by cdn@chromium.org, Jul 18 2012
Cc: sky@chromium.org
+sky for code review
Comment 9 by cdn@chromium.org, Jul 18 2012
Sky, I wanted to respond here to your questions on the code review given that this is a critical security issue. I don't want to give too much information on a public CL.

The constructor for DraggedTabControllerGtk assumes that the TabGtk which it takes as its second argument will be contained in the vector which it takes as its 3rd argument. The only things added to this vector are the TabGtks which are covered in that loop. Previously there was a naming overlap between the tab argument to this function and the tab variable which was scoped to the for loop. 

My assumption was that the author intended to pass the tab variable from the for loop to the contructor but because it had fallen out of scope at that point was accidentally passing the tab argument. Perhaps that was not a correct assumption and this actually intends to pass the tab argument to the constructor.

If that is the case then the correct fix would be to ensure that after the for loop has completed the TabGtk passed as an argument has been added to the vector.

Can you confirm which of these is correct?
Comment 10 by sky@chromium.org, Jul 18 2012
> My assumption was that the author intended to pass the tab variable from the for loop

The intention is to pass in the argument to MaybeStartDrag.

> If that is the case then the correct fix would be to ensure that after the for loop has completed the TabGtk passed as an argument has been added to the vector.

There is a DCHECK to that effect in DraggedTabControllerGtks constructor.
Project Member Comment 11 by bugdroid1@chromium.org, Jul 19 2012
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=147513

------------------------------------------------------------------------
r147513 | cdn@chromium.org | 2012-07-19T19:52:16.183991Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/tabs/tab_gtk.cc?r1=147513&r2=147512&pathrev=147513
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/tabs/tab_strip_gtk.cc?r1=147513&r2=147512&pathrev=147513

Change variable name to avoid scoping issues.
Add an error condition to avoid entering a drag when a tab is being closed.

BUG= 137541 
TEST=N/A

Review URL: https://chromiumcodereview.appspot.com/10797003
------------------------------------------------------------------------
Comment 12 by cdn@chromium.org, Jul 19 2012
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify Merge-Approved
Status: FixUnreleased
Labels: merge-merged-1180
merged to m21 in r148209
Project Member Comment 14 by bugdroid1@chromium.org, Jul 24 2012
Labels: -Merge-Approved
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=148209

------------------------------------------------------------------------
r148209 | inferno@chromium.org | 2012-07-24T21:42:15.492537Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/ui/gtk/tabs/tab_strip_gtk.cc?r1=148209&r2=148208&pathrev=148209
   M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/ui/gtk/tabs/tab_gtk.cc?r1=148209&r2=148208&pathrev=148209

Merge 147513 - Change variable name to avoid scoping issues.
Add an error condition to avoid entering a drag when a tab is being closed.

BUG= 137541 
TEST=N/A

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

TBR=cdn@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10809087
------------------------------------------------------------------------
Labels: -Mstone-20 Mstone-21
Labels: CVE-2012-2859
Project Member Comment 17 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.
Status: Fixed
Project Member Comment 19 by bugdroid1@chromium.org, Jan 18 2013
Labels: Restrict-View-EditIssue
Restrict-View-EditIssue is preferred since it allows anyone who can edit an issue (committers and contributors) to view the bug.
Project Member Comment 20 by bugdroid1@chromium.org, Mar 10 2013
Labels: -Type-Security -SecSeverity-Critical -Mstone-21 -SecImpacts-Stable -Area-Internals -Feature-TabContents Cr-UI-Browser-TabContents Security-Impact-Stable M-21 Cr-Internals Type-Bug-Security Security-Severity-Critical
Project Member Comment 21 by bugdroid1@chromium.org, Mar 14 2013
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Labels: -Restrict-View-SecurityNotify -Restrict-View-EditIssue
Project Member Comment 23 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member Comment 24 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Severity-Critical Security_Severity-Critical
Project Member Comment 25 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 26 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