Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 265889 Heap-use-after-free in WebCore::Document::updateLayout
Starred by 2 users Project Member Reported by clusterf...@chromium.org, Jul 30 2013 Back to list
Status: Duplicate
Merged: issue 313005
Owner:
Closed: Nov 2013
Cc:
Components:
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security
Nag

Blocked on:
issue 315979


Sign in to add a comment
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5068686729674752

Fuzzer: Inferno_twister

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x618000108080
Crash State:
  - crash stack -
  WebCore::DOMWindow::scrollY
  WebCore::DOMWindowV8Internal::pageYOffsetAttrGetterCallback
  - free stack -
  WebCore::FrameView::layout
  WebCore::Document::updateLayout
  


Fully reproducible crash found using linux_tsan_chrome_mp job type.
 
Cc: kcc@chromium.org glider@chromium.org
Labels: Stability-ThreadSanitizer
I will try locally to confirm that it is indeed a one-time crasher in asan, but fully reproducible in tsan.
Cc: dvyukov@chromium.org
+dvyukov FHI
Comment 3 by jln@chromium.org, Aug 1 2013
Cc: dcheng@chromium.org
Daniel, could you take a look or help triage this security issue?
Cc: eseidel@chromium.org japhet@chromium.org
Owner: abarth@chromium.org
Status: Assigned
Adam, any idea why FrameView::layout is blowing away the FrameView from underneath.

WARNING: ThreadSanitizer: heap-use-after-free (pid=24939)
  Read of size 8 at 0x7d5c0000d210 by main thread:
    #0 parent third_party/WebKit/Source/core/platform/Widget.h:95 (chrome+0x00000181af48)
    #1 WebCore::Widget::convertToContainingView(WebCore::IntRect const&) const third_party/WebKit/Source/core/platform/Widget.cpp:156 (chrome+0x00000181af48)
    #2 scrollY third_party/WebKit/Source/core/platform/ScrollView.h:161 (chrome+0x00000284996e)
    #3 WebCore::DOMWindow::scrollY() const third_party/WebKit/Source/core/page/DOMWindow.cpp:1068 (chrome+0x00000284996e)

FREE FRAMES
    #0 operator delete(void*) /usr/local/google/home/thakis/src/chrome/src/third_party/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:539 (chrome+0x000000798d4c)
    #1 WebCore::FrameView::~FrameView() third_party/WebKit/Source/core/page/FrameView.cpp:216 (chrome+0x000002883501)
    #2 deref third_party/WebKit/Source/wtf/RefCounted.h:174 (chrome+0x000002887d16)
    #3 derefIfNotNull<WebCore::FrameView> third_party/WebKit/Source/wtf/PassRefPtr.h:44 (chrome+0x000002887d16)
    #4 ~RefPtr third_party/WebKit/Source/wtf/RefPtr.h:50 (chrome+0x000002887d16)
    #5 ~FontCachePurgePreventer third_party/WebKit/Source/wtf/RefPtr.h:50 (chrome+0x000002887d16)
    #6 WebCore::FrameView::layout(bool) third_party/WebKit/Source/core/page/FrameView.cpp:1110 (chrome+0x000002887d16)
    #7 WebCore::Document::updateLayout() third_party/WebKit/Source/core/dom/Document.cpp:1722 (chrome+0x000001add580)
    #8 WebCore::Document::updateLayoutIgnorePendingStylesheets() third_party/WebKit/Source/core/dom/Document.cpp:1760 (chrome+0x000001add892)
    #9 WebCore::DOMWindow::scrollY() const third_party/WebKit/Source/core/page/DOMWindow.cpp:1066 (chrome+0x00000284995a)
Ah!
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/c
ore/page/FrameView.cpp&q=FrameView::layout&sq=package:chromium&type=cs&l=879

