Integer-overflow in gsm_mult |
||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=6216462640611328 Fuzzer: libFuzzer_media_pipeline_integration_fuzzer Job Type: libfuzzer_chrome_ubsan Platform Id: linux Crash Type: Integer-overflow Crash Address: Crash State: gsm_mult filter_value short_term_synth Sanitizer: undefined (UBSAN) Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=423338:423416 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6216462640611328 Issue filed automatically. See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.
,
Nov 7 2017
I cannot look at this as i have no access to the reports details "You (email=michaelni@gmx.at) are not authorized to access this page!"
,
Nov 7 2017
../../third_party/ffmpeg/libavcodec/gsmdec_template.c:44:20: runtime error: signed integer overflow: 27443 * 84135 cannot be represented in type 'int' #0 0xbbf883 in gsm_mult third_party/ffmpeg/libavcodec/gsmdec_template.c:44:20 #1 0xbbfb6b in filter_value third_party/ffmpeg/libavcodec/gsmdec_template.c:77:27 #2 0xbbf6c2 in short_term_synth third_party/ffmpeg/libavcodec/gsmdec_template.c:107:18 #3 0xbbf0c0 in gsm_decode_block third_party/ffmpeg/libavcodec/gsmdec_template.c:148:5 #4 0xbbedc8 in ff_msgsm_decode_block third_party/ffmpeg/libavcodec/msgsmdec.c:35:11 #5 0xb73779 in gsm_decode_frame third_party/ffmpeg/libavcodec/gsmdec.c:96:15 #6 0x7d3974 in decode_simple_internal third_party/ffmpeg/libavcodec/decode.c:417:15 #7 0x7d3712 in decode_simple_receive_frame third_party/ffmpeg/libavcodec/decode.c:620:15 #8 0x7d0268 in decode_receive_frame_internal third_party/ffmpeg/libavcodec/decode.c:638:15 #9 0x7cfc94 in avcodec_send_packet third_party/ffmpeg/libavcodec/decode.c:678:15 Michael, thanks for checking, sorry about the sudden cc. Presumably you are just getting this reply via e-mail. I've attached the trace and repro. @pnangunoori please don't automatically cc folks from the third party projects as they don't have access to these tools. Alternatively, Michael if this is something you want to be notified of more quickly of/automatically I can look into the process to see if it's possible for you to get automated access to these reports.
,
Nov 7 2017
,
Nov 8 2017
gsmdec_template.c:44 is
return (int)(a * (SUINT)b + (1 << 14)) >> 15;
The SUINT there should be unsigned
I see in BUILD.gn that CHECKED is defined, this is a bad idea. CHECKED is intended for codec debuging. And it does that by making overflows vissible through integer overflows. In this case here the overflow would mean that somethng in the input bitstream contains a large enough value to exceed the range of the used data types. Which is useful to know to debug a file that doesnt look correct when decoded but not usefull here
about more quick notifies, sure, but i cannot promise how quick i can look into issues. So as long as this doesnt come with the expectation that i have to look into it "ASAP". (for example ATM ive a headache and would, if this was a real issue that required real debuging work not do it ATM)
,
Nov 8 2017
,
Nov 8 2017
Thanks Michael, I think that makes sense. Will chat with the folks who added CHECKED to see why it was done. Sure no expectations of response, we'll continue to look into these. I'll look into the process and see if there are options here.
,
Nov 9 2017
Dale, is there anything I should do?
,
Nov 9 2017
I left a comment on https://chromium-review.googlesource.com/#/c/734324/ asking if either you or Matt remember why we decided to do enable this flag. It seemed to have something to do with when FLAC+MP4 went through security review. +wolenetz too.
,
Nov 9 2017
Actually I guess it was before flac+mp4, came from issue 686513 and issue 640820. I think we just don't need to be setting this #define.
,
Nov 9 2017
,
Nov 9 2017
I don't recall why we had that flag. Thanks for the fix!
,
Nov 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/6891d931f3ed40103d62dc606ba7fd5ce2ef971c commit 6891d931f3ed40103d62dc606ba7fd5ce2ef971c Author: Dale Curtis <dalecurtis@chromium.org> Date: Thu Nov 09 18:25:15 2017 Don't enable integer overflow for SUINT; no point. We ship production builds with this defined to unsigned which is well-defined. It's just noise (per ffmpeg maintainers) to have ubsan triggering when we define SUINT to integer so that overflow is undefined and caught. Per maintainer this is useful for codec debugging, but not for anything else. BUG= 782118 ,686513,640820 Change-Id: Ib51a94da52cd95a917817da59dffd049b0da9d96 Reviewed-on: https://chromium-review.googlesource.com/759698 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> [modify] https://crrev.com/6891d931f3ed40103d62dc606ba7fd5ce2ef971c/BUILD.gn
,
Nov 10 2017
ClusterFuzz has detected this issue as fixed in range 515364:515426. Detailed report: https://clusterfuzz.com/testcase?key=6216462640611328 Fuzzer: libFuzzer_media_pipeline_integration_fuzzer Job Type: libfuzzer_chrome_ubsan Platform Id: linux Crash Type: Integer-overflow Crash Address: Crash State: gsm_mult filter_value short_term_synth Sanitizer: undefined (UBSAN) Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=423338:423416 Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=515364:515426 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6216462640611328 See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Nov 10 2017
ClusterFuzz testcase 6216462640611328 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by pnangunoori@chromium.org
, Nov 7 2017Components: Blink>Media
Labels: M-63 Test-Predator-Wrong