Refactor ImfCheckFile and oss-fuzz tests #1257
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Address https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=46459 by reorganizing Imf::CheckFile and tests.
CheckFile's logic for reducing memory usage hadn't been updated to support reading multiple parts using the RgbaFile API.
To reduce code spaghetti, CheckFile now uses the built-in limits for image and tile size rather than its own checks. Static methods to read those values have been added to Imf::Header to allow CheckFile to restore the original values before returning. (This extends the API and ABI but is backwards compatible)
Additionally, the interpretation of 'enableCoreChecks' (and exrcheck's "-c" flag) have been changed to run only the Core API tests, and not the C++ API tests. Previously, only files which were considered 'valid' by the Core API would be tested with the C++ API.
This change also splits the Core and C++ API tests into two separate binaries with the oss-fuzz suite: the new binary runs just the Core API. This should prevent timeout errors and also help to triage where issues may be occurring. The downside of this change is that fuzz tests will abort earlier when run on large images, so will be less able to detect vulnerabilities that may be present without those limits set.
Note oss-fuzz 46413 and 46432 may also be marked as resolved by this PR, but nothing has been done here to address those issues, so new related fuzz reports will likely appear
Signed-off-by: Peter Hillman peterh@wetafx.co.nz