void FrameView::layout(bool allowSubtree)
{
.........
    // Protect the view from being deleted during layout (in recalcStyle)
    RefPtr<FrameView> protector(this);

DOMWindow::scrollY and other functions can't expect frameview to be alive after layout :(
Project Member Comment 6 by clusterf...@chromium.org, Aug 22 2013
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6627178628251648

Fuzzer: Inferno_twister_custom_bundle
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x618000089080
Crash State:
  - crash stack -
  WebCore::DOMWindow::scrollY
  WebCore::DOMWindowV8Internal::pageYOffsetAttrGetterCallback
  - free stack -
  WebCore::WidgetHierarchyUpdatesSuspensionScope::moveWidgets
  WebCore::ContainerNode::removeChild
  
Regressed: https://cluster-fuzz.appspot.com/revisions?range=110902:110925

Minimized Testcase (1.25 Kb): https://cluster-fuzz.appspot.com/download/AMIfv95yypycBta_13RyNPS5W5dJxlWmvlIbKfgiTQHLMOusF7jWF3Dhk84AL--imFpBWje1ytCBoVci_DQAat6qhUpjxE_Yhwo5xefcFECxbyP3p9wjd4J9gnfe9uNMcteNBA5ZelFrKZP3EyQBHYVTL0xR014WBg


Cc: esprehn@chromium.org
Is this a regression?  I shuffled the layout method last week.
Regression range points towards a very old range. Also, we had this stack before, but we now have a fully reliable repro in c#6.
It's somewhat lame the FrameView::layout() does a style resolve.  StyleResolve should be handled prior to layout.

StyleResolve is what can blow away the FrameView (display: none on an iframe/object/embed for instance).  Layout (theoretically) only moves stuff around. :)
Labels: Security_Impact-Stable Security_Impact-Beta M-30
Cc: -eseidel@chromium.org abarth@chromium.org
Owner: eseidel@chromium.org
Eric, Elliot, ideas on how to fix this ?
Labels: -M-30 M-29
Owner: esprehn@chromium.org
It seems we just want those methods to not store the FrameView* on the stack and should check if it is null after calling updateLayoutIgnorePendingStylesheets()
@inferno The minimized test case and the original CF report seem to be about entirely different things. The original bug is in AnimationController and the minimized is the scrollY one
Fix labels.
Project Member Comment 16 by clusterf...@chromium.org, Sep 8 2013
ClusterFuzz has detected this issue as fixed in range 218731:218751.

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

Fuzzer: Inferno_twister_custom_bundle
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x618000089080
Crash State:
  - crash stack -
  WebCore::DOMWindow::scrollY
  WebCore::DOMWindowV8Internal::pageYOffsetAttrGetterCallback
  - free stack -
  WebCore::WidgetHierarchyUpdatesSuspensionScope::moveWidgets
  WebCore::ContainerNode::removeChild
  
Regressed: https://cluster-fuzz.appspot.com/revisions?range=110902:110925
Fixed: https://cluster-fuzz.appspot.com/revisions?range=218731:218751

Minimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95yypycBta_13RyNPS5W5dJxlWmvlIbKfgiTQHLMOusF7jWF3Dhk84AL--imFpBWje1ytCBoVci_DQAat6qhUpjxE_Yhwo5xefcFECxbyP3p9wjd4J9gnfe9uNMcteNBA5ZelFrKZP3EyQBHYVTL0xR014WBg

If you suspect that the result above is incorrect, try re-doing that job on the testcase report page.
c#16 is a false positive. this bug is not fixed.

Reading c#14, Elliot, are you talking about  https://cluster-fuzz.appspot.com/testcase?key=6627178628251648 or c#0 testcase https://cluster-fuzz.appspot.com/testcase?key=5068686729674752. c#0 testcase was flaky. but https://cluster-fuzz.appspot.com/testcase?key=6627178628251648 seems to point to correct stack. 
Project Member Comment 18 by clusterf...@chromium.org, Sep 10 2013
Summary: Heap-use-after-free in WebCore::Document::updateLayout (was: Heap-use-after-free in WebCore::DOMWindow::scrollY)
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5943369637298176

Fuzzer: Ifratric-browserfuzzer-v3
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free WRITE 1
Crash Address: 0x618000070b88
Crash State:
  - crash stack -
  WebCore::Document::updateLayout
  WebCore::DOMWindow::scrollX
  - free stack -
  WebCore::WidgetHierarchyUpdatesSuspensionScope::moveWidgets
  WebCore::ContainerNode::removeChildren
  

