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:
User never visited
Closed: Sep 2011
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

Crashes in PhishingDOMFeatureExtractor::ExtractFeaturesWithTimeout

Project Member Reported by bryner@chromium.org, Sep 19 2011

Issue description

There are crashes showing up on all platforms in PhishingDOMFeatureExtractor::ExtractFeatuesWithTimeout.  An example report is: http://crash/reportdetail?reportid=a7999aeaec8a4348

Chrome_Mac ,  14.0.835.163

Thread 0 *CRASHED* ( EXC_BAD_ACCESS / KERN_INVALID_ADDRESS @ 0xffffffffc0000053 )

0x040b29c3	 [Google Chrome Framework	 - phishing_dom_feature_extractor.cc:157]	safe_browsing::PhishingDOMFeatureExtractor::ExtractFeaturesWithTimeout
0x0413cca4	 [Google Chrome Framework	 - message_loop.cc:104]	TaskClosureAdapter::Run
0x0413d764	 [Google Chrome Framework	 - ../base/callback.h:265]	MessageLoop::RunTask
0x0413f7e1	 [Google Chrome Framework	 - message_loop.cc:502]	MessageLoop::DoWork
0x0411107d	 [Google Chrome Framework	 - message_pump_mac.mm:258]	base::MessagePumpCFRunLoopBase::RunWork
0x90b5d3c4	 [CoreFoundation	 + 0x000733c4]	CFRunLoopRunSpecific
0x90b5daa7	 [CoreFoundation	 + 0x00073aa7]	CFRunLoopRunInMode
0x979452ab	 [HIToolbox	 + 0x000302ab]	RunCurrentEventLoopInMode
0x979450c4	 [HIToolbox	 + 0x000300c4]	ReceiveNextEventCommon
0x97944f38	 [HIToolbox	 + 0x0002ff38]	BlockUntilNextEventMatchingListInMode
0x914a76d4	 [AppKit	 + 0x000406d4]	_DPSNextEvent
0x914a6f87	 [AppKit	 + 0x0003ff87]	-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
0x9149ff9e	 [AppKit	 + 0x00038f9e]	-[NSApplication run]
0x04110f1a	 [Google Chrome Framework	 - message_pump_mac.mm:554]	base::MessagePumpNSApplication::DoRun
0x04110733	 [Google Chrome Framework	 - message_pump_mac.mm:175]	base::MessagePumpCFRunLoopBase::Run
0x0413d613	 [Google Chrome Framework	 - message_loop.cc:451]	MessageLoop::Run
0x05e5c056	 [Google Chrome Framework	 - renderer_main.cc:228]	RendererMain
0x0386e6d8	 [Google Chrome Framework	 - chrome_main.cc:552]	ChromeMain
0x00001f57	 [Google Chrome Helper	 - chrome_exe_main_mac.cc:16]	main
0x00001f15	 [Google Chrome Helper	 + 0x00000f15]	start
0x00000006			

Current guess is that we are accessing a deleted WebFrame through cur_frame_.

 

Comment 1 by kenrb@google.com, Sep 20 2011

Labels: -Type-Bug -Pri-2 Type-Security Pri-1 SecSeverity-High Restrict-View-SecurityTeam
Brian, how is this triggered? I ask because use-after-free conditions are usually security issues if they can be caused by an attacker. In this case this bug should have been filed using the security template for Chromium.

Sorry for locking this down after you opened a public discussion thread about it. :(

It can be opened up again if there is a reason to be confident that this would never be an attacker-controlled condition.

Comment 2 by bryner@chromium.org, Sep 20 2011

It would be triggered through a timing issue.  After the page finishes loading, we start running the phishing classifier on it, which includes DOM traversal.  DOM traversal tries to run for at most 50ms before pausing to allow other events to run on the WebKit thread.  If the frame that is being traversed is removed before traversal continues, we'd access a deleted WebFrame.

That's just my current hypothesis, I haven't tried to reproduce this.  If this is what's happening, then it may be possible to trigger with high probability by creating an iframe with a large DOM and removing it from the document on a setTimeout.  I can work on a test case if you think it's important to have one.

Comment 3 by kenrb@chromium.org, Sep 20 2011

Thanks Brian. It sounds like it is right to leave the security labels on this.

A test case would be useful to verify an eventual fix, no?

Comment 4 by bryner@chromium.org, Sep 22 2011

Cc: mattm@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 23 2011

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

------------------------------------------------------------------------
r102541 | bryner@chromium.org | Fri Sep 23 11:56:35 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/safe_browsing/phishing_dom_feature_extractor.cc?r1=102541&r2=102540&pathrev=102541
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/safe_browsing/phishing_dom_feature_extractor_browsertest.cc?r1=102541&r2=102540&pathrev=102541
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/safe_browsing/phishing_dom_feature_extractor.h?r1=102541&r2=102540&pathrev=102541

Change PhishingDOMFeatureExtractor to cache the WebDocument rather than a WebFrame pointer.

WebFrames are not guaranteed to remain valid across calls to ExtractFeaturesWithTmeout.

BUG= 97148 
TEST=PhishingDOMFeatureExtractorTest.SubframeRemoval


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

Comment 6 by kenrb@chromium.org, Sep 23 2011

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify Mstone-14 Merge-Approved
Status: FixUnreleased
Thanks Brian!


Comment 7 by kenrb@chromium.org, Sep 23 2011

Actually Brian would you mind doing the merge for this, to both M14 and M15?

Comment 8 by bryner@chromium.org, Sep 23 2011

Sure.  I'll probably let this bake in a canary just to make sure there are no unexpected issues.

I assume I don't need any additional branch approvals?

Comment 9 by kenrb@chromium.org, Sep 26 2011

No, it's discretionary at this point based on what you think the risk of breakage is. We try hard to get these kinds of fixes for security bugs into the earliest possible release.
Labels: -Mstone-14 Mstone-15
Labels: -Merge-Approved Merge-Merged merge-merged-874
merged to m15 in r102940
Is this still wanted for M14 as well?  If so, I'll go ahead and merge.
They're most likely won't be another m14. So, you can leave it as is and if we need to cut another m14 we'll pick it up.
Labels: SecImpacts-Stable
Batch update.
Labels: CVE-2011-3884

Comment 17 by cdn@chromium.org, May 15 2012

Status: Fixed
Marking old security bugs Fixed..
Project Member

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

Labels: -Type-Security -Area-Internals -Feature-Safebrowsing -SecSeverity-High -Mstone-15 -SecImpacts-Stable Cr-UI-Browser-SafeBrowsing M-15 Cr-Internals Security-Severity-High Security-Impact-Stable Type-Bug-Security
Project Member

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

Labels: Restrict-View-EditIssue
Project Member

Comment 21 by bugdroid1@chromium.org, Mar 13 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-Severity-High Security_Severity-High
Project Member

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

Labels: -Security-Impact-Stable Security_Impact-Stable
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
Labels: CVE_description-submitted

Sign in to add a comment