New issue
Advanced search Search tips

Issue 660848 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Selection#setBaseAndExtent() with (null, 1)

Project Member Reported by ClusterFuzz, Oct 31 2016

Issue description

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

Fuzzer: mbarbella_js_mutation_layout
Job Type: linux_debug_content_shell_drt
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  offset == 0 in Position.cpp
  blink::PositionTemplate<>::PositionTemplate
  blink::DOMSelection::setBaseAndExtent
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_debug_content_shell_drt&range=427578:427883

Minimized Testcase (0.13 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv94YEl4PJrhReasHo_ttwqMDuIda7nfmAYQ7a4YRJ9uJNM__2zoAAbliuRdXlhC6H6VyDGwhXa4K-FbrC7up3J8W7AbM5XpQN8tzDZIxRhX9TUQxl0ZSh_ZjNqhtDrlsBlTMjYL7D55LmIwQAZsw3n0yyvfNOA?testcase_id=6365491338936320
<script>
      function __f_1(a, b) {
        window.getSelection().setBaseAndExtent(a, 1, b, 1);
      }
          __f_1();
</script>


Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Owner: tkent@chromium.org
Status: Assigned (was: Untriaged)
tkent @ could you please look into this.please feel free to re-assigned back if needed. thanks in advance !

Comment 2 by tkent@chromium.org, Oct 31 2016

Components: Blink>Editing
Owner: yosin@chromium.org

Comment 3 by yosin@chromium.org, Nov 1 2016

Owner: yoichio@chromium.org
We call Position ctor with anchorNode=null, offset=1.
Please follow the spec about this case.

Comment 4 by yosin@chromium.org, Nov 1 2016

Summary: Selection#setBaseAndExtent() with (null, 1) (was: offset == 0 in Position.cpp)
I found setBaseAndExtent(a,1, null, 0) behaves as collapse(a,1) in Blink.
Edge even clear selection at that case.
Following Edge goes forward interoperability but may brake compatibility.
How can we do? Intent to ship or add UMA?

Comment 6 by yosin@chromium.org, Nov 2 2016

Please file an issue in https://github.com/w3c/editing/issues
and update the spec then follow the spec.
Project Member

Comment 7 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

Comment 8 by yosin@chromium.org, Nov 29 2016

Components: -Blink>Editing Blink>Editing>Selection
Updating the spec is Pri-1, but handling null, n>0 in DOMSelection::setBaseAndExtent() is Pri-2.
I will, but letting crash happen is not good because this is very easy and simple.
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 2 2016

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

commit ebec09cdbe2965a07c54715cf4ba3b50603efe39
Author: yoichio <yoichio@chromium.org>
Date: Fri Dec 02 11:23:46 2016

selection.setBaseAndExtent with null should not crash.

setBaseAndExtent is non-nullable in spec:
http://w3c.github.io/selection-api/#widl-Selection-setBaseAndExtent-void-Node-
anchorNode-unsigned-long-anchorOffset-Node-focusNode-unsigned-long-focusOffset

However, there is the issue for that:
https://github.com/w3c/selection-api/issues/72

Edge clears selection with null node and Blink virtually does same.

'Virtually' means we go though false DCHECK statements in the release build.
This CL refines code path both on debug and release.

Besides exact behavior, selection.setBaseAndExtent(null,x,null,y) should not crash
 and this CL checks it in DOMSelection.

This CL makes selection.setBaseAndExtent(null,x,null,y) clear Selection
 and keeps selection.setBaseAndExtent(base,x,null,y) as selection.collpase(base,x)
 except x can over base's length.

BUG= 660848 

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

[add] https://crrev.com/ebec09cdbe2965a07c54715cf4ba3b50603efe39/third_party/WebKit/LayoutTests/editing/selection/set_base_and_extent/set_null.html
[modify] https://crrev.com/ebec09cdbe2965a07c54715cf4ba3b50603efe39/third_party/WebKit/Source/core/editing/DOMSelection.cpp

Project Member

Comment 11 by ClusterFuzz, Dec 3 2016

ClusterFuzz has detected this issue as fixed in range 435881:435928.

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

Fuzzer: mbarbella_js_mutation_layout
Job Type: linux_debug_content_shell_drt
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  offset == 0 in Position.cpp
  blink::PositionTemplate<>::PositionTemplate
  blink::DOMSelection::setBaseAndExtent
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_debug_content_shell_drt&range=427578:427883
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_debug_content_shell_drt&range=435881:435928

Minimized Testcase (0.13 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv94YEl4PJrhReasHo_ttwqMDuIda7nfmAYQ7a4YRJ9uJNM__2zoAAbliuRdXlhC6H6VyDGwhXa4K-FbrC7up3J8W7AbM5XpQN8tzDZIxRhX9TUQxl0ZSh_ZjNqhtDrlsBlTMjYL7D55LmIwQAZsw3n0yyvfNOA?testcase_id=6365491338936320
<script>
      function __f_1(a, b) {
        window.getSelection().setBaseAndExtent(a, 1, b, 1);
      }
          __f_1();
</script>


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, Dec 3 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.

Sign in to add a comment