Minimized Testcase (0.64 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv94HzxuZpWiIsntDzzVvtM0RM3u6znN_8crChvD5YamFfgWaLxlvZsptDTnCeQc53skSUDeXErzf02h9kVNl7QheN7wEIajP6wKqqPjV7OoVBMqxFh_Yc70IjWKsaiOeL-eGAPFGrvCNrgZAvrflN5nzWI2k6A
<script>
function inituaf() {
  for(var i=0; i<100; i++) {
    for(var j=0; j<32; j++) {
    }
  }
}

function eventhandler2() {

  try { var00220 = document; } catch(err) { } //line 237
  try { var00221 = 0; } catch(err) { } //line 238
  try { var00222 = var00220.getElementsByTagName("iframe")[var00221]; } catch(err) { } //line 239
  try { var00223 = var00222.contentWindow; } catch(err) { } //line 240
  try { var00347 = var00223.scrollX; } catch(err) { } //line 375
  try { var00501 = document; } catch(err) { } //line 560
  try { var00501.open(); } catch(err) { } //line 718
}


</script>
><iframe></iframe>><object onbeforeload="eventhandler2()">

Additional requirements: Requires Interaction Gestures


Cc: ifratric@google.com
Ivan's DOM Fuzzer update turned up a nice repro for this.
Issue 293534 has been merged into this issue.
Cc: cloudfuz...@gmail.com
see also dup in https://code.google.com/p/chromium/issues/detail?id=293534
Project Member Comment 22 by clusterf...@chromium.org, Sep 17 2013
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5564153016090624

Fuzzer: Ifratric-browserfuzzer-v3
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free WRITE 1
Crash Address: 0x61800002cb88
Crash State:
  - crash stack -
  WebCore::Document::updateLayout
  WebCore::DOMWindow::scrollTo
  - free stack -
  WebCore::Frame::setView
  WebCore::FrameLoader::closeAndRemoveChild
  
Regressed: https://cluster-fuzz.appspot.com/revisions?range=221446:221565

Minimized Testcase (0.85 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv94v-UNRErn6rQLWkZsppC9aoCB3_UwBdBqy-uAfGF36Kz-_XxRc6sj5kxl6kdA8OWQI27Ylmi2vn-O-JpBQI7CDegT9LQXl3s7htZdxevuVNtYZ5hPmzucg3tUlWM0Jxvp98z8bg8V7lidYbSkZljJv8GmoGQ
<script>
function inituaf() {
  for(var i=0; i<100; i++) {
    for(var j=0; j<32; j++) {
    }
  }
  try { CollectGarbage(); } catch(err) {
    try { window.gc(); } catch(err) {
      for(var i=0; i<100; i++) {
      }
    }
  }
}

function eventhandler2() {

  try { var00002 = document; } catch(err) { } //line 2
  try { var00003 = var00002; } catch(err) { } //line 3
  try { var00043 = 0; } catch(err) { } //line 45
  try { var00044 = var00003.getElementsByTagName("iframe")[var00043]; } catch(err) { } //line 46
  try { var00045 = var00044.contentWindow; } catch(err) { } //line 47
  try { var00063 = -1; } catch(err) { } //line 67
  try { var00064 = 0; } catch(err) { } //line 68
  try { var00045.scrollTo(var00063,var00064); } catch(err) { } //line 69
  try { var00002.write(); } catch(err) { } //line 185
}


</script>
><object onbeforeload="eventhandler2()"><iframe>


Project Member Comment 23 by clusterf...@chromium.org, Sep 17 2013
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4541482648207360

Fuzzer: Ifratric-browserfuzzer-v3
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free WRITE 1
Crash Address: 0x61800002d388
Crash State:
  - crash stack -
  WebCore::Document::updateLayout
  WebCore::DOMWindow::scrollBy
  - free stack -
  WebCore::WidgetHierarchyUpdatesSuspensionScope::moveWidgets
  WebCore::ContainerNode::removeChildren
  
Regressed: https://cluster-fuzz.appspot.com/revisions?range=221567:221596

Minimized Testcase (1.13 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96E77wzG7oc3-9g7yid9tN8ZBub6ykoGOPI3euCKIQx4ETtpXac4Hupi2BFUVt6oRJqfbtJBvI--1VBKszOMkxDSBj7AO2cNima0LwHiLXbYptEM8sCWtS0ue_yxFJCu8C78zI3NviOxxr7f-bGwjXRCEwPsQ


Project Member Comment 24 by clusterf...@chromium.org, Sep 18 2013
ClusterFuzz has detected this issue as fixed in range 223408:223480.

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

Fuzzer: Ifratric-browserfuzzer-v3
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free WRITE 1
Crash Address: 0x61800002d388
Crash State:
  - crash stack -
  WebCore::Document::updateLayout
  WebCore::DOMWindow::scrollBy
  - free stack -
  WebCore::WidgetHierarchyUpdatesSuspensionScope::moveWidgets
  WebCore::ContainerNode::removeChildren
  
Regressed: https://cluster-fuzz.appspot.com/revisions?range=221567:221596
Fixed: https://cluster-fuzz.appspot.com/revisions?range=223408:223480

Minimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv96E77wzG7oc3-9g7yid9tN8ZBub6ykoGOPI3euCKIQx4ETtpXac4Hupi2BFUVt6oRJqfbtJBvI--1VBKszOMkxDSBj7AO2cNima0LwHiLXbYptEM8sCWtS0ue_yxFJCu8C78zI3NviOxxr7f-bGwjXRCEwPsQ

If you suspect that the result above is incorrect, try re-doing that job on the testcase report page.
Project Member Comment 25 by clusterf...@chromium.org, Sep 19 2013
ClusterFuzz has detected this issue as fixed in range 223300:223309.

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

Fuzzer: Ifratric-browserfuzzer-v3
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free WRITE 1
Crash Address: 0x61800002cb88
Crash State:
  - crash stack -
  WebCore::Document::updateLayout
  WebCore::DOMWindow::scrollTo
  - free stack -
  WebCore::Frame::setView
  WebCore::FrameLoader::closeAndRemoveChild
  
Regressed: https://cluster-fuzz.appspot.com/revisions?range=221446:221565
Fixed: https://cluster-fuzz.appspot.com/revisions?range=223300:223309

Minimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv94v-UNRErn6rQLWkZsppC9aoCB3_UwBdBqy-uAfGF36Kz-_XxRc6sj5kxl6kdA8OWQI27Ylmi2vn-O-JpBQI7CDegT9LQXl3s7htZdxevuVNtYZ5hPmzucg3tUlWM0Jxvp98z8bg8V7lidYbSkZljJv8GmoGQ

If you suspect that the result above is incorrect, try re-doing that job on the testcase report page.
Project Member Comment 26 by clusterf...@chromium.org, Sep 27 2013
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4938643508559872

Fuzzer: Ifratric-browserfuzzer-v3
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free WRITE 1
Crash Address: 0x618000062788
Crash State:
  - crash stack -
  WebCore::Document::updateLayout
  WebCore::Element::scrollIntoView
  - free stack -
  WebCore::Frame::setView
  WebCore::FrameLoader::closeAndRemoveChild
  
Regressed: https://cluster-fuzz.appspot.com/revisions?range=221446:221565

Minimized Testcase (0.90 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96fIyLELbZ0r3ifayDy-BYHcmIUiuzHrFa7erDEJFv3nmvkJJY7_yb65Afm69g4PEp43eepNUqPDBl-5uOqmhhAIY8hZtyXiUO76wiZjCDhxN3fXt0HdvGj7Wb3nhfFYi-z7oJtKJmun9z1oDQFtXNZVr0T-g


Project Member Comment 27 by clusterf...@chromium.org, Sep 27 2013
Labels: Nag
esprehn@: you haven't provided any bug update or come up with a fix for this issue in the last 7 days. Please note that this is a medium+ severity security vulnerability that needs your immediate response. If you have a patch in progress and don't want future nags, please add a codereview link and a WIP label. If the issue is already fixed or you can't reproduce it, please close the bug.
Issue 271157 has been merged into this issue.
Project Member Comment 29 by clusterf...@chromium.org, Oct 1 2013
Labels: -M-29 M-30
Fixing milestone and impact labels.
Project Member Comment 30 by clusterf...@chromium.org, Oct 6 2013
esprehn@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!)

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Project Member Comment 31 by clusterf...@chromium.org, Oct 6 2013
esprehn@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!)

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Project Member Comment 32 by clusterf...@chromium.org, Oct 6 2013
esprehn@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!)

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Sorry for the multiple nag comments in the last 24 hrs. It was supposed to be just one per week :), but a bug in sheriffbot caused it to generate multiple ones. Sorry for the inconvenience.
Project Member Comment 34 by clusterf...@chromium.org, Oct 14 2013
esprehn@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!)

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Owner: eseidel@chromium.org
Elliot has 6 p1s.
Issue 309209 has been merged into this issue.
Cc: attek...@gmail.com
Project Member Comment 38 by clusterf...@chromium.org, Oct 25 2013
eseidel@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!)

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Project Member Comment 39 by clusterf...@chromium.org, Nov 2 2013
eseidel@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!)

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Issue 314471 has been merged into this issue.
Issue 314471 has been merged into this issue.
Elliot, can you please help to look into this (saw your suggestion in #c13). This high severity has been easily fuzzed by external reporters, so it will be great if we can get this fixed. 
Comment 43 Deleted
Reduction:
<script>
function crash() {
  document.getElementsByTagName("iframe")[0].contentWindow.scrollX;
  document.open();
}
</script>
<iframe></iframe><object onbeforeload="crash()">

Nate is considering a larger change to make onbeforeload always asynchronous.  That would solve this entire class of bugs. :)
The alert should not fire twice... but it does.

