Repository navigation
Fix heap over-read of truncated display-matrix side-data in FFmpegAssetTrack - #930
Open
YuriNachos wants to merge 1 commit into
Open
YuriNachos wants to merge 1 commit into
YuriNachos wants to merge 1 commit into
Conversation
…etTrack av_display_rotation_get dereferences a 3x3 matrix of 9 x Int32 (36 bytes) unconditionally, but the AV_PKT_DATA_DISPLAYMATRIX side-data branch read it with no sideData.size check. FFmpeg normally allocates exactly 36 bytes, but a malformed/cropped MKV or MP4 can attach a shorter allocation, over-reading adjacent heap. PR kingslay#908 hardened only the rotation result (isFinite), not the read length. Guard the length before the read via a testable displayRotation(sideDataSize:matrix:) helper that returns nil unless sideDataSize >= 9 * MemoryLayout<Int32>.size, and continue past the side-data when it does. The track still initialises and rotation keeps its default 0; the existing isFinite NaN path is untouched, so well-formed streams are unchanged. Adds DisplayMatrixRotationTest covering size 36 (identity -> 0, 90deg matrix -> +90) and size 35 / 0 (nil, no read).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
av_display_rotation_getdereferences a 3×3 matrix of 9 ×Int32(36 bytes) unconditionally, but theAV_PKT_DATA_DISPLAYMATRIXside-data branch read it with no length check. FFmpeg emits exactly 36 bytes for a well-formed display matrix, but a malformed/cropped container can hand us a shorter allocation, so the read over-runs into adjacent heap. PR #908 hardened only the rotation result (isFinite), not the read itself — the over-read still happens beforeisFiniteis evaluated.Fix
Extract a
displayRotation(sideDataSize:matrix:)helper that returnsnilunlesssideDataSize >= 9 * MemoryLayout<Int32>.size, andcontinuepast the side-data when it does. The track still initialises and rotation keeps its default0; behaviour is unchanged for well-formed streams. Different file/line from #928 (audionominalFrameRate) and the open #927 (FFmpegDecode.swift).Test plan
swift test— 13 passed, 0 failed, including a newDisplayMatrixRotationTestcoveringsize == 36(identity matrix → rotation 0; 90° matrix → finite, in range) andsize < 36(truncated → no read / nil). No signing/device needed.