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

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2014
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment

Heap-use-after-free in blink::EventHandlerRegistry::documentDetached

Project Member Reported by ClusterFuzz, Aug 26 2014

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6680344293015552

Fuzzer: Inferno_twister
Job Type: Windows_asan_chrome

Crash Type: Heap-use-after-free READ 4
Crash Address: 0x03326190
Crash State:
  blink::EventHandlerRegistry::documentDetached
  blink::Document::detach
  blink::Document::prepareForDestruction
  

Minimized Testcase (1.36 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97hDdnMzwCYIz18WFl3bxpLB6ckAESdYS7TZlKlij5cvAPQJbNEAKGqgy7LE8n3LDf3JLaDHZ8ZMGBZB16OlTXBR1ZMA9zLBSE0uO2ku2agN5CczFAUVeGm02BxYFdkqCyFWE5Gi4huC-nO58ImGyHIbU7C9g

Additional requirements: Requires HTTP

Filer: inferno
 
Owner: skyos...@chromium.org
Status: Assigned
Project Member

Comment 2 by ClusterFuzz, Aug 26 2014

Labels: Pri-1 M-37
Author: skyostil@chromium.org
Component: blink
Changelist: https://chromium.googlesource.com/chromium/blink.git/+/4645dec9b19f9021c835f02b659acd2223789958
The CL last changed line 232 of file EventHandlerRegistry.cpp, which is stack frame 0.
Cc: abarth@chromium.org rbyers@chromium.org
Please take a look or revert your change.
Status: Started
Looking...
DOM is hard :P Fix here: https://codereview.chromium.org/519003003/
Cc: tkent@chromium.org kochi@chromium.org hayato@chromium.org
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 3 2014

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

------------------------------------------------------------------
r181302 | chrome-bot@google.com | 2014-09-03T09:50:06.705291Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/TreeScopeAdopter.cpp?r1=181302&r2=181301&pathrev=181302
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/event-listener-on-attribute-inside-shadow-dom-expected.txt?r1=181302&r2=181301&pathrev=181302
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/event-listener-on-attribute-inside-shadow-dom.html?r1=181302&r2=181301&pathrev=181302

Tell synthetic attribute nodes inside shadow DOM about move to new Document

Node::didMoveToNewDocument() is called to notify a Node that it has
been moved under a new Document. This patch fixes a bug in
TreeScopeAdopter where this notification was not done for synthetic
attribute nodes inside a shadow DOM. The bug caused an issue where an
event handler registered for an attribute node was not correctly
unregistered when the parent element was moved to a new document.

BUG= 407477 

Review URL: https://codereview.chromium.org/519003003
-----------------------------------------------------------------
Status: Fixed
Project Member

Comment 10 by ClusterFuzz, Sep 3 2014

Labels: -Restrict-View-SecurityTeam Merge-Triage M-38 Restrict-View-SecurityNotify
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz
Project Member

Comment 11 by ClusterFuzz, Sep 4 2014

ClusterFuzz has detected this issue as fixed in range 293090:293176.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6680344293015552

Fuzzer: Inferno_twister
Job Type: Windows_asan_chrome

Crash Type: Heap-use-after-free READ 4
Crash Address: 0x03326190
Crash State:
  blink::EventHandlerRegistry::documentDetached
  blink::Document::detach
  blink::Document::prepareForDestruction
  
Fixed: https://cluster-fuzz.appspot.com/revisions?job=windows_asan_chrome&range=293090:293176

Minimized Testcase (1.36 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97hDdnMzwCYIz18WFl3bxpLB6ckAESdYS7TZlKlij5cvAPQJbNEAKGqgy7LE8n3LDf3JLaDHZ8ZMGBZB16OlTXBR1ZMA9zLBSE0uO2ku2agN5CczFAUVeGm02BxYFdkqCyFWE5Gi4huC-nO58ImGyHIbU7C9g

Additional requirements: Requires HTTP

If you suspect that the result above is incorrect, try re-doing that job on the testcase report page.

Labels: -M-37 -Merge-Triage Merge-Requested
Matthew - Merge Requested for M38 (Branch 2125)
Labels: -Merge-Requested Merge-Approved
skyostil@ - please merge to M38 / branch 2125
Project Member

Comment 15 by bugdroid1@chromium.org, Sep 26 2014

Labels: -Merge-Approved merge-merged-2125
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=182750

------------------------------------------------------------------
r182750 | skyostil@chromium.org | 2014-09-26T10:33:32.168039Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/branches/chromium/2125/Source/core/dom/TreeScopeAdopter.cpp?r1=182750&r2=182749&pathrev=182750
   A http://src.chromium.org/viewvc/blink/branches/chromium/2125/LayoutTests/fast/events/event-listener-on-attribute-inside-shadow-dom-expected.txt?r1=182750&r2=182749&pathrev=182750
   A http://src.chromium.org/viewvc/blink/branches/chromium/2125/LayoutTests/fast/events/event-listener-on-attribute-inside-shadow-dom.html?r1=182750&r2=182749&pathrev=182750

Merge 181302 "Tell synthetic attribute nodes inside shadow DOM a..."

> Tell synthetic attribute nodes inside shadow DOM about move to new Document
> 
> Node::didMoveToNewDocument() is called to notify a Node that it has
> been moved under a new Document. This patch fixes a bug in
> TreeScopeAdopter where this notification was not done for synthetic
> attribute nodes inside a shadow DOM. The bug caused an issue where an
> event handler registered for an attribute node was not correctly
> unregistered when the parent element was moved to a new document.
> 
> BUG= 407477 
> 
> Review URL: https://codereview.chromium.org/519003003

TBR=skyostil@chromium.org

Review URL: https://codereview.chromium.org/599733005
-----------------------------------------------------------------
Labels: Release-0-M38
Project Member

Comment 17 by ClusterFuzz, Dec 10 2014

Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member

Comment 18 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 19 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