New issue
Advanced search Search tips

Issue 893954 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Integer-overflow in blink::list_marker_text::ToCJKIdeographic

Project Member Reported by ClusterFuzz, Oct 10

Issue description

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

Fuzzer: inferno_twister
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  blink::list_marker_text::ToCJKIdeographic
  blink::list_marker_text::GetText
  blink::LayoutCounter::OriginalText
  
Sanitizer: undefined (UBSAN)

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

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Cc: kkaluri@chromium.org
Labels: Test-Predator-Wrong M-70
Owner: masonfreed@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.

Using Code Search for the file, "layout_counter.cc" suspecting the below Cl might have caused this issue

Suspect CL: https://chromium.googlesource.com/chromium/src/+/3fe4904a00ba85fe97d3b64dee683803e8d8049b

masonfreed@ -- Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Thanks!
Owner: ----
Status: Available (was: Assigned)
Hi, that's definitely not related to my CL. That CL, while it touched many files, did not alter any behavior at all.

This looks like an integer overflow that has been there probably for a while. Sorry, I don't have the context to know who might know more about it!
Components: Blink>Layout
Status: Untriaged (was: Available)
This is old. Also failing in a random old commit (r530763 - Jan 2018).
Owner: mstensho@chromium.org
Status: Assigned (was: Untriaged)
Also triggers an assertion failure right afterwards.

STDERR: [1:1:1126/144355.119081:FATAL:list_marker_text.cc(405)] Check failed: group[4] == kNoChar || group[4] == kDigit0 || group[4] == kDigit1. 
STDERR: #0 0x7f43cdcffe3f base::debug::StackTrace::StackTrace()
STDERR: #1 0x7f43cdc2510b logging::LogMessage::~LogMessage()
STDERR: #2 0x7f43c7666d92 blink::list_marker_text::ToCJKIdeographic()
STDERR: #3 0x7f43c7666506 blink::list_marker_text::GetText()
STDERR: #4 0x7f43c7571502 blink::LayoutCounter::OriginalText()
STDERR: #5 0x7f43c75728ed blink::LayoutCounter::UpdateCounter()

Easy to fix. I'll just do it.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 27

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

commit 3b49cce012e0241dcd8b1677bfda4553c06fc841
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Tue Nov 27 11:01:16 2018

More predictable handling of large negative CJK counter values.

Everything is first clamped to [INT_MIN, INT_MAX]. Then, if the value is
negative, it will be negated to become positive. This won't work if the
value is exactly INT_MIN, though, since INT_MIN (MSB set to 1; all other
bits set to 0) has no positive counterpart. So handle this manually.

Adding a non-exportable web test for this, since the behavior for such
numbers is only defined between -9999 and 9999 [1].

[1] https://www.w3.org/TR/css-counter-styles-3/#complex-cjk

Bug:  893954 
Change-Id: I9f3bfad6e73623c27f2e1e427a96c45ad81d6265
Reviewed-on: https://chromium-review.googlesource.com/c/1350949
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611066}
[modify] https://crrev.com/3b49cce012e0241dcd8b1677bfda4553c06fc841/third_party/blink/renderer/core/layout/list_marker_text.cc
[add] https://crrev.com/3b49cce012e0241dcd8b1677bfda4553c06fc841/third_party/blink/web_tests/fast/css/content-counter-large-negative-cjk-expected.html
[add] https://crrev.com/3b49cce012e0241dcd8b1677bfda4553c06fc841/third_party/blink/web_tests/fast/css/content-counter-large-negative-cjk.html

Status: Fixed (was: Assigned)
Project Member

Comment 8 by ClusterFuzz, Nov 28

ClusterFuzz has detected this issue as fixed in range 611065:611066.

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

Fuzzer: inferno_twister
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  blink::list_marker_text::ToCJKIdeographic
  blink::list_marker_text::GetText
  blink::LayoutCounter::OriginalText
  
Sanitizer: undefined (UBSAN)

Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=611065:611066

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

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 9 by ClusterFuzz, Nov 28

Labels: ClusterFuzz-Verified
Status: Verified (was: Fixed)
ClusterFuzz testcase 6326815453609984 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