Integer-overflow in views::DesktopWindowTreeHostX11::SetOpacity |
|||||
Issue descriptionDetailed 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.
,
Dec 26 2016
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.
,
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).
,
Dec 27 2016
Most probably new crash is caused by rolling new clang that allows better undefined behavior detection.
,
Dec 27 2016
Opacity is set to `const float kDragWidgetOpacity = .75f;`. As I mentioned earlier, this will obviously cause cause integer overflow.
,
Dec 27 2016
,
Dec 27 2016
Seems to be introduced on 25 May 2016 with https://codereview.chromium.org/2009333002
,
Jan 26 2017
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.
,
Jan 26 2017
,
Jan 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0f6dfbc4793fab2a08ea912039ca8ed6f958d81d commit 0f6dfbc4793fab2a08ea912039ca8ed6f958d81d Author: eustas <eustas@chromium.org> Date: Fri Jan 27 10:50:43 2017 Fix integer overflow. BUG= 676902 Review-Url: https://codereview.chromium.org/2598383002 Cr-Commit-Position: refs/heads/master@{#446639} [modify] https://crrev.com/0f6dfbc4793fab2a08ea912039ca8ed6f958d81d/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc [modify] https://crrev.com/0f6dfbc4793fab2a08ea912039ca8ed6f958d81d/ui/views/widget/widget.cc
,
Jan 27 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by msrchandra@chromium.org
, Dec 26 2016Components: Internals>Views
Labels: Test-Predator-Wrong-CLs
Owner: varkha@chromium.org
Status: Assigned (was: Untriaged)