Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1808123004:
[libfuzzer] Add a sanity check for too big images in libpng_read_fuzzer. (Closed)
Description[libfuzzer] Add a sanity check for too big images in libpng_read_fuzzer.
This change has been mirrored from google3 png fuzzer. Should prevent some timeouts.
R=aizatsky@chromium.org, inferno@chromium.org, kcc@chromium.org, krasin@chromium.org
BUG=584819
TBR=aizatsky@chromium.org
Committed: https://crrev.com/47be097348e455997215e3504358dcf114b32d64
Cr-Commit-Position: refs/heads/master@{#381749}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Replace multiply with division to avoid overflow. #
Total comments: 1
Messages
Total messages: 24 (11 generated)
Description was changed from ========== [libfuzzer] Add a sanity check for too big images in libpng_read_fuzzer. This change for mirrored from google3 png fuzzer. Should prevent some timeouts. R=aizatsky@chromium.org, inferno@chromium.org, kcc@chromium.org, krasin@chromium.org BUG=584819 ========== to ========== [libfuzzer] Add a sanity check for too big images in libpng_read_fuzzer. This change has been mirrored from google3 png fuzzer. Should prevent some timeouts. R=aizatsky@chromium.org, inferno@chromium.org, kcc@chromium.org, krasin@chromium.org BUG=584819 ==========
Description was changed from ========== [libfuzzer] Add a sanity check for too big images in libpng_read_fuzzer. This change has been mirrored from google3 png fuzzer. Should prevent some timeouts. R=aizatsky@chromium.org, inferno@chromium.org, kcc@chromium.org, krasin@chromium.org BUG=584819 ========== to ========== [libfuzzer] Add a sanity check for too big images in libpng_read_fuzzer. This change has been mirrored from google3 png fuzzer. Should prevent some timeouts. R=aizatsky@chromium.org, inferno@chromium.org, kcc@chromium.org, krasin@chromium.org BUG=584819 TBR=aizatsky@chromium.org ==========
krasin@google.com changed reviewers: + krasin@google.com
https://codereview.chromium.org/1808123004/diff/1/testing/libfuzzer/fuzzers/l... File testing/libfuzzer/fuzzers/libpng_read_fuzzer.cc (right): https://codereview.chromium.org/1808123004/diff/1/testing/libfuzzer/fuzzers/l... testing/libfuzzer/fuzzers/libpng_read_fuzzer.cc:95: if (height * width > 100000000) what if height * width overflow?
inferno@chromium.org changed reviewers: - krasin@google.com
The CQ bit was checked by inferno@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1808123004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1808123004/1
The CQ bit was unchecked by krasin@google.com
The CQ bit was unchecked by aarya@google.com
On 2016/03/17 17:02:58, commit-bot: I haz the power wrote: > CQ is trying da patch. Follow status at > https://chromium-cq-status.appspot.com/patch-status/1808123004/1 > View timeline at > https://chromium-cq-status.appspot.com/patch-timeline/1808123004/1 We should address Ivan's comment before committing. Move the multiply to a divide on right side.
On 2016/03/17 17:04:20, aarya wrote: > On 2016/03/17 17:02:58, commit-bot: I haz the power wrote: > > CQ is trying da patch. Follow status at > > https://chromium-cq-status.appspot.com/patch-status/1808123004/1 > > View timeline at > > https://chromium-cq-status.appspot.com/patch-timeline/1808123004/1 > > We should address Ivan's comment before committing. Move the multiply to a > divide on right side. Good point. We can also cast height to uint64_t
On 2016/03/17 17:02:48, krasin wrote: > https://codereview.chromium.org/1808123004/diff/1/testing/libfuzzer/fuzzers/l... > File testing/libfuzzer/fuzzers/libpng_read_fuzzer.cc (right): > > https://codereview.chromium.org/1808123004/diff/1/testing/libfuzzer/fuzzers/l... > testing/libfuzzer/fuzzers/libpng_read_fuzzer.cc:95: if (height * width > > 100000000) > what if height * width overflow? Thanks for noticing this!
The CQ bit was checked by mmoroz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from inferno@chromium.org Link to the patchset: https://codereview.chromium.org/1808123004/#ps20001 (title: "Replace multiply with division to avoid overflow.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1808123004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1808123004/20001
Message was sent while issue was closed.
Description was changed from ========== [libfuzzer] Add a sanity check for too big images in libpng_read_fuzzer. This change has been mirrored from google3 png fuzzer. Should prevent some timeouts. R=aizatsky@chromium.org, inferno@chromium.org, kcc@chromium.org, krasin@chromium.org BUG=584819 TBR=aizatsky@chromium.org ========== to ========== [libfuzzer] Add a sanity check for too big images in libpng_read_fuzzer. This change has been mirrored from google3 png fuzzer. Should prevent some timeouts. R=aizatsky@chromium.org, inferno@chromium.org, kcc@chromium.org, krasin@chromium.org BUG=584819 TBR=aizatsky@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [libfuzzer] Add a sanity check for too big images in libpng_read_fuzzer. This change has been mirrored from google3 png fuzzer. Should prevent some timeouts. R=aizatsky@chromium.org, inferno@chromium.org, kcc@chromium.org, krasin@chromium.org BUG=584819 TBR=aizatsky@chromium.org ========== to ========== [libfuzzer] Add a sanity check for too big images in libpng_read_fuzzer. This change has been mirrored from google3 png fuzzer. Should prevent some timeouts. R=aizatsky@chromium.org, inferno@chromium.org, kcc@chromium.org, krasin@chromium.org BUG=584819 TBR=aizatsky@chromium.org Committed: https://crrev.com/47be097348e455997215e3504358dcf114b32d64 Cr-Commit-Position: refs/heads/master@{#381749} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/47be097348e455997215e3504358dcf114b32d64 Cr-Commit-Position: refs/heads/master@{#381749}
Message was sent while issue was closed.
https://codereview.chromium.org/1808123004/diff/20001/testing/libfuzzer/fuzze... File testing/libfuzzer/fuzzers/libpng_read_fuzzer.cc (right): https://codereview.chromium.org/1808123004/diff/20001/testing/libfuzzer/fuzze... testing/libfuzzer/fuzzers/libpng_read_fuzzer.cc:95: if (height > 100000000 / width) Sorry for not catching up in time, but what if width == 0?
Message was sent while issue was closed.
On 2016/03/17 18:25:02, krasin1 wrote: > https://codereview.chromium.org/1808123004/diff/20001/testing/libfuzzer/fuzze... > File testing/libfuzzer/fuzzers/libpng_read_fuzzer.cc (right): > > https://codereview.chromium.org/1808123004/diff/20001/testing/libfuzzer/fuzze... > testing/libfuzzer/fuzzers/libpng_read_fuzzer.cc:95: if (height > 100000000 / > width) > Sorry for not catching up in time, but what if width == 0? Oh, my bad, I shouldn't hurry up with that. Thanks!
Message was sent while issue was closed.
On 2016/03/18 08:22:44, mmoroz wrote: > On 2016/03/17 18:25:02, krasin1 wrote: > > > https://codereview.chromium.org/1808123004/diff/20001/testing/libfuzzer/fuzze... > > File testing/libfuzzer/fuzzers/libpng_read_fuzzer.cc (right): > > > > > https://codereview.chromium.org/1808123004/diff/20001/testing/libfuzzer/fuzze... > > testing/libfuzzer/fuzzers/libpng_read_fuzzer.cc:95: if (height > 100000000 / > > width) > > Sorry for not catching up in time, but what if width == 0? > > Oh, my bad, I shouldn't hurry up with that. Thanks! Fortunately, png_check_IHDR() doesn't accept width or height of 0, but I've uploaded additional condition: https://codereview.chromium.org/1809383002. |
