!node || (node->isElementNode()) |
||||||
Issue descriptionDetailed report: https://cluster-fuzz.appspot.com/testcase?key=6595391946752000 Fuzzer: bj_broddelwerk Job Type: linux_debug_chrome Platform Id: linux Crash Type: ASSERT Crash Address: Crash State: !node || (node->isElementNode()) blink::toElement blink::HTMLStackItem::element Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_debug_chrome&range=268656:269696 Minimized Testcase (0.97 Kb): https://cluster-fuzz.appspot.com/download/AMIfv95tC1oCT262LMx-ywEiHC4jeKwa8Wv9_MK3w1_CdLXKgmr0t3EL7miGcTfb7ivI7QBqpd95jK5NnmGftQz3Rd_tTALmg6HDjMo9mQQtBzCtOesPVSUjBP55scybrYPyCGUrYGGuPPKS7rvxXwp8lvTe71comQ?testcase_id=6595391946752000 Issue manually filed by: mmohammad See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
,
Aug 9 2016
,
Aug 11 2016
Here is a simplified reproduction that works on ToT debug:
<script>
element = document.getElementsByTagName("*")[0];
element.insertAdjacentHTML('afterbegin', '<tr><frameset></frameset></tr>');
</script>
,
Aug 11 2016
Loading triager here, assigning to csharrison@
,
Aug 11 2016
Looks like we should be early returning at line 615 (my best guess). I'll run this through a debugger and see what the element stack looks like.
,
Aug 11 2016
Open element stack looks like: [m_headElement, m_bodyElement] So, it looks like there's an error in parsing where we forget to pop the head element.
,
Aug 11 2016
NVM that analysis is wrong, we don't have an active open head element. Looks like we have [(null), body element], where the (null) is what triggers the assert.
,
Aug 11 2016
Yeah looks like we don't go down the isParsingFragmentOrTemplateContents() path and crash when we take openElements()->top() which is not an element (it's a DocumentFragment). I think this is against the spec, which says we should always make the root an html element, but I'm not familiar enough with this code. "In the fragment case, the stack of open elements is initialised to contain an html element that is created as part of that algorithm. (The fragment case skips the "before html" insertion mode.)" dominicc, kouhei, any ideas here?
,
Aug 11 2016
Ah: looks like we do skip this step for efficiency and use the fragment as root node here: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp?rcl=1470626560&l=291 And the rabbit hole grows deeper...
,
Aug 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/681c95bfedb5ee83897f8a0d4104c91049ba16f6 commit 681c95bfedb5ee83897f8a0d4104c91049ba16f6 Author: csharrison <csharrison@chromium.org> Date: Mon Aug 15 07:08:44 2016 Fix frameset within fragment asserts in HTMLTreeBuilder The HTMLTreeBuilder does not expect a frameset within a fragment in the "in body" insertion mode. It seems like this is not specifically disallowed in the spec, event though there are other cases where the algorithm aborts here due to parsing a fragment. https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inbody BUG= 636057 Review-Url: https://codereview.chromium.org/2242703002 Cr-Commit-Position: refs/heads/master@{#411945} [add] https://crrev.com/681c95bfedb5ee83897f8a0d4104c91049ba16f6/third_party/WebKit/LayoutTests/fast/parser/frameset-in-fragment.html [modify] https://crrev.com/681c95bfedb5ee83897f8a0d4104c91049ba16f6/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp
,
Aug 16 2016
ClusterFuzz has detected this issue as fixed in range 411943:411945. Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6595391946752000 Fuzzer: bj_broddelwerk Job Type: linux_debug_chrome Platform Id: linux Crash Type: ASSERT Crash Address: Crash State: !node || (node->isElementNode()) blink::toElement blink::HTMLStackItem::element Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_debug_chrome&range=268656:269696 Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_debug_chrome&range=411943:411945 Minimized Testcase (0.97 Kb): https://cluster-fuzz.appspot.com/download/AMIfv95tC1oCT262LMx-ywEiHC4jeKwa8Wv9_MK3w1_CdLXKgmr0t3EL7miGcTfb7ivI7QBqpd95jK5NnmGftQz3Rd_tTALmg6HDjMo9mQQtBzCtOesPVSUjBP55scybrYPyCGUrYGGuPPKS7rvxXwp8lvTe71comQ?testcase_id=6595391946752000 See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Aug 16 2016
ClusterFuzz testcase is verified as fixed, closing issue. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Nov 22 2016
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by mmohammad@chromium.org
, Aug 9 2016Components: Blink>HTML
Status: Available (was: Untriaged)