<script>
function crash() {
  alert("FOO");
  document.getElementsByTagName("iframe")[0].contentWindow.scrollX;
  document.open();
}
</script>
<iframe></iframe><object onbeforeload="crash()">


First dispatch:
#0  WebCore::Node::dispatchBeforeLoadEvent (this=0x210093944048, sourceURL=...)
    at ../../third_party/WebKit/Source/core/dom/Node.cpp:2334
#1  0x0000000001cd0dbf in WebCore::FrameLoader::loadWithNavigationAction (this=0x396c32fbeb98, request=..., 
    action=..., type=WebCore::FrameLoadTypeInitialInChildFrame, formState=..., substituteData=..., 
    clientRedirect=WebCore::NotClientRedirect, overrideEncoding=...)
    at ../../third_party/WebKit/Source/core/loader/FrameLoader.cpp:1305
#2  0x0000000001cd00a2 in WebCore::FrameLoader::load (this=0x396c32fbeb98, passedRequest=...)
    at ../../third_party/WebKit/Source/core/loader/FrameLoader.cpp:728
#3  0x0000000004e7936b in blink::WebFrameImpl::createChildFrame (this=0x396c33294920, request=..., ownerElement=
    0x210093944048) at ../../third_party/WebKit/Source/web/WebFrameImpl.cpp:2213
