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

Fix out of bounds read in isValidBoxFileType() #2180

Merged
merged 2 commits into from
Apr 1, 2022
Merged

Conversation

piponazo
Copy link
Collaborator

Fix #2178

I just added a new unit test to validate the implementation and I also validated locally the changes with the POCs provided in the issue. Since in this case the buggy function was quite well isolated and I already had unit tests for it, I do not need it is needed to add some python tests taking as arguments the POC files. But let me know if you think otherwise.

@codecov
Copy link

codecov bot commented Mar 31, 2022

Codecov Report

Merging #2180 (74622cf) into main (3b9fcb4) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2180   +/-   ##
=======================================
  Coverage   63.30%   63.30%           
=======================================
  Files          99       99           
  Lines       19596    19596           
  Branches     9559     9559           
=======================================
  Hits        12406    12406           
  Misses       5116     5116           
  Partials     2074     2074           
Impacted Files Coverage Δ
src/jp2image_int.cpp 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b9fcb4...74622cf. Read the comment docs.

@piponazo piponazo self-assigned this Mar 31, 2022
kevinbackhouse
kevinbackhouse previously approved these changes Mar 31, 2022
@kevinbackhouse
Copy link
Collaborator

Sometimes, I just add the poc images to test/data. Then they get picked up by the fuzzer automatically, which means that it takes less time for the fuzzer to find similar bugs in the future.

@kevinbackhouse
Copy link
Collaborator

I added one of the poc files to test/data.

@piponazo
Copy link
Collaborator Author

piponazo commented Apr 1, 2022

I added one of the poc files to test/data.

Ups, this change broke the execution of tests because now the regression_tests are automatically trying to run over all test data. I will take care of fixing the situation 😉

@piponazo piponazo merged commit b8cb4e0 into main Apr 1, 2022
@piponazo piponazo deleted the main_issue2178 branch April 1, 2022 07:07
@kevinbackhouse
Copy link
Collaborator

I added one of the poc files to test/data.

Ups, this change broke the execution of tests because now the regression_tests are automatically trying to run over all test data. I will take care of fixing the situation wink

Sorry, I forgot about that. Thanks for fixing it!

@kmilos
Copy link
Collaborator

kmilos commented Apr 1, 2022

Fix on 0.27-maint as well please?

@piponazo
Copy link
Collaborator Author

piponazo commented Apr 1, 2022

Fix on 0.27-maint as well please?

Sure, thanks for the reminder. I thought about it yesterday but I forgot about it later 😅

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.

Out of bounds read in isValidBoxFileType (jp2image_int.cpp)
3 participants