-
Notifications
You must be signed in to change notification settings - Fork 7
VPLAY-12333 mp4demux hardening #830
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 15 commits
8867965
562b881
789a87e
7309613
1137d11
7250d01
dfb2427
6d80135
0cf5294
b008810
1762ff5
107adc9
74227ca
bb87599
9befbf7
4379972
b5db7e0
8eb2b65
c88341c
19c71f4
45ff94d
e485800
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -79,18 +79,20 @@ enum mp4LogLevel | |||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| enum Mp4ParseError | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| MP4_PARSE_OK = 0, /**< No error */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_INVALID_BOX, /**< Invalid box encountered */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_INVALID_CONSTANT_IV_SIZE, /**< Invalid constant IV size */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_SAMPLE_COUNT_MISMATCH, /**< Sample count mismatch */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_UNSUPPORTED_ENCRYPTION_SCHEME, /**< Invalid auxiliary info type */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_MISSING_DATA_OFFSET, /**< Missing data offset in TRUN */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_INVALID_PADDING, /**< Invalid padding value */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_UNSUPPORTED_SAMPLE_ENTRY_COUNT,/**< Unsupported sample entry count */ | ||||||||||||||||||||||||||
| MP4_PARSE_OK, /**< No error */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_INVALID_BOX, /**< Invalid box header size */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_INVALID_IV_SIZE, /**< Invalid IV size (expected 8 or 16) */ | ||||||||||||||||||||||||||
pstroffolino marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||
| MP4_PARSE_ERROR_SAMPLE_COUNT_MISMATCH, /**< Explicit sample count doesn't match implicit sample count */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_UNSUPPORTED_ENCRYPTION_SCHEME, /**< Expected cenc or cbcs */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_INVALID_PADDING, /**< Unexpected Video Padding field (should be 0xffff) */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_UNSUPPORTED_SAMPLE_ENTRY_COUNT,/**< Zero sample entry count */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_UNSUPPORTED_STREAM_FORMAT, /**< Unsupported stream format */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_INVALID_ESDS_TAG, /**< Invalid ESDS tag */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_DATA_BOUNDARY_MISMATCH, /**< Data boundary mismatch */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_INVALID_INPUT /**< Invalid input to parse function */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_DATA_BOUNDARY_MISMATCH, /**< Data boundary mismatch - referencing invalid memory */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_INVALID_INPUT, /**< Invalid input to parse function; nullptr or zero length */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_INVALID_KID, /**< Invalid (huge) kidCount */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_INVALID_ENTRY_COUNT, /**< Entry count is zero */ | ||||||||||||||||||||||||||
| MP4_PARSE_ERROR_VARIABLE_LENGTH_OVERFLOW /**< Value encoded using octets exceed 32 bits */ | ||||||||||||||||||||||||||
pstroffolino marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
pstroffolino marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
|
|
@@ -143,6 +145,10 @@ class Mp4Demux | |||||||||||||||||||||||||
| // Parser state | ||||||||||||||||||||||||||
| const uint8_t *moofPtr; /**< Base address for sample data */ | ||||||||||||||||||||||||||
| const uint8_t *ptr; /**< Current parser position */ | ||||||||||||||||||||||||||
| const uint8_t *endPtr; /**< Absolute end boundary of the current parse buffer */ | ||||||||||||||||||||||||||
| // MDAT range tracking (for sample data validation) | ||||||||||||||||||||||||||
| const uint8_t *mdatStart; /**< Pointer to the first byte of the payload of the current or most recently parsed 'mdat' box. Set when an 'mdat' box is parsed and used as the lower bound when validating that computed sample data offsets and sizes remain within the available payload range. */ | ||||||||||||||||||||||||||
| const uint8_t *mdatEnd; /**< Pointer to one past the last byte of the payload of the current or most recently parsed 'mdat' box. Set together with mdatStart during 'mdat' parsing and used as the upper bound during sample data bounds checking to ensure no read extends beyond the validated 'mdat' payload. */ | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Box header fields | ||||||||||||||||||||||||||
| uint8_t version; /**< Box version */ | ||||||||||||||||||||||||||
|
|
@@ -167,6 +173,12 @@ class Mp4Demux | |||||||||||||||||||||||||
| MediaCodecInfo codecInfo; /**< Codec information */ | ||||||||||||||||||||||||||
| Mp4ParseError parseError; /**< Current parse error state */ | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * @brief log human readable parse error and update state | ||||||||||||||||||||||||||
| * @param parseError one of Mp4ParseError | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| void setParseError( Mp4ParseError ); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * @brief Read n bytes from current position in big-endian format | ||||||||||||||||||||||||||
| * @param n Number of bytes to read | ||||||||||||||||||||||||||
|
|
@@ -321,7 +333,7 @@ class Mp4Demux | |||||||||||||||||||||||||
| * @brief Read length field with variable encoding | ||||||||||||||||||||||||||
| * @return Length value | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| * @return Length value | |
| * | |
| * This method reads a length field that is encoded using a variable-length | |
| * scheme. The returned value is always non-negative and represented as | |
| * an unsigned 32-bit integer. | |
| * | |
| * @return Length value as uint32_t | |
| * | |
| * @note The return type of this method was changed from int to uint32_t | |
| * to improve type safety (lengths are never negative). Callers | |
| * should ensure they do not rely on negative values and avoid | |
| * unintended narrowing conversions to signed types. |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ReadLen function's return type changes from int to uint32_t, which is a breaking API change. While this is an improvement (length should be unsigned), any calling code that stores the result in an int variable may experience issues. Ensure all callers have been reviewed and updated appropriately.
| uint32_t ReadLen(); | |
| int ReadLen(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed explicit initializer (= 0) for the first enum value. While this is technically correct (first enum value defaults to 0), removing the explicit initialization changes the existing pattern and could be considered a breaking change if external code depends on this specific pattern for compatibility checks. For consistency with the previous explicit initialization, consider keeping "MP4_PARSE_OK = 0".