#4  0x0000000004eea760 in blink::FrameLoaderClientImpl::createFrame (this=0x396c336fa450, url=..., name=..., 
    referrer=..., ownerElement=0x210093944048)
    at ../../third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp:596
#5  0x0000000001a1bf08 in WebCore::HTMLFrameOwnerElement::loadOrRedirectSubframe (this=0x210093944048, url=..., 
    frameName=..., lockBackForwardList=true)
    at ../../third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp:142
#6  0x00000000052f41d0 in WebCore::HTMLFrameElementBase::openURL (this=0x210093944048, lockBackForwardList=true)
    at ../../third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp:93
#7  0x00000000052f47a4 in WebCore::HTMLFrameElementBase::setNameAndOpenURL (this=0x210093944048)
    at ../../third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp:142
#8  0x00000000052f484a in WebCore::HTMLFrameElementBase::didNotifySubtreeInsertionsToDocument (
    this=0x210093944048) at ../../third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp:159
#9  0x0000000004f27d62 in WebCore::ChildNodeInsertionNotifier::notify (this=0x7fffdbc5ebc8, node=...)
    at ../../third_party/WebKit/Source/core/dom/ContainerNodeAlgorithms.h:238
#10 0x0000000004f22a9d in WebCore::ContainerNode::parserAppendChild (this=0x2100939382a0, newChild=...)
    at ../../third_party/WebKit/Source/core/dom/ContainerNode.cpp:656
#11 0x000000000542ab1a in WebCore::insert (task=...)
    at ../../third_party/WebKit/Source/core/html/parser/HTMLConstructionSite.cpp:107
#12 0x000000000542625f in WebCore::executeInsertTask (task=...)
    at ../../third_party/WebKit/Source/core/html/parser/HTMLConstructionSite.cpp:114
#13 0x000000000542611e in WebCore::HTMLConstructionSite::executeTask (this=0x396c337e6938, task=...)
    at ../../third_party/WebKit/Source/core/html/parser/HTMLConstructionSite.cpp:171
#14 0x0000000005427435 in WebCore::HTMLConstructionSite::executeQueuedTasks (this=0x396c337e6938)
    at ../../third_party/WebKit/Source/core/html/parser/HTMLConstructionSite.cpp:309
