Skip to content
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

Sanitise jpgc box size before memory allocation #1019

Closed
wants to merge 1 commit into from

Conversation

lovell
Copy link
Contributor

@lovell lovell commented Oct 30, 2023

I think the work for #922 may have removed some existing security checks around memory allocation.

EDIT: The jpgC logic was introduced in commit 3e82d51 (and #922 moved it).

This PR adds sanity checking for jpgC box sizes and a 32 byte test file discovered via fuzz testing that will attempt to allocate ~9 exabytes and therefore segfault when parsed using v1.17.x. I've not checked other box types, but it's possible this sanity check may need to be applied in a more general manner to other parse functions.

@lovell
Copy link
Contributor Author

lovell commented Nov 1, 2023

It looks like this was fixed via commit 8ff8227

@farindk
Copy link
Contributor

farindk commented Nov 2, 2023

It looks like this was fixed via commit 8ff8227

Yes. Sorry for the double work. I also saw this is a fuzzer output.

@farindk farindk closed this Nov 2, 2023
@lovell lovell deleted the sanitise-jpgc-box-size branch November 2, 2023 10:42
@farindk
Copy link
Contributor

farindk commented Nov 2, 2023

it's possible this sanity check may need to be applied in a more general manner to other parse functions.

Right, this all started because of the changed handling of boxes with zero size field (undefined size). That should probably be handled in a more general way. I've opened #1021 for this.

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