Integer-overflow in SkIRect::height |
|||||
Issue descriptionDetailed report: https://cluster-fuzz.appspot.com/testcase?key=6723049080750080 Fuzzer: miaubiz_svg_fuzzer Job Type: linux_ubsan_chrome Platform Id: linux Crash Type: Integer-overflow Crash Address: Crash State: SkIRect::height SkCanvas::resetForNextPicture SkRecorder::reset Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_ubsan_chrome&range=370022:370027 Minimized Testcase (1.58 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97vGl1zMMvk4K0owrSC5w1mv1km6ijMBFpssJBT-gi1a_CznA3MquaiEqEcvmIaEyEU3ITaHvnL7H2de1_yQiWm8TC_gDPr2Sy4V-TB3_waGVFOgO34FfhFL4vhIO6FqPDC_7Qwggn5xExSPfDlL786c6Lttw?testcase_id=6723049080750080 Filer: thestig See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
,
Jul 18 2016
Suspected CLs No CL in the regression range changes the crashed files. The result is the blame information. Author: reed@android.com Project: chromium-skia Changelist: https://chromium.googlesource.com/skia.git/+/8a1c16ff38322f0210116fa7293eb8817c7e477e Time: Wed Dec 17 15:59:43 2008 The CL last changed line 78 of file SkRect.h, which is stack frame 0. Author: reed Project: chromium-skia Changelist: https://chromium.googlesource.com/skia.git/+/157f36d358814a2235aa6284b78a67b725076063 Time: Wed Oct 15 14:05:09 2014 The CL last changed line 80 of file SkRect.h, which is stack frame 1. Author: mtklein Project: chromium-skia Changelist: https://chromium.googlesource.com/skia.git/+/feaadee1c38e1d4e1ec0069a3509ef6fbc5fbeff Time: Wed Apr 08 18:25:48 2015 The CL last changed line 645 of file SkCanvas.cpp, which is stack frame 2. Author: mtklein Project: chromium-skia Changelist: https://chromium.googlesource.com/skia.git/+/feaadee1c38e1d4e1ec0069a3509ef6fbc5fbeff Time: Wed Apr 08 18:25:48 2015 The CL last changed line 59 of file SkRecorder.cpp, which is stack frame 3. Author: mtklein Project: chromium-skia Changelist: https://chromium.googlesource.com/skia.git/+/d711d115d28b9838303dcc232516aa2f552f3a2a Time: Wed Jul 01 14:04:37 2015 The CL last changed line 45 of file SkPictureRecorder.cpp, which is stack frame 4. Author: fmalita Project: chromium Changelist: https://chromium.googlesource.com/chromium/src//+/6f694423303b95fedc99848ad436817e169f022c Time: Thu Mar 31 02:49:24 2016 The CL last changed line 269 of file GraphicsContext.cpp, which is stack frame 5. Author: ortuno Project: chromium Changelist: https://chromium.googlesource.com/chromium/src//+/3fd03b556675834c683e1453a168a874269b83fc Time: Wed Jun 15 18:10:34 2016 The CL last changed line 256 of file optional.h, which is stack frame 6. Suspected Project: chromium-skia Suspected Component: Internals>Skia Possible suspect : https://chromium.googlesource.com/skia.git/+/feaadee1c38e1d4e1ec0069a3509ef6fbc5fbeff Please reassign if this is not related to your change.
,
Jul 18 2016
Looks like either we've either got an unsorted SkRect (bottom -2147483648, top 204) or we've roundedOut() into that situation when turning it into an SkIRect.
Will try something like,
static SkIRect safe_round_out(SkRect bounds) {
// Do we need / want bounds.sort() ?
if (bounds.intersect(SkIRect::MakeLargest())) {
return bounds.roundOut();
}
return SkIRect::MakeEmpty();
}
Do we already have anything like that? Didn't notice one.
,
Jul 18 2016
off-hand, its easy to make a height overflow: top = -max_int bottom = max_int height = bottom - top
,
Jul 18 2016
I've got the same issue basically in Blink's IntRect due to an in-range x() added to and in-range width() to get an out-of-range right edge position. https://bugs.chromium.org/p/chromium/issues/detail?id=628870 It seems like it's not a problem because negative values in integers are fine, at least for us. Checking for bounds on every integer operation will kill performance, so I want to suppress these ubsan warnings. Is Skia in the same situation?
,
Jul 18 2016
It's possible, but I'm not sure. I'd like to continue digging into this tomorrow with a debugger. This might be one of either of these two cases: - large and inverted float rect that could round out to a large inverted int rect - large and non-inverted float rect than cannot round out to int rect (it's not guaranteed, but on x86 typically out of range float->int casts produce -2147483648 like we see here) Mike's right that the realistic limits for SkIRect are a lot lower than the types make them out to be, and in the context of a device-space clip for an SkPicture, they're even lower than that. We are not yet in a technological position to support an SkPicture describing a petapixel space. I suspect that code snippet in #3 is still going to be the right approach, perhaps with MakeLargest() replaced with something like MakeSaneClipUpperLimit().
,
Jul 28 2016
ClusterFuzz has detected this issue as fixed in range 407167:408336. Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6723049080750080 Fuzzer: miaubiz_svg_fuzzer Job Type: linux_ubsan_chrome Platform Id: linux Crash Type: Integer-overflow Crash Address: Crash State: SkIRect::height SkCanvas::resetForNextPicture SkRecorder::reset Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_ubsan_chrome&range=370022:370027 Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_ubsan_chrome&range=407167:408336 Minimized Testcase (1.58 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97vGl1zMMvk4K0owrSC5w1mv1km6ijMBFpssJBT-gi1a_CznA3MquaiEqEcvmIaEyEU3ITaHvnL7H2de1_yQiWm8TC_gDPr2Sy4V-TB3_waGVFOgO34FfhFL4vhIO6FqPDC_7Qwggn5xExSPfDlL786c6Lttw?testcase_id=6723049080750080 See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Jul 28 2016
ClusterFuzz testcase is verified as fixed, closing issue. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Nov 22 2016
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 |
|||||
Comment 1 by thestig@chromium.org
, Jul 16 2016