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

opj_{compress,decompress,dump}: fix possible buffer overflows in path manipulation functions #1346

Closed
wants to merge 2 commits into from

Conversation

kaniini
Copy link

@kaniini kaniini commented Apr 27, 2021

If img_fol->imgdirpath is itself of OPJ_PATH_LEN length, other buffers on the stack will be overwritten by the image filename.

Any output of a function where paths are concatenated should likely be OBJ_PATH_LEN * 2, but I did not change this as it would cause an ABI break.

@kaniini
Copy link
Author

kaniini commented Apr 27, 2021

Also include a fix for CVE-2021-29338.

@rouault
Copy link
Collaborator

rouault commented May 6, 2021

Would you mind rebasing on top of latest master, now that master has solved the continuous integration issues ?

@kaniini
Copy link
Author

kaniini commented May 6, 2021 via email

@rouault rouault added this to the 2.5.0 milestone May 6, 2021
@rouault
Copy link
Collaborator

rouault commented May 6, 2021

Ah, our CI still uses MSVC 2010 in one of the setups, and it lacks snprintf()...
Could you add something along the following (untested!) in the files that use snprintf() ?

#if defined(_MSC_VER) && _MSC_VER < 1900
#define snprintf(buf, len, format,...) _snprintf_s((buf), (len), (len), (format), __VA_ARGS__)
#endif

@rouault
Copy link
Collaborator

rouault commented May 6, 2021

There's also code formatting issues. See https://travis-ci.org/github/uclouvain/openjpeg/jobs/769723267 for directions to fix

@kaniini
Copy link
Author

kaniini commented May 6, 2021 via email

@rouault
Copy link
Collaborator

rouault commented May 6, 2021

I'll take care of this over the weekend, unless you need it ASAP?

that's fine. no emergency

@rouault
Copy link
Collaborator

rouault commented Jun 7, 2021

This pull request is probably no longer needed since f0629cb . Closing it. If there are remaining elements, please issue a new pull request against latest master.

@rouault rouault closed this Jun 7, 2021
@rouault
Copy link
Collaborator

rouault commented Jun 29, 2021

The changes to (char*)calloc(num_images, OPJ_PATH_LEN * sizeof(char)); would still need to be done and haven't been addressed by f0629cb

@baparham
Copy link
Contributor

@rouault @kaniini are there plans to follow up on making a new PR for the fix to CVE-2021-29338 to resolve #1338 ?

baparham added a commit to baparham/openjpeg that referenced this pull request Jan 12, 2022
rouault pushed a commit that referenced this pull request Jan 12, 2022
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.

None yet

3 participants