Skip to content

Conversation

jessa0
Copy link
Contributor

@jessa0 jessa0 commented Jan 8, 2023

The subtraction overflow will cause a panic when overflow panics are enabled. It seems more robust to return an error instead. The 64-bit box size can only be this small in malformed files, but I'm working on a project to parse and sanitize untrusted MP4 using this library, and hopefully panics can be reserved for code bugs rather than malformed input.

@jessa0
Copy link
Contributor Author

jessa0 commented Jan 8, 2023

It also seems that a largesize of 8 will be conflated with the size 0 meaning "box data extends to EOF".. I'm pushing a potential solution to that, where largesize of 0 still indicates data till EOF, otherwise largesize smaller than 16 is rejected as malformed (as it's shorter than the length of the header itself).

@jessa0 jessa0 force-pushed the jessa0/box-size-overflow branch from 0483069 to 76987c9 Compare January 8, 2023 22:54
@alfg alfg merged commit 16c1b5d into alfg:master Jan 10, 2023
@alfg
Copy link
Owner

alfg commented Jan 10, 2023

Looks good. Thank you!

It seems more robust to return an error instead
Agreed!

jprochazk pushed a commit to jprochazk/mp4 that referenced this pull request Sep 18, 2024
* don't subtract overflow when 64-bit box size is less than 8

* fix largesize of 8 being conflated with a size of 0
CandleCandle pushed a commit to CandleCandle/mp4-rust that referenced this pull request May 7, 2025
* don't subtract overflow when 64-bit box size is less than 8

* fix largesize of 8 being conflated with a size of 0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants