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

Limit total size of image instead of width and height individually. #1204

Merged
merged 3 commits into from
Jul 4, 2024

Conversation

fancycode
Copy link
Member

This allows processing panorama images with could be more than 32k pixels wide but a lot less than 32k pixexls high (e.g. 40000 x 5000 pixels). This fits in the memory limit defined by the previous 32k x 32k pixels limit but would be rejected without this PR.

@fancycode fancycode force-pushed the limit-total-image-size branch 4 times, most recently from 067eb55 to ba9a6ff Compare June 28, 2024 09:06
@farindk
Copy link
Contributor

farindk commented Jun 28, 2024

Thank you, that makes sense.
But now we get an integer overflow somewhere:

file_fuzzer: /src/libheif/fuzzing/file_fuzzer.cc:41: void TestDecodeImage(struct heif_context *, const struct heif_image_handle *, size_t): Assertion `height >= 0' failed.

@fancycode fancycode force-pushed the limit-total-image-size branch 4 times, most recently from d897445 to dc5808d Compare July 1, 2024 07:48
@fancycode
Copy link
Member Author

Thank you, that makes sense. But now we get an integer overflow somewhere:

file_fuzzer: /src/libheif/fuzzing/file_fuzzer.cc:41: void TestDecodeImage(struct heif_context *, const struct heif_image_handle *, size_t): Assertion `height >= 0' failed.

Fixed (and added fuzzing input to corpus). The size check is now implemented in a central location so it doesn't need to be modified in lots of places if it needs to be changed in the future.

Problem was caused by a fuzzing input that had width 0 and height 4294967103. This then overflowed the height which was cast to int in

image->set_resolution(width, height);

@fancycode fancycode force-pushed the limit-total-image-size branch from dc5808d to fdc1ef7 Compare July 1, 2024 07:50
fancycode added 3 commits July 1, 2024 09:54
Fixes compiler errors like the following with clang-12:

/path/to/libheif/libheif/api/libheif/heif.cc:1673:10: error: result of comparison of constant 4294967295 with expression of type 'uint16_t' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
      cp > std::numeric_limits<std::underlying_type<heif_color_primaries>::type>::max()) {
      ~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/path/to/libheif/libheif/api/libheif/heif.cc:1714:10: error: result of comparison of constant 4294967295 with expression of type 'uint16_t' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
      tc > std::numeric_limits<std::underlying_type<heif_transfer_characteristics>::type>::max()) {
      ~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/path/to/libheif/libheif/api/libheif/heif.cc:1751:10: error: result of comparison of constant 4294967295 with expression of type 'uint16_t' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
      mc > std::numeric_limits<std::underlying_type<heif_matrix_coefficients>::type>::max()) {
      ~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 errors generated.
@fancycode fancycode force-pushed the limit-total-image-size branch from fdc1ef7 to a31ffe6 Compare July 1, 2024 07:54
@farindk farindk merged commit fba9ee8 into master Jul 4, 2024
36 checks passed
@farindk
Copy link
Contributor

farindk commented Jul 4, 2024

Thank you

@fancycode fancycode deleted the limit-total-image-size branch July 4, 2024 10:30
fancycode referenced this pull request Jul 4, 2024
kleisauke added a commit to kleisauke/libvips that referenced this pull request Jul 18, 2024
This is required since PR strukturag/libheif#1204 (libheif 1.18.0),
which limits the total image size instead of the width and height
individually.
jcupitt pushed a commit to libvips/libvips that referenced this pull request Jul 18, 2024
This is required since PR strukturag/libheif#1204 (libheif 1.18.0),
which limits the total image size instead of the width and height
individually.
kleisauke added a commit to kleisauke/libvips that referenced this pull request Jul 25, 2024
This is required since PR strukturag/libheif#1204 (libheif 1.18.0),
which limits the total image size instead of the width and height
individually.

(cherry picked from commit 9a3eb99)
kleisauke added a commit to libvips/libvips that referenced this pull request Jul 25, 2024
This is required since PR strukturag/libheif#1204 (libheif 1.18.0),
which limits the total image size instead of the width and height
individually.

(cherry picked from commit 9a3eb99)
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.

2 participants