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

Issue 676902 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Integer-overflow in views::DesktopWindowTreeHostX11::SetOpacity

Project Member Reported by ClusterFuzz, Dec 24 2016

Issue description

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

Fuzzer: inferno_twister
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  views::DesktopWindowTreeHostX11::SetOpacity
  views::DesktopDragDropClientAuraX11::CreateDragWidget
  views::DesktopDragDropClientAuraX11::StartDragAndDrop
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_ubsan_chrome&range=370022:370027

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv94lMfKt3TyGLdjKjvbk1VbLjXsgT_wDT36-6Dzw0BNPyFjf7o-lUKZII6O2CdJW5d48Ok5tJPK786TyfPytbdIf7xJx37535dZCgTduzLqSUFYo2d8AOBX_NEEl5wwn78BVKLj_7LacYTfGF6C7M-CTlozHgKDuEYENeFTv008yRKZvMsk?testcase_id=4570807469867008


Additional requirements: Requires Gestures

Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Cc: msrchandra@chromium.org
Components: Internals>Views
Labels: Test-Predator-Wrong-CLs
Owner: varkha@chromium.org
Status: Assigned (was: Untriaged)
Find it and CL did not provide any possible suspects.
Assigning to the concern owner from Code Search using the file, "desktop_window_tree_host_x11.cc"

Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/baad79f91de6ea9fd9ecc1b94980d71f2e7b92c1

@varkha --  Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.

Comment 2 by varkha@chromium.org, Dec 26 2016

Cc: varkha@chromium.org
Owner: eustas@chromium.org
Could it be a fuzzing change r370026? It is in the range. Also I don't think the commit in#1 is in play here - there's no theme change in the call stack is there? It should be possible to come firm this with a revert of either CL.

Comment 3 by eustas@chromium.org, Dec 27 2016

brotli fuzzer is a standalone binary, not included into chromium.

Line `unsigned long cardinality = static_cast<int>(opacity * 255) * 0x1010101;` seems to be obviously the cause of overflow (when opacity < 0 or opacity > 0.5).

First, casting to int is wrong, something unsigned should be used.
Second, `opacity` value should be clamped to [0.0, 1.0]
Third, I think it is preferable to create uint8_t value and then clone it with logical, but not arithmetical operations (this is not performance critical path, we don't care if we trade one multiplication to 2 shift + 2 or).

Going to check what caused bad `opacity` values, and fix it (or find right owner).

Comment 4 by eustas@chromium.org, Dec 27 2016

Most probably new crash is caused by rolling new clang that allows better undefined behavior detection.

Comment 5 by eustas@chromium.org, Dec 27 2016

Opacity is set to `const float kDragWidgetOpacity = .75f;`.
As I mentioned earlier, this will obviously cause cause integer overflow.

Comment 6 by eustas@chromium.org, Dec 27 2016

Status: Started (was: Assigned)
Fix: https://codereview.chromium.org/2598383002/

Comment 7 by eustas@chromium.org, Dec 27 2016

Cc: sky@chromium.org
Seems to be introduced on 25 May 2016 with https://codereview.chromium.org/2009333002
Project Member

Comment 8 by ClusterFuzz, Jan 26 2017

Status: WontFix (was: Started)
ClusterFuzz testcase 4570807469867008 is flaky and no longer reproduces, so closing issue.

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

Comment 9 by eustas@chromium.org, Jan 26 2017

Status: Started (was: WontFix)
Waiting for review: https://codereview.chromium.org/2598383002/
Status: Fixed (was: Started)

Sign in to add a comment