Badly formed files can cause `create_sample_table()` to OOM.
Nobody has claimed this yet.
Assessment
- Difficulty
- 5/5
- Estimated time
- Over a week
- Newbie friendliness
- 25/100
Research direction
Start by tracing create_sample_table() and the handling of the stsc and stsz boxes, then review how get_indices exposes parsed data. Compare the failure scenarios from Gecko Bugs 1661368 and 1814791, including mismatched or oversized sample counts. Done means preventing malformed files from causing arbitrary allocation or OOM while preserving valid-file parsing.
Written by the indexing model from the issue text.
Description
See Gecko Bug 1661368 and more recently Bug 1814791.
There is currently no validation of whether a file with an stsc box with a large sample count is actually referencing offsets that fall within the file, which allows create_sample_table() to allocate an arbitrarily large vector and OOM.
We could just set a configurable hard limit on the number of samples it can allocate, but I don't know if that's the best solution.
We can also fail early when the relevant sample boxes have mismatched sample counts, but I'm not sure if there are files that currently parse and break that rule, and as tnikkel pointed out, the fuzzer may still find a way to create a matching sample count that causes OOM. Might still be worth implementing this if it makes sense to fail that way.
I think the most ideal solution is instead to make create_sample_table() aware of the amount of data in the file that is currently available to the caller, and use the stsz box to determine how many samples the vector needs to allocate to reach EOF. For cases where the full size is not known beforehand, API users would have to call get_indices multiple times and be aware that their pointers will be invalid, but that doesn't seem like a difficult problem.
What I wonder is whether the available data is something that should be accessible from other parts of mp4parse as well, for example to allow the box parsing to indicate that it hit the end of the available data but can continue parsing when more data is available.
Feel free to let me know if this idea doesn't make sense, we can use this issue to brainstorm more solutions if necessary.
- Dominant language
- Rust
- Stars
- 448
- Forks
- 72
- Avg merge
- 6d 4h
- Merged PRs (30d)
- 1
Getting set up
We have not checked this project's setup files yet. Start from its README, and see our first-contribution guide for the general steps.
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from mozilla/mp4parse-rust
-
Difficulty 4/5 3-5 days Newbie friendliness 48/100
mozilla/mp4parse-rust#444 · 5 comments ·
-
Difficulty 3/5 1-2 days Newbie friendliness 45/100
mozilla/mp4parse-rust#441 ·
-
senc boxOpen
Difficulty 5/5 Over a week Newbie friendliness 15/100
mozilla/mp4parse-rust#415 ·
-
Difficulty 4/5 3-5 days Newbie friendliness 28/100
mozilla/mp4parse-rust#414 · 4 comments ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 35/100
mozilla/mp4parse-rust#412 · 2 comments ·
All issues in mozilla/mp4parse-rust
Similar issues
-
type/bug
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
stackabletech/kafka-operator#1033 · 1 comment ·
Maintainers usually reply within 1 day
-
bug good first issue needs testing
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
Maintainers usually reply within 3 days
-
Difficulty 2/5 1-3 hours Newbie friendliness 90/100
farion1231/cc-switch#7744 · 1 comment ·
Maintainers usually reply within 1 day
-
datafusion
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
apache/iceberg-rust#3297 ·
Maintainers usually reply within 1 day