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

Strengthen integer overflow check in opj_pi_create_decode #825

Closed
wants to merge 13 commits into from
Closed

Strengthen integer overflow check in opj_pi_create_decode #825

wants to merge 13 commits into from

Conversation

trylab
Copy link
Contributor

@trylab trylab commented Sep 8, 2016

The crash still happens!!!

l_tcp->numlayers and l_step_l are both OPJ_UINT32 type variables. Thus using SIZE_MAX or ((size_t)-1) to check integer overflow is insufficient. We should use (OPJ_UINT32)-1 here.

trylab and others added 13 commits August 16, 2016 17:22
Prevent an integer overflow issue in function opj_pi_create_decode of
pi.c.
Making it more secure to call opj_calloc.
Remove header file limits.h
Replace OPJ_UINT32 with SIZE_MAX
clang 3.9 is currently unavailable for precise through apt
Ignore all files and directories which are generated by `cmake . && make`.

Signed-off-by: Stefan Weil <[email protected]>
Visual Studio 2015 does not pass regression tests with `__restrict` so kept disabled for MSVC.
Need to check proper usage of OPJ_RESTRICT (if correct then there’s
probably a bug  in vc14)

Closes #661
In case multiple ihdr box are present, only the first one shall be
taken into account.
l_tcp->numlayers and l_step_l are both OPJ_UINT32 type variables. Thus
using SIZE_MAX or ((size_t)-1) to check integer overflow is
insufficient. We should use (OPJ_UINT32)-1 here.
@stweil
Copy link
Contributor

stweil commented Sep 8, 2016

Could you please test the current code with this modification:

l_current_pi->include = (OPJ_INT16*) opj_calloc(((size_t)l_tcp->numlayers +1) * l_step_l, sizeof(OPJ_INT16)); 

The type cast is needed because otherwise addition and multiplication are done using 32 bit. The result is converted to size_t and passed to opj_calloc.

@mayeut
Copy link
Collaborator

mayeut commented Sep 8, 2016

@trylab,

@stweil is right see ef01f18

Once verified with latest commit, could you share your POC file so that it can be added to the test suite please.

@trylab
Copy link
Contributor Author

trylab commented Sep 8, 2016

@mayeut I opened (and closed) a issue to track it. You can get a POC at #826

@mayeut
Copy link
Collaborator

mayeut commented Sep 8, 2016

Adding data to the test suite in progress.
Thanks

@mayeut mayeut closed this Sep 8, 2016
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.

3 participants