New issue
Advanced search Search tips

Issue 635432 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Integer-overflow in gfx::Rect::right

Project Member Reported by ClusterFuzz, Aug 8 2016

Issue description

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

Fuzzer: inferno_twister
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  gfx::Rect::right
  gfx::Rect::Intersect
  cc::PictureLayerImpl::UpdateViewportRectForTilePriorityInContentSpace
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_ubsan_chrome&range=370022:370027

Minimized Testcase (1.71 Kb): https://cluster-fuzz.appspot.com/download/AMIfv94H75nQhDnVqm9W3CbGNzdVMRf_d0CBXyNvrI4W_Ddd4EKtq6Hkk7L1t5Ehlwg25KGTFGPvBl1-iR0H5_3Gnv4kZjijaUHRjr10MWW3IgoSTyDpzgNoMHVkf5jt-Jye0DNuNzSg3VhrQcm5mWkk52_X3J4J0w?testcase_id=4512273148411904

Issue manually filed by: ajha

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 

Comment 1 by ajha@chromium.org, Aug 8 2016

Cc: ajha@chromium.org
Components: Internals
Labels: Te-Logged M-53
Owner: a...@chromium.org
Status: Assigned (was: Untriaged)
Findit Result:
==============
Suspected CLs	No CL in the regression range changes the crashed files. The result is the blame information.

Author: Peter Kasting
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/275539a60ec716bea022200fa650a409772a81bf
Time: Wed Jun 15 01:41:42 2016
The CL last changed line 77 of file rect.h, which is stack frame 0.

Author: danakj
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/a5cc021b6ee19af6a931db1042ec2ab562506010
Time: Wed Oct 15 21:27:42 2014
The CL last changed line 126 of file rect.cc, which is stack frame 4.

Author: vmpstr
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/65d8098f4449c3a556b8ca588a2ab0ec6c702eff
Time: Mon Jun 15 23:20:13 2015
The CL last changed line 517 of file picture_layer_impl.cc, which is stack frame 5.

Author: hush
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/9178c77a0167ee7c73364b21a6f3a633029b7a44
Time: Thu Jan 22 01:57:39 2015
The CL last changed line 455 of file picture_layer_impl.cc, which is stack frame 6.

Suspected Project: chromium-buildtools
================================================

None of the CL from the above findit result looks related.

Based on the code search on 'rect.h', suspected change: https://codereview.chromium.org/1543183002

avi@: Could you please take a look at this and help in investigating this further.

Thank you!


Comment 2 by a...@chromium.org, Aug 8 2016

Components: -Internals Internals>Compositing
Owner: enne@chromium.org
Integer overflow is a misuse of gfx::Rect. My change was to the internals of gfx::Rect.

The first stack frame of the use is cc::PictureLayerImpl::UpdateViewportRectForTilePriorityInContentSpace. Assigning to an owner of that.

Comment 3 by enne@chromium.org, Aug 8 2016

Cc: enne@chromium.org
Owner: vmp...@chromium.org
This maybe a duplicate of  issue 635511  or maybe a similar issue.  Assigning to vmpstr to investigate as well.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 11 2016

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

commit fc7eed4f2a4fbb35d1d243ca07fac20dcb20b07f
Author: vmpstr <vmpstr@chromium.org>
Date: Thu Aug 11 19:16:48 2016

cc: Change visible rect in content space calculation to not overflow.

This patch changes the intersect call to use safe intersect math since
the rect intersect version might overflow (although it is likely
faster)

R=enne, danakj
BUG= 635432 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2234183002
Cr-Commit-Position: refs/heads/master@{#411396}

[modify] https://crrev.com/fc7eed4f2a4fbb35d1d243ca07fac20dcb20b07f/cc/layers/picture_layer_impl.cc

Comment 5 by vmp...@chromium.org, Aug 11 2016

Status: Fixed (was: Assigned)
This one should be fixed. 

Comment 6 by ajha@chromium.org, Aug 12 2016

vmpstr@: Once the fix is baked in canary, Shall we get this merged to M-53 as well. CF shows this impacting M-52 and M-53.

Thank you!
Project Member

Comment 7 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

Sign in to add a comment