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

xiaochengh@: You seem to be an expert in the TextIterator. Can you PTAL?

We got

offset + length_to_append > text_.length()

in TextIteratorTextState::AppendTextTo()

Investigating...

Root cause is that when emitting from ::first-letter, RestoreCollapsedTrailingSpace() uses the wrong LayoutText. It should use the one for ::first-letter, but is using the one for remaining text instead.

As a result, it sets the text run end over the string length, causing the later buffer overflow.

Minimal repro:

<style>
font {
  -webkit-appearance: progress-bar;
}

:first-letter {
  color:black
}

* {
  -webkit-rtl-ordering: visual;
  -webkit-margin-end: -1px
}
</style>
<font dir="rtl">iC <q></q></font>
<script>
window.find();
</script>

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

commit 3558806d40c71d496235807ea0f816823d8e4412
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu Aug 10 05:09:15 2017

Use the correct LayoutText in RestoreCollapsedTrailingSpace()

Function TextIteratorTextNodeHandler::RestoreCollapsedTrailingSpace()
may use the incorrect LayoutText when emitting text from :first-letter,
where it should use the LayoutText for :first-letter but is using the
one for remaining text instead.

This patch fixes the above bug.

Bug:  752941 
Change-Id: I115bbc7de2cc2bfb97266b7e4e438519f17ea75a
Reviewed-on: https://chromium-review.googlesource.com/609318
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493238}
[add] https://crrev.com/3558806d40c71d496235807ea0f816823d8e4412/third_party/WebKit/LayoutTests/editing/text-iterator/first_letter_find_string_crash.html
[modify] https://crrev.com/3558806d40c71d496235807ea0f816823d8e4412/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.cpp
[modify] https://crrev.com/3558806d40c71d496235807ea0f816823d8e4412/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp

Add a reminder for me to merge to M61 on this Fri.

ClusterFuzz has detected this issue as fixed in range 493236:493241.

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

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

Crash Type: Heap-buffer-overflow READ 1
Crash Address: 0x23928a3d
Crash State:
  blink::TextIteratorTextState::AppendTextTo
  blink::TextIteratorAlgorithm<class blink::EditingAlgorithm<class blink::FlatTree
  blink::TextIteratorAlgorithm<class blink::EditingAlgorithm<class blink::FlatTree
  
Sanitizer: address (ASAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=443500:443512
Fixed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=493236:493241

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

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.

ClusterFuzz testcase 5951261534060544 is verified as fixed, so closing issue as verified.

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

The NextAction date has arrived: 2017-08-11

This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

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

+awhalley@ for M61 merge review.

govind@ - good for 61

Approving merge to M61 branch 3163 based on comment #19. Please merge ASAP so we can take it in for this week Beta. Thank you.

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

commit d8be0b2d6059b892ee8b7e1209bfbe5ecce8021e
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Mon Aug 14 20:14:41 2017

Use the correct LayoutText in RestoreCollapsedTrailingSpace()

Function TextIteratorTextNodeHandler::RestoreCollapsedTrailingSpace()
may use the incorrect LayoutText when emitting text from :first-letter,
where it should use the LayoutText for :first-letter but is using the
one for remaining text instead.

This patch fixes the above bug.

TBR=xiaochengh@chromium.org

(cherry picked from commit 3558806d40c71d496235807ea0f816823d8e4412)

Bug:  752941 
Change-Id: I115bbc7de2cc2bfb97266b7e4e438519f17ea75a
Reviewed-on: https://chromium-review.googlesource.com/609318
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#493238}
Reviewed-on: https://chromium-review.googlesource.com/614381
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#551}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[add] https://crrev.com/d8be0b2d6059b892ee8b7e1209bfbe5ecce8021e/third_party/WebKit/LayoutTests/editing/text-iterator/first_letter_find_string_crash.html
[modify] https://crrev.com/d8be0b2d6059b892ee8b7e1209bfbe5ecce8021e/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.cpp
[modify] https://crrev.com/d8be0b2d6059b892ee8b7e1209bfbe5ecce8021e/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp

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