New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 636057 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

!node || (node->isElementNode())

Project Member Reported by ClusterFuzz, Aug 9 2016

Issue description

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

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.
 
Cc: tkent@chromium.org yukishiino@chromium.org
Components: Blink>HTML
Status: Available (was: Untriaged)

Comment 2 by tkent@chromium.org, Aug 9 2016

Cc: -yukishiino@chromium.org -tkent@chromium.org
Components: -Blink>HTML Blink>HTML>Parser
Status: Untriaged (was: Available)
Here is a simplified reproduction that works on ToT debug:

<script>
  element = document.getElementsByTagName("*")[0];
  element.insertAdjacentHTML('afterbegin', '<tr><frameset></frameset></tr>');
</script>


Owner: csharrison@chromium.org
Status: Assigned (was: Untriaged)
Loading triager here, assigning to csharrison@
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.
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. 
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.
Cc: dominicc@chromium.org kouhei@chromium.org
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?
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...
Project Member

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

Project Member

Comment 11 by ClusterFuzz, 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.
Project Member

Comment 12 by ClusterFuzz, Aug 16 2016

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase is verified as fixed, closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Project Member

Comment 13 by sheriffbot@chromium.org, Nov 22 2016

Labels: -Restrict-View-EditIssue
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