Use-of-uninitialized-value in media::DecoderBuffer::timestamp |
||||||||||||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=5471674945503232 Fuzzer: libFuzzer_mediasource_MP4_AACLC_AVC_pipeline_integration_fuzzer Job Type: libfuzzer_chrome_msan Platform Id: linux Crash Type: Use-of-uninitialized-value Crash Address: Crash State: media::DecoderBuffer::timestamp media::SourceBufferRange::UpdateEndTime media::SourceBufferRangeByPts::UpdateEndTimeUsingLastGOP Sanitizer: memory (MSAN) Recommended Security Severity: Medium Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=506132:506173 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5471674945503232 Issue filed automatically. See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
,
Oct 4 2017
This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it. If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 4 2017
,
Oct 4 2017
wolenetz@, could you please take a look? Any chance that this was caused by https://chromium.googlesource.com/chromium/src/+/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f ?
,
Oct 4 2017
This code path is enabled when (disabled by default) feature MseBufferByPts is enabled. Regardless, I'll investigate and get an appropriate fix done for M-63.
,
Oct 4 2017
Note to self: same fuzzer case, but with debug asan, also gives a crash.
,
Oct 9 2017
With --dcheck_always_on, even the msan repro hits the same error as debug asan did (before hitting the msan repro itself). So I'll work on the DCHECK failure first, then see if that is the root cause of the msan issue.
,
Oct 9 2017
It looks like the enforcement of keys ordered-by-PTS in SBRByts's |keyframe_map_| can break the assumption of ordered-by-buffers-index in some (bad) input sequences like: A continuous coded frame group (determined by DTS sequence per spec and impl) with keyframe PTS out-of-order. With some extra logging and a detailed repro case (which I'll simplify before landing a fix): [26995:26995:1009/142104.151532:1482669340140:VERBOSE1:source_buffer_range_by_pts.cc(596)] MDW keyframe map: index base=0, bufcount=1 [26995:26995:1009/142104.151572:1482669340179:VERBOSE1:source_buffer_range_by_pts.cc(598)] pts 101000, idx = 0 [26995:26995:1009/142104.151609:1482669340216:VERBOSE1:source_buffer_range_by_pts.cc(598)] pts 121000, idx = 2 [26995:26995:1009/142104.151645:1482669340253:VERBOSE1:source_buffer_range_by_pts.cc(598)] pts 132000, idx = 3 [26995:26995:1009/142104.151682:1482669340289:VERBOSE1:source_buffer_range_by_pts.cc(598)] pts 153000, idx = 5 [26995:26995:1009/142104.151719:1482669340326:VERBOSE1:source_buffer_range_by_pts.cc(598)] pts 9000000, idx = 4
,
Oct 9 2017
,
Oct 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bb7a79b020601682553fa8ac7ab94be627e9dc58 commit bb7a79b020601682553fa8ac7ab94be627e9dc58 Author: Matt Wolenetz <wolenetz@chromium.org> Date: Mon Oct 09 22:17:23 2017 MSE: Improve timestamp granularity in debug logs This change switches from InSecondsF() to InMicroseconds() in DVLOG and DCHECK within FrameProcessor and SourceBufferStream. This helps debug fuzzer scenarios, like bug 771482 , where timestamp differences in the low microseconds are hidden when using InSecondsF(). BUG= 771482 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: Ie52fb4bc104804928dc25ace40c8dde9a38fa79f Reviewed-on: https://chromium-review.googlesource.com/707311 Reviewed-by: Dan Sanders <sandersd@chromium.org> Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org> Cr-Commit-Position: refs/heads/master@{#507508} [modify] https://crrev.com/bb7a79b020601682553fa8ac7ab94be627e9dc58/media/filters/frame_processor.cc [modify] https://crrev.com/bb7a79b020601682553fa8ac7ab94be627e9dc58/media/filters/source_buffer_stream.cc
,
Oct 10 2017
,
Oct 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5356f74e8106352af92d016155f61d6f629c172f commit 5356f74e8106352af92d016155f61d6f629c172f Author: Matt Wolenetz <wolenetz@chromium.org> Date: Thu Oct 12 05:06:25 2017 MSE: Improve SBS and SBRByPts DEBUG logging detail Adds verbose debug logging in SourceBufferStream::Append() and in most SBRByPts methods. BUG= 771482 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: Ief09a681e1a3bba0a8d52d99db7653b116ba5226 Reviewed-on: https://chromium-review.googlesource.com/714238 Reviewed-by: Dan Sanders <sandersd@chromium.org> Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org> Cr-Commit-Position: refs/heads/master@{#508257} [modify] https://crrev.com/5356f74e8106352af92d016155f61d6f629c172f/media/filters/source_buffer_range_by_pts.cc [modify] https://crrev.com/5356f74e8106352af92d016155f61d6f629c172f/media/filters/source_buffer_range_by_pts.h [modify] https://crrev.com/5356f74e8106352af92d016155f61d6f629c172f/media/filters/source_buffer_stream.cc
,
Oct 18 2017
,
Oct 24 2017
For more information, please see https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md. The link referenced in the description is no longer valid.
,
Oct 30 2017
[Bulk Edit] URGENT - PTAL. M63 Stable promotion is coming soon and your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP. Thank you.
,
Nov 1 2017
ClusterFuzz testcase 5113306363985920 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Nov 1 2017
,
Nov 1 2017
,
Nov 3 2017
+awhalley@, PTAL and expedite the fix if it is indeed M63 Stable blocker. Thank you.
,
Nov 6 2017
M63 Stable promotion is coming soon and your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge to M63 ASAP. Thank you.
,
Nov 10 2017
per mail with wolenetz@, this code won't be enabled by default or Finch before 64, so no need to merge to 63. Removing release blocker label.
,
Nov 10 2017
This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it. If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/469d91812fee7b2db7fd55d55bcb7663784c507d commit 469d91812fee7b2db7fd55d55bcb7663784c507d Author: Matt Wolenetz <wolenetz@chromium.org> Date: Tue Nov 14 03:28:27 2017 MSE: If timestampOffset change jumps far enough forward, introduce gap When buffering by either PTS or DTS, and in segments AppendMode, if the web app sets timestampOffset such that the next coded frame for a track emitted from the coded frame processing algorithm is far enough in the future, a buffered range gap can be introduced. This change makes similar behavior occur when in sequence AppendMode. Prior to this change, the intent of sequence mode was assumed to be to not introduce such gaps when jumping forwards due to a newly set timestampOffset. However, such behavior is neither explicit nor required in the MSE spec, and implementing it, especially when buffering compliantly ByPts, is complex and unintuitive. For example, should previously buffered frames in the "gap" be removed? Worse, the previous implementation used an implicit expansion of our buffering adjacency "fudge room" to coalesce such a gap in sequence mode when buffering by DTS, using inter-frame DTS deltas. The net of this change also fixes one source of SBRByPts range adjacency CHECK failures. Subsequent changes will address other sources of related failures in the referenced bugs, below. BUG= 771482 , 771480 , 771473 , 771481 , 771539 , 781766 , 775967 TEST=Updated FrameProcessorTests and added LargeTimestampOffsetJumpForward Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: Iecc30c6ef6bf5c3f7433152cc62793806ecbd653 Reviewed-on: https://chromium-review.googlesource.com/757756 Reviewed-by: Dan Sanders <sandersd@chromium.org> Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org> Cr-Commit-Position: refs/heads/master@{#516169} [modify] https://crrev.com/469d91812fee7b2db7fd55d55bcb7663784c507d/media/filters/frame_processor.cc [modify] https://crrev.com/469d91812fee7b2db7fd55d55bcb7663784c507d/media/filters/frame_processor.h [modify] https://crrev.com/469d91812fee7b2db7fd55d55bcb7663784c507d/media/filters/frame_processor_unittest.cc
,
Nov 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d5f07c6849faa9a88e6358eb001de5a75443d791 commit d5f07c6849faa9a88e6358eb001de5a75443d791 Author: Matt Wolenetz <wolenetz@chromium.org> Date: Wed Nov 15 20:43:05 2017 MSE: When buffering ByPts, handle keyframe with PTS < previous keyframe If FrameProcessor doesn't signal SourceBufferStream of a new coded frame group when buffering ByPts and an otherwise continuous (in DTS) frame sequence has keyframe PTS that does not increase, then code faults are hit. Specifically, A) the underlying SourceBufferRangeByPts could have its keyframe map become out of sequence with respect to the buffers it indexes, violating assumptions of its ordering, and B) internal adjacency checks for the new frame with the previous append can fail. This change adds such signalling to FrameProcessor to avoid these faults when keyframe PTS sequence decreases in a CFG when buffering ByPts. Note that "deep" SAP-Type-2 sequences can still trigger (B), tracked by bug 773115 . TEST=New test cases' ProcessFrames calls triggered A or B previously: SegmentsModeNewByPts/FrameProcessorTest.OOOKeyframePts_1/0 (Hit A) SequenceModeNewByPts/FrameProcessorTest.OOOKeyframePts_1/0 (Hit A)* SegmentsModeNewByPts/FrameProcessorTest.OOOKeyframePts_2/0 (Hit B) SequenceModeNewByPts/FrameProcessorTest.OOOKeyframePts_2/0 (Hit B) * - this test also triggered (B) prior to landing prereq CL 469d9181 BUG= 771482 , 771480 , 771473 , 771481 , 771539 , 781766 , 775967 , 773115 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I80902f3ce1c0a9262459df0a1092d6bb5cbc400f Reviewed-on: https://chromium-review.googlesource.com/728860 Reviewed-by: Dan Sanders <sandersd@chromium.org> Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org> Cr-Commit-Position: refs/heads/master@{#516814} [modify] https://crrev.com/d5f07c6849faa9a88e6358eb001de5a75443d791/media/filters/frame_processor.cc [modify] https://crrev.com/d5f07c6849faa9a88e6358eb001de5a75443d791/media/filters/frame_processor_unittest.cc [modify] https://crrev.com/d5f07c6849faa9a88e6358eb001de5a75443d791/media/filters/source_buffer_range.cc
,
Nov 16 2017
ClusterFuzz has detected this issue as fixed in range 516799:516837. Detailed report: https://clusterfuzz.com/testcase?key=5471674945503232 Fuzzer: libFuzzer_mediasource_MP4_AACLC_AVC_pipeline_integration_fuzzer Job Type: libfuzzer_chrome_msan Platform Id: linux Crash Type: Use-of-uninitialized-value Crash Address: Crash State: media::DecoderBuffer::timestamp media::SourceBufferRange::UpdateEndTime media::SourceBufferRangeByPts::UpdateEndTimeUsingLastGOP Sanitizer: memory (MSAN) Recommended Security Severity: Medium Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=506132:506173 Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=516799:516837 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5471674945503232 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 16 2017
wolenetz: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers? If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one? If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 30 2017
wolenetz: Is this bug fixed now? Also, does it affect platforms other than just Linux and macOS? (It seems to be in platform-independent code.)
,
Dec 1 2017
wolenetz: Uh oh! This issue still open and hasn't been updated in the last 29 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers? If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one? If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 5 2017
This bug appears to be fixed. I can no longer repro on ToT. I'll follow-up individually on the various other bugs that were duplicated into this one.
,
Dec 5 2017
ClusterFuzz has detected this issue as fixed in range 516799:516837. Detailed report: https://clusterfuzz.com/testcase?key=5471674945503232 Fuzzer: libFuzzer_mediasource_MP4_AACLC_AVC_pipeline_integration_fuzzer Job Type: libfuzzer_chrome_msan Platform Id: linux Crash Type: Use-of-uninitialized-value Crash Address: Crash State: media::DecoderBuffer::timestamp media::SourceBufferRange::UpdateEndTime media::SourceBufferRangeByPts::UpdateEndTimeUsingLastGOP Sanitizer: memory (MSAN) Recommended Security Severity: Medium Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=506132:506173 Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=516799:516837 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5471674945503232 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.
,
Dec 5 2017
@#27: Yes, this is fixed now. It's also in an off-by-default feature (kMseBufferByPts), for which experimentation may begin soon in M64 canary/dev. Yes, it is cross-platform.
,
Dec 6 2017
@#31, I meant M65...
,
Mar 14 2018
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 27 2018
,
Mar 31 2018
|
||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||
Comment 1 by sheriffbot@chromium.org
, Oct 4 2017