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

validate filesize before allocating chunk memory #1161

Conversation

peterhillman
Copy link
Contributor

Address https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=39332
Prevent excessive memory allocation by ensuring file is large enough to contain chunk table before allocating memory

Signed-off-by: Peter Hillman [email protected]

@@ -70,6 +70,17 @@ extract_chunk_table (
return ctxt->report_error (
ctxt, EXR_ERR_INVALID_ARGUMENT, "Invalid file with no chunks");

if (chunkbytes + chunkoff > (uint64_t) ctxt->file_size)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi, can't remember but I do allow a stream implementation to return -1 if they do not know the file size, which should then return false and succeed for this, but just thought I'd mention it so you know - I have been a bit pedantic about checking that elsewhere, although the math here is pretty simple...

@cary-ilm cary-ilm merged commit 35b6908 into AcademySoftwareFoundation:master Sep 28, 2021
@lgritz
Copy link
Contributor

lgritz commented Sep 29, 2021

Sorry to report this, but last night's OIIO "bleeding edge" CI test that uses openexr master has a number of "EXR Error (attrib2.exr): EXR_ERR_INVALID_ARGUMENT chunk table size (32) too big for file size (0)" for exr files that I'm sure are fine and that I've been using for tests forever.

@peterhillman
Copy link
Contributor Author

OK, I'll do the thing that @kdt3rd said to do. New PR coming.

cary-ilm pushed a commit to cary-ilm/openexr that referenced this pull request Oct 14, 2021
cary-ilm pushed a commit that referenced this pull request Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants