New issue
Advanced search Search tips

Issue 660269 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

DCHECK hit in RangeBoundaryPoint::set: childBefore == offset ? NodeTraversal::childAt(*container, offset - 1) : 0

Project Member Reported by ClusterFuzz, Oct 28 2016

Issue description

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

Fuzzer: ifratric-browserfuzzer-v3
Job Type: windows_syzyasan_chrome
Platform Id: windows

Crash Type: UNKNOWN
Crash Address: 0x0000000b
Crash State:
  blink::Range::getBorderAndTextQuads
  blink::Range::boundingRect
  blink::Range::getBoundingClientRect
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=windows_syzyasan_chrome&range=427353:427578

Minimized Testcase (1.68 Kb): https://cluster-fuzz.appspot.com/download/AMIfv975v85wA2K6LOHR7Jkxpwe3gV-fW3qz33v45am8zW5oOnDJp1gD8qYPNEl67Vss8ANtVYmrEfhGewycGETV-KNRIWFFMI6p0yyMN6GbGH0653BuYe4QEtgdnpvzVlI8tVQPFeq3o_26psOvQF1QUGLAoLRH3A?testcase_id=4771142067027968

Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Components: Blink>Editing
Labels: M-56 Te-Logged
Owner: xiaoche...@chromium.org
Status: Assigned (was: Untriaged)
Possible suspect CL
https://chromium.googlesource.com/chromium/src/+/32a86fdfe234f0b8ceefb6b5fad5a95de3d9bef6
xiaochengh@, could you please take a look and help us to find correct owner if it is not related your changes.
Components: -Blink>Editing Blink>DOM
Summary: DCHECK hit in RangeBoundaryPoint::set: childBefore == offset ? NodeTraversal::childAt(*container, offset - 1) : 0 (was: Crash in blink::Range::getBorderAndTextQuads)
Can also repro with Linux.

The testcase hits the following assertion, which seems to cause the crash:

[1:1:1031/191400:16913803637:FATAL:RangeBoundaryPoint.h(156)] Check failed: childBefore == offset ? NodeTraversal::childAt(*container, offset - 1) : 0 (FRAME vs. AUDIO)
#0 0x7f7dc10934ee base::debug::StackTrace::StackTrace()
#1 0x7f7dc1102a1f logging::LogMessage::~LogMessage()
#2 0x7f7db7fd4df5 blink::RangeBoundaryPoint::set()
#3 0x7f7db7fcfbea blink::Range::insertNode()
#4 0x7f7db8f2bc09 blink::RangeV8Internal::insertNodeMethod()
...

The following is a manually minimized test case hitting the same assertion:

<audio src="aOyer">

<script>
const audio = document.querySelector('audio');
const newNode = document.createElement('frame');

function handler(event) {
  document.body.webkitRequestFullscreen(); 
  var range = document.createRange(); 
  range.setStartBefore(audio); 
  range.insertNode(newNode); 
  range.deleteContents();
}

window.onpageshow = handler;
document.onwebkitfullscreenerror = handler;
audio.onloadstart = handler;
</script>

The crashing scenario seems to be like this:

1. Range::insertNode is called, trying to insert a FRAME at the beginning of the Range
2. Step 1 calls ContainerNode::insertBefore to insert the FRAME into DOM tree, which also causes loading of the frame
3. The FRAME destroys itself during loading. As a result, the FRAME is not in DOM tree when ContainerNode::insertBefore returns
4. Range::insertNode assumes that nothing special happened during ContainerNode::insertBefore other than the insertion of FRAME, and thus, hits the assertion in RangeBoundaryPoint::set

Not sure how the frame gets destroyed during the loading, and will investigate further.
Labels: -OS-Windows OS-All
Project Member

Comment 4 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
Project Member

Comment 5 by ClusterFuzz, Jan 21 2017

ClusterFuzz has detected this issue as fixed in range 444720:444724.

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

Fuzzer: ifratric-browserfuzzer-v3
Job Type: windows_syzyasan_chrome
Platform Id: windows

Crash Type: UNKNOWN
Crash Address: 0x0000000b
Crash State:
  blink::Range::getBorderAndTextQuads
  blink::Range::boundingRect
  blink::Range::getBoundingClientRect
  
Memory Tool: SYZYASAN

Regressed: https://cluster-fuzz.appspot.com/revisions?job=windows_syzyasan_chrome&range=427353:427578
Fixed: https://cluster-fuzz.appspot.com/revisions?job=windows_syzyasan_chrome&range=444720:444724

Minimized Testcase (1.68 Kb): https://cluster-fuzz.appspot.com/download/AMIfv975v85wA2K6LOHR7Jkxpwe3gV-fW3qz33v45am8zW5oOnDJp1gD8qYPNEl67Vss8ANtVYmrEfhGewycGETV-KNRIWFFMI6p0yyMN6GbGH0653BuYe4QEtgdnpvzVlI8tVQPFeq3o_26psOvQF1QUGLAoLRH3A?testcase_id=4771142067027968

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 6 by ClusterFuzz, Jan 21 2017

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: ClusterFuzz-Wrong
Status: Assigned (was: Verified)
The minimized test case no longer repro simply because script-initiated webkitRequestFullscreen is banned

The unminimized test case still repros and hits the same assertion as in #2 (@r445580)

Comment 8 by yuzus@chromium.org, Apr 4 2017

What is the situation of this now?
Cc: xiaoche...@chromium.org
Owner: ----
Status: Available (was: Assigned)
No progress. Feel free to take it.

Comment 10 by yuzus@chromium.org, Apr 12 2017

Owner: yuzus@chromium.org
Status: Started (was: Available)
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8de91ba0d69c4f2b64407c5a218774e554e0c9a

commit f8de91ba0d69c4f2b64407c5a218774e554e0c9a
Author: yuzus <yuzus@chromium.org>
Date: Thu Apr 13 09:00:25 2017

Make Pageshow event asynchronous in EventQueueScope

As per spec Pageshow event must be fired asynchonously all the time, but to be compatible with other browsers blink used to fire Pageshow synchronously.
And thus Pageshow event was able to modify nodes when it should not.

This CL makes Pageshow fired asynchronously when it is in EventQueueScope to solve the problem.
BUG= 660269 ,543424

Review-Url: https://codereview.chromium.org/2815803003
Cr-Commit-Position: refs/heads/master@{#464347}

[add] https://crrev.com/f8de91ba0d69c4f2b64407c5a218774e554e0c9a/third_party/WebKit/LayoutTests/fast/dom/Range/insertNode-trigger-onpageshow-crash.html
[modify] https://crrev.com/f8de91ba0d69c4f2b64407c5a218774e554e0c9a/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp

Comment 12 by yuzus@chromium.org, Apr 14 2017

Status: Fixed (was: Started)
So what was happening here was below:
1: audio.onloadstart -> handler is called
2: document.onwebkitfullscreenerror -> handler is called several times
3: During Range::insertNode(), onpageshow is triggered 
4: window.onpageshow -> handler is called synchronously, deleting frame
5: Coming back to Range::insertNode(), where frame is already deleted

The problem was that PageShow event was implemented as a synchronous though it is supposed to be asynchronous.
In the CL above I changed the PageShow event to asynchronous when needed.
I didn't make it completely asynchronous to match other browsers' implementations.

Labels: -ClusterFuzz-Wrong
We have made a bunch of changes on ClusterFuzz side, so resetting ClusterFuzz-Wrong label.

Sign in to add a comment