New issue
Advanced search Search tips

Issue 840979 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Security



Sign in to add a comment

TextOffsetMapping make blink::SlotAssignment::Trace() to crash

Project Member Reported by ClusterFuzz, May 8 2018

Issue description

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: UNKNOWN READ
Crash Address: 0x0000023d77c0
Crash State:
  blink::SlotAssignment::Trace
  blink::TraceTrait<blink::SlotAssignment>::Trace
  blink::ScriptWrappable::Wrap
  
Sanitizer: thread (TSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=550101:550107

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, May 8 2018

Components: Blink>Internals>WTF Blink>MemoryAllocator>GarbageCollection
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, May 8 2018

Labels: Test-Predator-Auto-Owner
Owner: yosin@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/1324be868df000cb2e070199ce2d10cd0c5a2706 (Make NextWordPosition() to utilize TextOffsetMapping).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.

Comment 3 by yosin@chromium.org, May 9 2018

Components: -Blink>MemoryAllocator>GarbageCollection -Blink>Internals>WTF Blink>Editing>Selection
Status: Started (was: Assigned)

Comment 4 by yosin@chromium.org, May 9 2018

Summary: TextOffsetMapping make blink::SlotAssignment::Trace() to crash (was: Crash in blink::SlotAssignment::Trace)
Project Member

Comment 5 by sheriffbot@chromium.org, May 9 2018

Labels: M-67
Project Member

Comment 6 by sheriffbot@chromium.org, May 9 2018

Labels: ReleaseBlock-Stable
This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by sheriffbot@chromium.org, May 9 2018

Labels: Pri-1
Cc: awhalley@chromium.org
+awhalley@ (Security TPM) for triage.

Comment 9 by yosin@chromium.org, May 10 2018

Status: Fixed (was: Started)
http://crrev.com/c/1018923 should fix this issue.
Project Member

Comment 10 by ClusterFuzz, May 10 2018

ClusterFuzz has detected this issue as fixed in range 557407:557410.

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: UNKNOWN READ
Crash Address: 0x0000023d77c0
Crash State:
  blink::SlotAssignment::Trace
  blink::TraceTrait<blink::SlotAssignment>::Trace
  blink::ScriptWrappable::Wrap
  
Sanitizer: thread (TSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=550101:550107
Fixed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=557407:557410

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

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.
Project Member

Comment 11 by ClusterFuzz, May 10 2018

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

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

Comment 12 by sheriffbot@chromium.org, May 10 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: Merge-Request-67
requesting merge of https://chromium.googlesource.com/chromium/src.git/+/4c414662683ec6083af7de599683c7fc177dc5d6 per comment 9
Project Member

Comment 14 by sheriffbot@chromium.org, May 15 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
+awhalley@ for M67 merge review
govind@ - good for 67
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge for cl listed at #13 to M67 branch 3396 based on comment #16. Pls merge now so we can take it in for tomorrow's beta release. Thank you.
AFAIK there is no need to merge to M67, as the culprit patch is already reverted there:

Revert "Make NextWordPosition() to utilize TextOffsetMapping"

This reverts commit 1324be868df000cb2e070199ce2d10cd0c5a2706 for M67/Beta

TextOffsetMapping is not ready for release.
It should support edge cases.

TBR=yosin@chromium.org

Bug:  832055 ,  832061 ,  832101 ,  832261 ,  832350 ,  832497 ,  832639 ,  833172 ,  833180 
Change-Id: Iac5f58716619a626650088a55109922daf1a4f3a
Reviewed-on: https://chromium-review.googlesource.com/1013445
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#36}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}

It seems that ClusterFuzz calculates the impacted branches from revision numbers on trunk, instead of testing the actual branches
Labels: -ReleaseBlock-Stable -M-67 -Merge-Approved-67 M-68 Merge-Rejected-67
Thanks xiaochengh@ - yep, confirmed by code inspection. 

Comment 20 by yosin@chromium.org, May 15 2018

>#c7, this is already reverted on M67 on April 17 by http://crrev.com/c/1013445
Project Member

Comment 21 by sheriffbot@chromium.org, Aug 16

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment