New issue
Advanced search Search tips

Issue 835779 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Null-dereference READ in blink::ComputeInlineBoxPositionForInlineAdjustedPosition

Project Member Reported by ClusterFuzz, Apr 23 2018

Issue description

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

Fuzzer: ochang_domfuzzer
Job Type: linux_asan_chrome_v8_arm
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x00000000
Crash State:
  blink::ComputeInlineBoxPositionForInlineAdjustedPosition
  blink::ComputeInlineBoxPosition
  blink::RenderedPosition::RenderedPosition
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_v8_arm&range=551404:551408

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Apr 23 2018

Components: Blink>Editing
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, Apr 23 2018

Labels: Test-Predator-Auto-Owner
Owner: xiaoche...@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/614797964fd241c101fa60be39803c525eca6033 (Merge LocalCaretRect's line end hack into ComputeInlineBoxPosition()).

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

Comment 3 by bugdroid1@chromium.org, Apr 24 2018

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

commit 6b3a18f2b4200d348033000c7151692cd45f635f
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Tue Apr 24 04:49:56 2018

Fix ComputeInlineBoxPosition crash when after BR and next line has no leaf child

ComputeInlineBoxPosition() handles after-BR positions by looking into
the first leaf child of the next line, which crashes if the next line
has no leaf child.

This patch changes the function to iteratively check the succeeding
lines until we find a line with leaf child, to stop the crash and make
the behavior saner.

Bug:  835779 
Change-Id: Iafd04aaf467a8e53c92cb0d36ab291ff3627e67e
Reviewed-on: https://chromium-review.googlesource.com/1024799
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552999}
[modify] https://crrev.com/6b3a18f2b4200d348033000c7151692cd45f635f/third_party/blink/renderer/core/editing/inline_box_position.cc
[modify] https://crrev.com/6b3a18f2b4200d348033000c7151692cd45f635f/third_party/blink/renderer/core/editing/local_caret_rect_test.cc

Project Member

Comment 4 by ClusterFuzz, Apr 24 2018

ClusterFuzz has detected this issue as fixed in range 552998:552999.

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

Fuzzer: ochang_domfuzzer
Job Type: linux_asan_chrome_v8_arm
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x00000000
Crash State:
  blink::ComputeInlineBoxPositionForInlineAdjustedPosition
  blink::ComputeInlineBoxPosition
  blink::RenderedPosition::RenderedPosition
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_v8_arm&range=551404:551408
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_v8_arm&range=552998:552999

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

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 5 by ClusterFuzz, Apr 24 2018

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: Merge-Request-67
The crashing code was introduce in M66, and still reproduces there, for example, crash 27845ab065f97345f4.

Note that the stack trace in that report is different from here because the relevant code was moved in a refactoring patch (r551408).

I'm not going to request for merge into M66, since the crash occurs only in an edge case.
Cc: gov...@chromium.org
Status: Assigned (was: Verified)
Ping for merge review...

Comment 8 by gov...@chromium.org, Apr 25 2018

Pls wait for auto approval script to kick it in. If not, I will review it. Thank you.
Project Member

Comment 9 by sheriffbot@chromium.org, Apr 25 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact 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
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 25 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/efac74f6da92ac890a27bed5b3d25fe62e7f1785

commit efac74f6da92ac890a27bed5b3d25fe62e7f1785
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Apr 25 18:49:02 2018

Fix ComputeInlineBoxPosition crash when after BR and next line has no leaf child

ComputeInlineBoxPosition() handles after-BR positions by looking into
the first leaf child of the next line, which crashes if the next line
has no leaf child.

This patch changes the function to iteratively check the succeeding
lines until we find a line with leaf child, to stop the crash and make
the behavior saner.

Bug:  835779 
Tbr: yosin@chromium.org, yoichio@chromium.org
Change-Id: I9d949129f087264e8332e88007ef6a6982e15638
Reviewed-on: https://chromium-review.googlesource.com/1024799
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#552999}(cherry picked from commit 6b3a18f2b4200d348033000c7151692cd45f635f)
Reviewed-on: https://chromium-review.googlesource.com/1028587
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#300}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/efac74f6da92ac890a27bed5b3d25fe62e7f1785/third_party/blink/renderer/core/editing/local_caret_rect.cc
[modify] https://crrev.com/efac74f6da92ac890a27bed5b3d25fe62e7f1785/third_party/blink/renderer/core/editing/local_caret_rect_test.cc

Status: Fixed (was: Assigned)

Sign in to add a comment