Skip to content

Harden image loader size arithmetic - #340

Merged
hpjansson merged 3 commits into
hpjansson:masterfrom
zouyonghe:fix/image-loader-overflow-checks
Jul 26, 2026
Merged

hpjansson merged 3 commits into
hpjansson:masterfrom
zouyonghe:fix/image-loader-overflow-checks

Conversation

@zouyonghe

@zouyonghe zouyonghe commented Jun 23, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #339.

Summary

This PR hardens decoded image buffer size calculations for loader paths that consume dimensions from untrusted image files.

The change introduces a shared checked-size helper and applies it before allocation, copying, or passing output buffers to decoder APIs.

Changes

  • Add chicle_checked_image_buffer_size() for checked 64-bit image buffer sizing.
  • Use checked sizing before AVIF RGB frame allocation.
  • Use checked sizing for HEIF image dimensions and stride validation.
  • Use checked sizing for JXL frame allocation and JxlDecoderSetImageOutBuffer().
  • Use checked sizing before WebP frame buffer copies.
  • Return explicitly when WebP frame-size validation fails.
  • Recompute JXL frame buffer size before each frame allocation and check the cumulative frame budget before allocating.
  • Reject failed or negative fstat() sizes before mapping files.
  • Add loader-arithmetic-test coverage for oversized dimension rejection, zero dimensions/channels, exact max_size boundaries, G_MAXSIZE / n_channels boundaries, and valid size acceptance.

Verification

Confirmed the previous unsafe arithmetic pattern can overflow under clang integer sanitizer with a minimal reproducer:

runtime error: unsigned integer overflow: 4294967295 * 4294967295 cannot be represented in type 'uint32_t'

Ran the new regression test:

tests/loader-arithmetic-test
5/5 PASS

Ran CI-like sanitizer build and C unit tests:

byte-fifo-test           PASS
canvas-test              PASS
term-info-test           PASS
loader-arithmetic-test   PASS

Ran a clean normal build and full test suite:

tests/test-suite.log

TOTAL: 13
PASS:  13
FAIL:  0
ERROR: 0

Notes

The sanitizer run still reports existing unrelated UBSan recover-mode findings in areas such as chafa-symbol-map, smolscale, and chafa-string-util. Those reports are not introduced by this PR. The clean normal build and full test suite pass.

@hpjansson

Copy link
Copy Markdown
Owner

Thanks for this! I'd make one change before merging: The image size checking function should probably live in chicle-util.c. It's too tiny to warrant its own code unit.

@zouyonghe

zouyonghe commented Jul 3, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks for the suggestion. I moved the image buffer size helper into chicle-util.[ch] and removed the separate chicle-image-size.h header.

I also updated the loader arithmetic test to link chicle-util.c directly. While checking that, I enabled subdir-objects for the test Makefile so Automake does not warn about the cross-directory test source.

Verified with:

  • ./autogen.sh
  • make -C tests loader-arithmetic-test && ./tests/loader-arithmetic-test
  • make

@hpjansson

Copy link
Copy Markdown
Owner

Thanks! It looks good now.

I may refine the build mechanics later on, since this introduces unit tests for libchicle too, and we may want to use a more general approach (one for libchafa, one for libchicle). Also I think we need to be able to read() even if the fstat() fails, but your patch is better than what we had, since it fixes a bug that was there currently.

@hpjansson
hpjansson merged commit cdd64f5 into hpjansson:master Jul 26, 2026
1 check passed
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.

Harden image loader buffer size arithmetic

2 participants