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

Issue 748570 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 751817



Sign in to add a comment

SelectionController::UpdateSelectionForMouseDownDispatchingSelectStart() should not use invalid SelectionInFlatTree

Project Member Reported by ClusterFuzz, Jul 25 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6076383544737792

Fuzzer: inferno_layout_test_unmodified
Job Type: windows_asan_chrome
Platform Id: windows

Crash Type: Null-dereference READ
Crash Address: 0x00000008
Crash State:
  blink::FlatTreeTraversal::TraverseChild
  blink::FlatTreeTraversal::ChildAt
  blink::PositionIteratorAlgorithm<blink::EditingAlgorithm<blink::FlatTreeTraversa
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome&range=456190:456233

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6076383544737792


Additional requirements: Requires Gestures

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Cc: msrchandra@chromium.org
Labels: M-62 Test-Predator-Wrong
Owner: r...@opera.com
Status: Assigned (was: Untriaged)
Predator and CL did not provide any possible suspects.
Using Code Search for the file, "FlatTreeTraversal.cpp assigning to concern owner.

Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/5e53c60ebe374282c8c2864a8e6d1d864afa221a

@rune -- Could you please look into the issue, please re-assign if it has nothing to do with your changes.
Thank You.

Comment 2 by r...@opera.com, Jul 31 2017

Added a simpler test with interaction requirements in comment.
crash.html
200 bytes View Download

Comment 3 by r...@opera.com, Aug 2 2017

Components: Blink>Editing>Selection
Owner: ----
Status: Untriaged (was: Assigned)
Just saw the regression range now which doesn't contain any of my changes. The editing code fails DCHECKs all over the place before the crash because the selectstart handler modifies the DOM, and parts of the editing code expects the DOM/shadow distribution/layout to not change.

Blocking: 751817
Owner: yosin@chromium.org
Status: Assigned (was: Untriaged)

Comment 5 by yosin@chromium.org, Aug 3 2017

Status: Started (was: Assigned)
I take care.

Comment 6 by yosin@chromium.org, Aug 3 2017

With sample HTML in #c2, I hit DCHECK() in SelectionTemplate::Base()

Check failed: base_.GetDocument()->DomTreeVersion() == dom_tree_version_ (16 vs. 11)SelectionInFlatTree(Dirty: 11 != 16 base: BODY@offsetInAnchor[0], extent: BODY@offsetInAnchor[0])

Note: The checker in SelectionTemplate works as expected.

SelectionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> >::AssertValid() Line 105SelectionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> >::Base() Line 60
CanonicalizeSelection<blink::EditingAlgorithm<blink::FlatTreeTraversal> >(const blink::SelectionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> > & selection) Line 245
VisibleSelectionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> >::Validate(const blink::SelectionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> > & passed_selection, blink::TextGranularity granularity) Line 491
VisibleSelectionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> >::VisibleSelectionTemplate(const blink::SelectionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> > & selection, blink::TextGranularity granularity) Line 56
VisibleSelectionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> >::Create() Line 61
CreateVisibleSelection(const blink::SelectionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> > & selection) Line 70
SelectionController::SetNonDirectionalSelectionIfNeeded(const blink::SelectionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> > & passed_selection, blink::TextGranularity granularity, blink::SelectionController::EndPointsAdjustmentMode endpoints_adjustment_mode, blink::HandleVisibility handle_visibility) Line 768
SelectionController::UpdateSelectionForMouseDownDispatchingSelectStart(blink::Node * target_node, const blink::SelectionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> > & selection, blink::TextGranularity granularity, blink::HandleVisibility handle_visibility) Line 510
SelectionController::HandleSingleClick(const blink::EventWithHitTestResults<blink::WebMouseEvent> & event) Line 365
SelectionController::HandleMousePressEvent(const blink::EventWithHitTestResults<blink::WebMouseEvent> & event) Line 958
MouseEventManager::HandleMousePressEvent(const blink::EventWithHitTestResults<blink::WebMouseEvent> & event) Line 679
EventHandler::HandleMousePressEvent(const blink::WebMouseEvent & mouse_event) Line 726
PageWidgetEventHandler::HandleMouseDown(blink::LocalFrame & main_frame, const blink::WebMouseEvent & event) Line 253
WebViewImpl::HandleMouseDown(blink::LocalFrame & main_frame, const blink::WebMouseEvent & event) Line 433
PageWidgetDelegate::HandleInputEvent(const blink::WebCoalescedInputEvent & handler, blink::LocalFrame * coalesced_event) Line 167
WebViewImpl::HandleInputEvent(const blink::WebCoalescedInputEvent & coalesced_event) Line 2204
WebViewFrameWidget::HandleInputEvent(const blink::WebCoalescedInputEvent & event) Line 95

Comment 7 by yosin@chromium.org, Aug 3 2017

In review: http://crrev.com/c/599967

Comment 8 by yosin@chromium.org, Aug 4 2017

Summary: SelectionController::UpdateSelectionForMouseDownDispatchingSelectStart() should not use invalid SelectionInFlatTree (was: Null-dereference READ in blink::FlatTreeTraversal::TraverseChild)
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 9 2017

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

commit e8794d470e391d9876e5cf031f8d59c8e7f4e70b
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Wed Aug 09 01:26:31 2017

Make UpdateSelectionForMouseDownDispatchingSelectStart() not to use invalid SelectionInFlatTree

This patch makes |UpdateSelectionForMouseDownDispatchingSelectStart()| of
|SelectionController| not to use invalid |SelectionInFlatTree| with
newly introduced class |SelectionInFlatTree::InvalidSelectionNullifier|.

Note: "invalid selection" means selections having disconnected position or
position in another document.

Bug:  748570 
Change-Id: I7ffbbc75351a7d8c81ba217e35f3f7ba367c52dd
Reviewed-on: https://chromium-review.googlesource.com/599967
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492802}
[add] https://crrev.com/e8794d470e391d9876e5cf031f8d59c8e7f4e70b/third_party/WebKit/LayoutTests/editing/selection/mouse/selectstart_modify_crash.html
[modify] https://crrev.com/e8794d470e391d9876e5cf031f8d59c8e7f4e70b/third_party/WebKit/Source/core/editing/SelectionController.cpp
[modify] https://crrev.com/e8794d470e391d9876e5cf031f8d59c8e7f4e70b/third_party/WebKit/Source/core/editing/SelectionTemplate.cpp
[modify] https://crrev.com/e8794d470e391d9876e5cf031f8d59c8e7f4e70b/third_party/WebKit/Source/core/editing/SelectionTemplate.h

Project Member

Comment 10 by ClusterFuzz, Aug 9 2017

ClusterFuzz has detected this issue as fixed in range 492772:492814.

Detailed report: https://clusterfuzz.com/testcase?key=6076383544737792

Fuzzer: inferno_layout_test_unmodified
Job Type: windows_asan_chrome
Platform Id: windows

Crash Type: Null-dereference READ
Crash Address: 0x00000008
Crash State:
  blink::FlatTreeTraversal::TraverseChild
  blink::FlatTreeTraversal::ChildAt
  blink::PositionIteratorAlgorithm<blink::EditingAlgorithm<blink::FlatTreeTraversa
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome&range=456190:456233
Fixed: https://clusterfuzz.com/revisions?job=windows_asan_chrome&range=492772:492814

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6076383544737792


Additional requirements: Requires Gestures

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Status: Fixed (was: Started)
Project Member

Comment 12 by ClusterFuzz, Aug 9 2017

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

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

Sign in to add a comment