New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 655473 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Integer-overflow in blink::CounterDirectives::combinedValue

Project Member Reported by ClusterFuzz, Oct 13 2016

Issue description

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

Fuzzer: miaubiz_svg_fuzzer
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  blink::CounterDirectives::combinedValue
  planCounter
  blink::makeCounterNodeIfNeeded
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_ubsan_chrome&range=370022:370027

Minimized Testcase (1.10 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97b5HGz-LK4gK0AoPx7H_2c57mpKZb7Y4Y4QIIz6wkRP8ZDe9YB_fc1_XpKoIQ1xcEW6SkOu5NPG6lxR0eKE6tssDui2Ia2hGNvTzzFA4q6vTB6fSqqG19-ci2h13ZkoFUo1TtakE5wQmsBCdsBfjDLrIJyIw?testcase_id=6001544598388736

Issue manually filed by: nyerramilli

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Cc: nyerramilli@chromium.org
Components: Tools>Test>FindIt>WrongResult Infra>Git
Labels: -Type-Bug findit-wrong M-54 Te-Logged Type-Bug-Regression
Owner: szager@chromium.org
Status: Assigned (was: Untriaged)
Providing Findit results for internal purpose:

Suspected CLs	Git blame below is NOT necessarily who introduced the crash nor the owner for it. Please check the code before assigning to anyone.(No CL in the regression range changed the crashing files.)

Author: Blink Reformat
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/1c8e1a7719e9d223cc84e838c9a31a0210f5878b
Time: Sat Oct 01 00:25:32 2016
The CL last changed line 87 of file CounterDirectives.h, which is stack frame 0.

Author: Blink Reformat
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/1c8e1a7719e9d223cc84e838c9a31a0210f5878b
Time: Sat Oct 01 00:25:32 2016
The CL last changed line 173 of file LayoutCounter.cpp, which is stack frame 1.

Author: Blink Reformat
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/1c8e1a7719e9d223cc84e838c9a31a0210f5878b
Time: Sat Oct 01 00:25:32 2016
The CL last changed line 381 of file LayoutCounter.cpp, which is stack frame 2.

Author: Blink Reformat
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/1c8e1a7719e9d223cc84e838c9a31a0210f5878b
Time: Sat Oct 01 00:25:32 2016
The CL last changed line 655 of file LayoutCounter.cpp, which is stack frame 3.

Author: Blink Reformat
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/1c8e1a7719e9d223cc84e838c9a31a0210f5878b
Time: Sat Oct 01 00:25:32 2016
The CL last changed line 1859 of file LayoutObject.cpp, which is stack frame 4.

Author: Blink Reformat
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/1c8e1a7719e9d223cc84e838c9a31a0210f5878b
Time: Sat Oct 01 00:25:32 2016
The CL last changed line 259 of file LayoutBoxModelObject.cpp, which is stack frame 5.

Author: Blink Reformat
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/1c8e1a7719e9d223cc84e838c9a31a0210f5878b
Time: Sat Oct 01 00:25:32 2016
The CL last changed line 243 of file LayoutBox.cpp, which is stack frame 6.

Suspected Project: chromium
Suspected Component: Blink>Layout


using codesearch, seeing some changes to 'LayoutBoxModelObject.cpp' in
https://chromium.googlesource.com/chromium/src/+/80e6cf8d1fa064239b919bd111dc9304209dc9d3

szager@, could you please check and help.

Components: -Tools>Test>FindIt>WrongResult
Labels: Test-Predator-Wrong
Project Member

Comment 3 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
Project Member

Comment 4 by ClusterFuzz, Oct 1 2017

Components: Blink>CSS Blink>Layout
Labels: Test-Predator-AutoComponents
Automatically applying components based on information from OWNERS files. If this seems incorrect, please apply the Test-Predator-Wrong-Components label.

Comment 5 by shend@chromium.org, Oct 2 2017

Cc: szager@chromium.org
Components: -Blink>Layout -Infra>Git
Owner: ----
Status: Available (was: Assigned)
Looks like this FIXME needs to be fixed: https://chromium.googlesource.com/chromium/src/+/9da284ba7ab742893cba73c1078908eac5d3a6d1/third_party/WebKit/Source/core/style/CounterDirectives.h#86

Making Blink>CSS the sole component as per our triage procedure.

Comment 6 by shend@chromium.org, Oct 3 2017

Cc: meade@chromium.org
Hi Eddy, do you know what we're supposed to do with integer overflows? Can we just cap the value at INT_MAX?
Labels: Update-Weekly

Comment 8 by shend@chromium.org, Oct 9 2017

Cc: shend@chromium.org

Comment 9 by shend@chromium.org, Oct 10 2017

Owner: shend@chromium.org
Status: Started (was: Available)
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 17 2017

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

commit e18ac62b00618f26ef125554dd8241b2aed711e9
Author: Darren Shen <shend@chromium.org>
Date: Tue Oct 17 10:44:55 2017

Prevent counter values from over/underflowing.

Currently we over/underflow if a counter value goes beyond the limits
of a 32 bit integer. The spec [1] says we can ignore the increment
if it would cause an over/underflow. This patch implements this
behaviour.

Firefox follows the spec in this.

[1] https://drafts.csswg.org/css-lists-3/#valdef-counter-reset-custom-ident-integer

Bug:  655473 
Change-Id: I6e10b6e4f5f672d723a0ccb69fd309d52d1d1204
Reviewed-on: https://chromium-review.googlesource.com/708105
Reviewed-by: Justin Schuh <jschuh@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509351}
[add] https://crrev.com/e18ac62b00618f26ef125554dd8241b2aed711e9/third_party/WebKit/LayoutTests/fast/css/counters/counter-overflow.html
[modify] https://crrev.com/e18ac62b00618f26ef125554dd8241b2aed711e9/third_party/WebKit/Source/core/layout/CounterNode.cpp
[modify] https://crrev.com/e18ac62b00618f26ef125554dd8241b2aed711e9/third_party/WebKit/Source/core/style/CounterDirectives.h

Project Member

Comment 11 by ClusterFuzz, Oct 18 2017

ClusterFuzz has detected this issue as fixed in range 509326:509389.

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

Fuzzer: miaubiz_svg_fuzzer
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  blink::CounterDirectives::CombinedValue
  PlanCounter
  blink::MakeCounterNodeIfNeeded
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=370022:370027
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=509326:509389

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

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 12 by ClusterFuzz, Oct 18 2017

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: -Test-Predator-AutoComponents Test-Predator-Auto-Components

Sign in to add a comment