#15 0x00000000053d3a0a in WebCore::HTMLTreeBuilder::constructTree (this=0x396c337e6920, token=0x7fffdbc5ede8)
    at ../../third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp:391

Second:
#0  WebCore::Node::dispatchBeforeLoadEvent (this=0x210093948048, sourceURL=...)
    at ../../third_party/WebKit/Source/core/dom/Node.cpp:2334
#1  0x00000000019f310e in WebCore::HTMLPlugInElement::dispatchBeforeLoadEvent (this=0x210093948048, sourceURL=...)
    at ../../third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp:232
#2  0x00000000019ee85c in WebCore::HTMLObjectElement::updateWidget (this=0x210093948048, 
    pluginCreationOption=WebCore::CreateOnlyNonNetscapePlugins)
    at ../../third_party/WebKit/Source/core/html/HTMLObjectElement.cpp:305
#3  0x00000000019f2bcd in WebCore::HTMLPlugInElement::updateWidgetIfNecessary (this=0x210093948048)
    at ../../third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp:142
#4  0x00000000019f2afd in WebCore::HTMLPlugInElement::updateWidgetCallback (n=0x210093948048)
    at ../../third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp:130
#5  0x0000000004fd7144 in WebCore::PostAttachCallbacks::SuspendScope::~SuspendScope (this=0x7fffdbc5ec90)
    at ../../third_party/WebKit/Source/core/dom/PostAttachCallbacks.cpp:68
#6  0x0000000004f377fc in WebCore::Document::recalcStyle (this=0x210093934a98, change=WebCore::Force)
    at ../../third_party/WebKit/Source/core/dom/Document.cpp:1727
#7  0x0000000004f33c61 in WebCore::Document::updateStyleIfNeeded (this=0x210093934a98)
    at ../../third_party/WebKit/Source/core/dom/Document.cpp:1750
#8  0x0000000004f43bcf in WebCore::Document::finishedParsing (this=0x210093934a98)
    at ../../third_party/WebKit/Source/core/dom/Document.cpp:4286
#9  0x00000000054287ac in WebCore::HTMLConstructionSite::finishedParsing (this=0x396c337e6938)
    at ../../third_party/WebKit/Source/core/html/parser/HTMLConstructionSite.cpp:530
#10 0x00000000053e0f39 in WebCore::HTMLTreeBuilder::finished (this=0x396c337e6920)
    at ../../third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp:2847
#11 0x00000000053a8cfa in WebCore::HTMLDocumentParser::end (this=0x396c335983a0)
    at ../../third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:759
#12 0x00000000053a4f34 in WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd (this=0x396c335983a0)
    at ../../third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:770
#13 0x00000000053a4ced in WebCore::HTMLDocumentParser::prepareToStopParsing (this=0x396c335983a0)
    at ../../third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:197
#14 0x00000000053a7744 in WebCore::HTMLDocumentParser::processParsedChunkFromBackgroundParser (
    this=0x396c335983a0, popChunk=...)
    at ../../third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:442
#15 0x00000000053a5eae in WebCore::HTMLDocumentParser::pumpPendingSpeculations (this=0x396c335983a0)
    at ../../third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:472
#16 0x00000000053a658a in WebCore::HTMLDocumentParser::didReceiveParsedChunkFromBackgroundParser (
    this=0x396c335983a0, chunk=...) at ../../third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:320
Owner: japhet@chromium.org
Unclear to me why we'd ever dispatch in the second case.  Nate?
Cc: tkent@chromium.org
One dispatch is for the <iframe>, and one is for the <object>, right? Am I misunderstanding the stacks?
Comment 50 by tkent@chromium.org, Nov 10 2013
Blockedon: chromium:315979
Owner: tkent@chromium.org
It looks like tkent fixed the crash, but are we still getting double beforeload calls?  If so, that still makes no sense.  But that can also be broken off into a separate bug.
Eric, Kent - as per comment #51, which changeset was it that fixed the security crash. I can click redo on remaining testcases (we had a ton of repros for this one) to verify this if everything is fixed.
Comment 53 by tkent@chromium.org, Nov 10 2013
infeno, the CL is not landed yet.  https://codereview.chromium.org/61763016/

Comment 54 by tkent@chromium.org, Nov 11 2013
#51, My CL also fixes double beforeload dispatches though I don't know the reason yet :-)

Thanks a lot Kent for working on this. onbeforeload has been a major cause in many external security reports and it is great to make it asynchronous.
Project Member Comment 56 by clusterf...@chromium.org, Nov 13 2013
Labels: -M-30 M-31
Migrating old milestone labels.
Project Member Comment 57 by clusterf...@chromium.org, Nov 13 2013
Labels: Hotlist-Webkit
Project Member Comment 58 by bugdroid1@chromium.org, Nov 15 2013
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=162073

------------------------------------------------------------------------
r162073 | tkent@chromium.org | 2013-11-15T06:03:53.336263Z

Changed paths:
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/beforeload-iframe-crash.html?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/plugins/no-mime-with-valid-extension.html?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-none-blocked.html?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.cpp?r1=162073&r2=162072&pathrev=162073
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/beforeload-input-time-crash-expected.txt?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/plugins/get-user-agent-with-null-npp-from-npp-new.html?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/compositing/plugins/composited-plugin.html?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/plugintypes-notype-data.html?r1=162073&r2=162072&pathrev=162073
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/resources/plugin.js?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/plugins/netscape-plugin-map-data-to-src-expected.txt?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/plugins/mouse-click-plugin-clears-selection.html?r1=162073&r2=162072&pathrev=162073
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/beforeload-assertion-expected.txt?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/plugins/plugin-initiate-popup-window-expected.txt?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/testing/Internals.h?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-none-allowed.html?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/plugins/windowless_plugin_paint_test.html?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/FrameView.h?r1=162073&r2=162072&pathrev=162073
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/beforeload-input-time-crash.html?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/plugintypes-nourl-blocked.html?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/security/contentSecurityPolicy/resources/multiple-iframe-plugin-test.js?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/plugins/netscape-plugin-map-data-to-src.html?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/security/mixedContent/insecure-plugin-in-iframe.html?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/html/HTMLPlugInElement.cpp?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/testing/Internals.idl?r1=162073&r2=162072&pathrev=162073
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/beforeload-assertion.html?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.h?r1=162073&r2=162072&pathrev=162073
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/beforeload-iframe-crash-expected.txt?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/plugins/no-mime-with-valid-extension-expected.txt?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-no-url-blocked.html?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/plugins/get-user-agent-with-null-npp-from-npp-new-expected.txt?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/testing/Internals.cpp?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/compositing/plugins/invalidate_rect-expected.txt?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/FrameView.cpp?r1=162073&r2=162072&pathrev=162073
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/plugins/plugin-initiate-popup-window.html?r1=162073&r2=162072&pathrev=162073

Do not dispatch 'beforeload' event inside FrameView::layout().

Executing JavaScript code inside FrameView::layout() is problematic.
e.g. an assertion failure tested in fast/events/beforeload-assertion.html.
We should avoid it.

This CL  makes 'beforeload' event dispatching for plugins and iframes asynchronous,
except plugin access from JavaScript code (HTMLPlugInElement::
renderWidgetForJSBindings).

BUG= 265889 , 313005 , 315979 
R=esprehn@chromium.org

Review URL: https://codereview.chromium.org/61763016
------------------------------------------------------------------------
Comment 59 by tkent@chromium.org, Nov 15 2013
Status: Fixed
r162073 has some site-compatibility risk.  I don't recommend to merge it to the stable branch.

Project Member Comment 60 by clusterf...@chromium.org, Nov 15 2013
Labels: -M-31
Project Member Comment 61 by clusterf...@chromium.org, Nov 15 2013
Labels: M-32 M-31 Merge-Triage
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 62 by clusterf...@chromium.org, Nov 15 2013
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Mergedinto: 313005
Status: Duplicate
Labels: -Merge-Triage
Project Member Comment 65 by clusterf...@chromium.org, Nov 27 2013
ClusterFuzz has detected this issue as fixed in range 235481:235506.

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

Fuzzer: Ifratric-browserfuzzer-v3
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free WRITE 1
Crash Address: 0x618000062788
Crash State:
  - crash stack -
  WebCore::Document::updateLayout
  WebCore::Element::scrollIntoView
  - free stack -
  WebCore::Frame::setView
  WebCore::FrameLoader::closeAndRemoveChild
  
Regressed: https://cluster-fuzz.appspot.com/revisions?range=221446:221565
Fixed: https://cluster-fuzz.appspot.com/revisions?range=235481:235506

Minimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv96fIyLELbZ0r3ifayDy-BYHcmIUiuzHrFa7erDEJFv3nmvkJJY7_yb65Afm69g4PEp43eepNUqPDBl-5uOqmhhAIY8hZtyXiUO76wiZjCDhxN3fXt0HdvGj7Wb3nhfFYi-z7oJtKJmun9z1oDQFtXNZVr0T-g

If you suspect that the result above is incorrect, try re-doing that job on the testcase report page.
Project Member Comment 66 by clusterf...@chromium.org, Nov 27 2013
ClusterFuzz has detected this issue as fixed in range 235481:235506.

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

Fuzzer: Ifratric-browserfuzzer-v3
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free WRITE 1
Crash Address: 0x618000070b88
Crash State:
  - crash stack -
  WebCore::Document::updateLayout
  WebCore::DOMWindow::scrollX
  - free stack -
  WebCore::WidgetHierarchyUpdatesSuspensionScope::moveWidgets
  WebCore::ContainerNode::removeChildren
  
Fixed: https://cluster-fuzz.appspot.com/revisions?range=235481:235506

Minimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv94HzxuZpWiIsntDzzVvtM0RM3u6znN_8crChvD5YamFfgWaLxlvZsptDTnCeQc53skSUDeXErzf02h9kVNl7QheN7wEIajP6wKqqPjV7OoVBMqxFh_Yc70IjWKsaiOeL-eGAPFGrvCNrgZAvrflN5nzWI2k6A

Additional requirements: Requires Interaction Gestures

If you suspect that the result above is incorrect, try re-doing that job on the testcase report page.
Project Member Comment 67 by clusterf...@chromium.org, Dec 6 2013
ClusterFuzz has detected this issue as fixed in range 239161:239175.

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

Fuzzer: Inferno_twister
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free READ 4
Crash Address: 0x602000020698
Crash State:
  - crash stack -
  WebCore::AnimationController::endAnimationUpdate
  WebCore::ThreadTimers::sharedTimerFiredInternal
  - free stack -
  WebCore::Frame::~Frame
  WebCore::FrameView::~FrameView
  
Regressed: https://cluster-fuzz.appspot.com/revisions?range=138451:138454
Fixed: https://cluster-fuzz.appspot.com/revisions?range=239161:239175

Minimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95PdI-6tmfujqPUX1c5eMW50Y2hP8C7GSUz_JsHUhhLZSYijHLg2bAFmbaV3t5bANocN-fKx_doBnMbjsbC4B6MU9tYM_2MY6q3rBsX5OS8TxE_7OpbIulvnRzPDle24E5eKL0Lm4MnPTATF9_afbZKFrxMsw

If you suspect that the result above is incorrect, try re-doing that job on the testcase report page.
Project Member Comment 68 by clusterf...@chromium.org, Dec 6 2013
ClusterFuzz has detected this issue as fixed in range 239161:239175.

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

Fuzzer: Inferno_twister
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free READ 4
Crash Address: 0x602000020698
Crash State:
  - crash stack -
  WebCore::AnimationController::endAnimationUpdate
  WebCore::ThreadTimers::sharedTimerFiredInternal
  - free stack -
  WebCore::Frame::~Frame
  WebCore::FrameView::~FrameView
  
Regressed: https://cluster-fuzz.appspot.com/revisions?range=138451:138454
Fixed: https://cluster-fuzz.appspot.com/revisions?range=239161:239175

Minimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95PdI-6tmfujqPUX1c5eMW50Y2hP8C7GSUz_JsHUhhLZSYijHLg2bAFmbaV3t5bANocN-fKx_doBnMbjsbC4B6MU9tYM_2MY6q3rBsX5OS8TxE_7OpbIulvnRzPDle24E5eKL0Lm4MnPTATF9_afbZKFrxMsw

If you suspect that the result above is incorrect, try re-doing that job on the testcase report page.
Cc: ddkil...@apple.com
Project Member Comment 70 by clusterf...@chromium.org, Mar 28 2014
Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member Comment 71 by clusterf...@chromium.org, Feb 2 2016
Labels: -Security_Impact-Beta
Project Member Comment 72 by sheriffbot@chromium.org, Oct 1
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 73 by sheriffbot@chromium.org, Oct 2
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