Repository navigation
heif/avif: validate container image sizes before decoding - #498
alepacheco wants to merge 3 commits into
Conversation
decodeHeicUltraHdr() and decodeAvifUltraHdr() size the output images from heif_image_handle_get_width/height(), which libheif derives from the 'ispe' and 'clap' properties, and copy_img_plane() then copies that many rows and bytes per row out of the decoded image. Nothing ties those properties to the coded image: an 'ispe' larger than the coded size, or a 'clap' declaring a larger crop than the coded image can provide, makes the copy read past the end of the decoded plane (heap-buffer-overflow). copy_img_plane() now fails the decode when the decoded plane is smaller than the size being copied (or missing). All base image and gain map copies in both backends go through it. Files whose coded image is larger than declared keep decoding as before.
The HEIF and AVIF decoders take the size of the base image and of the gain map from heif_image_handle_get_width/height() without checking it. That size comes from the container: an image item without an 'ispe' property has a size of 0x0, and 'ispe' can declare a size far above what the library supports. A 0x0 gain map is accepted, allocated as an empty image and passed to applyGainMap(), which divides the base image width by the gain map width and then samples the gain map with coordinates clamped to w - 1 and h - 1. With w == 0 those clamps wrap around and sampleMap*() reads past the end of the gain map buffer (heap-buffer-overflow). A huge 'ispe' is allocated as is. The decoders now read the image size only through get_image_size(), which rejects sizes that are zero or above kMaxWidth x kMaxHeight before anything is decoded or allocated, the same limits the JPEG path enforces. uhdr_dec_probe() applies the same check to the base image and the gain map, so probe and decode agree on what is decodable; like uhdr_is_supported_gainmap_image(), it now rejects a file whose gain map has an unsupported size even when only SDR output is requested.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Small test-portability issue: with -DUHDR_MAX_DIMENSION=16384, these dimensions are within the supported limit, so probing correctly succeeds and this assertion fails. Could the oversized case use the configured kMaxWidth/kMaxHeight, with a conditional skip if exceeding those would also exceed libheif’s parser limits? |
The oversized 'ispe' case hardcoded 16384x16384, which is within the supported size when the library is built with -DUHDR_MAX_DIMENSION=16384. Use kMaxWidth + 1 and kMaxHeight + 1 instead, one dimension at a time so the pixel count stays small, and skip when such a size would already be rejected by libheif's own image size limits.
|
@gregbenz Thanks, good catch. Done in d3a5c76: the oversized case now uses |
decodeHeicUltraHdr()anddecodeAvifUltraHdr()take the size of the base image and of the gain map fromheif_image_handle_get_width/height(), which libheif derives from theispeandclapproperties of the file, and use it without checking it against anything. Two heap overflows follow, both reachable fromuhdr_decode()on an untrusted HEIC/AVIF file:copy_img_plane()copies that many rows and bytes per row out of the decoded image. Anispelarger than the coded size, or aclapdeclaring a larger crop than the coded image can provide, makes the copy read past the end of the decoded plane.ispeproperty has a size of 0x0. It is allocated as an empty image and passed toapplyGainMap(), which divides by the gain map width and samples it with coordinates clamped tow - 1andh - 1; withw == 0the clamps wrap around andsampleMap*()reads past the end of the gain map buffer.This PR makes the container size something the decoders check before they use it:
heif/avif: reject unsupported image sizes before decoding: the decoders read the image size only throughget_image_size(), which rejects sizes that are zero or abovekMaxWidthxkMaxHeightbefore anything is decoded or allocated, the same limits the JPEG path enforces.uhdr_dec_probe()applies the same check (is_supported_heif_dimensions(), now available without the item API) to the base image and the gain map, so probe and decode agree on what is decodable.heif/avif: never copy more than the decoded plane holds:copy_img_plane()fails the decode when the decoded plane is smaller than the size being copied, or missing. All base image and gain map copies in both backends go through it. Files whose coded image is larger than declared keep decoding as before.Tests:
UltraHdrApiTest.HeifRejectsImageSizeLargerThanDecodedImageandUltraHdrApiTest.HeifRejectsUnsupportedImageSizesedit theispe/clapproperties and their associations in the existinggainmap_hevc_16x16.heicfixture in memory. Without the fixes AddressSanitizer reports a heap-buffer-overflow incopy_img_plane()and insampleMap3Channel()respectively; with them, the full unit test suite passes.