Commit e523ec0b for libheif
commit e523ec0bf379110b7c33d4c159f8b1202d332157
Author: Dirk Farin <dirk.farin@gmail.com>
Date: Mon May 18 17:46:49 2026 +0200
fix tile coordinates validation in rotated images
diff --git a/libheif/api/libheif/heif_tiling.cc b/libheif/api/libheif/heif_tiling.cc
index 720acaff..cc9f8171 100644
--- a/libheif/api/libheif/heif_tiling.cc
+++ b/libheif/api/libheif/heif_tiling.cc
@@ -71,7 +71,18 @@ heif_error heif_image_handle_get_grid_image_tile_id(const heif_image_handle* han
}
const ImageGrid& gridspec = gridItem->get_grid_spec();
- if (tile_x >= gridspec.get_columns() || tile_y >= gridspec.get_rows()) {
+
+ if (process_image_transformations) {
+ // transform_requested_tile_position_to_original_tile_position() validates
+ // (tile_x, tile_y) against the *displayed* tile-grid dimensions (which may
+ // differ from the file's dimensions when a 90°/270° rotation is present)
+ // before mapping the coordinates back to the in-file grid.
+ Error err = gridItem->transform_requested_tile_position_to_original_tile_position(tile_x, tile_y);
+ if (err) {
+ return err.error_struct(handle->context.get());
+ }
+ }
+ else if (tile_x >= gridspec.get_columns() || tile_y >= gridspec.get_rows()) {
return {
heif_error_Usage_error,
heif_suberror_Unspecified,
@@ -79,10 +90,6 @@ heif_error heif_image_handle_get_grid_image_tile_id(const heif_image_handle* han
};
}
- if (process_image_transformations) {
- gridItem->transform_requested_tile_position_to_original_tile_position(tile_x, tile_y);
- }
-
*tile_item_id = gridItem->get_grid_tiles()[tile_y * gridspec.get_columns() + tile_x];
return heif_error_ok;
diff --git a/libheif/image-items/image_item.cc b/libheif/image-items/image_item.cc
index fe6a3fea..76718974 100644
--- a/libheif/image-items/image_item.cc
+++ b/libheif/image-items/image_item.cc
@@ -747,29 +747,52 @@ Error ImageItem::transform_requested_tile_position_to_original_tile_position(uin
return propertiesResult.error();
}
+ // The caller's (tile_x, tile_y) are in the *displayed* tile grid, so they
+ // must be validated against the displayed dimensions, not the in-file ones.
+ // For rotations of 90°/270° the displayed grid has its columns and rows
+ // swapped relative to the file. Using the file dims (as before) both let
+ // out-of-range coordinates through and produced unsigned underflows inside
+ // the inverse-rotation formulas (e.g. `num_rows - 1 - tile_x` with
+ // `tile_x >= num_rows`).
heif_image_tiling tiling = get_heif_image_tiling();
+ if (Error err = process_image_transformations_on_tiling(tiling)) {
+ return err;
+ }
+
+ if (tile_x >= tiling.num_columns || tile_y >= tiling.num_rows) {
+ return {heif_error_Usage_error,
+ heif_suberror_Unspecified,
+ "Tile coordinate out of range for displayed image"};
+ }
+
+ // Walk the property chain in reverse, undoing each transformation as we go.
+ // Track the current (intermediate) tile-grid dimensions so each inverse uses
+ // the right extent and so 90°/270° rotations swap dims for subsequent steps.
+ uint32_t cur_cols = tiling.num_columns;
+ uint32_t cur_rows = tiling.num_rows;
- //for (auto& prop : std::ranges::reverse_view(propertiesResult.value)) {
for (auto propIter = propertiesResult->rbegin(); propIter != propertiesResult->rend(); propIter++) {
if (auto irot = std::dynamic_pointer_cast<Box_irot>(*propIter)) {
switch (irot->get_rotation_ccw()) {
case 90: {
- uint32_t tx0 = tiling.num_columns - 1 - tile_y;
+ uint32_t tx0 = cur_rows - 1 - tile_y;
uint32_t ty0 = tile_x;
- tile_y = ty0;
tile_x = tx0;
+ tile_y = ty0;
+ std::swap(cur_cols, cur_rows);
break;
}
case 270: {
uint32_t tx0 = tile_y;
- uint32_t ty0 = tiling.num_rows - 1 - tile_x;
- tile_y = ty0;
+ uint32_t ty0 = cur_cols - 1 - tile_x;
tile_x = tx0;
+ tile_y = ty0;
+ std::swap(cur_cols, cur_rows);
break;
}
case 180: {
- tile_x = tiling.num_columns - 1 - tile_x;
- tile_y = tiling.num_rows - 1 - tile_y;
+ tile_x = cur_cols - 1 - tile_x;
+ tile_y = cur_rows - 1 - tile_y;
break;
}
case 0:
@@ -783,10 +806,10 @@ Error ImageItem::transform_requested_tile_position_to_original_tile_position(uin
if (auto imir = std::dynamic_pointer_cast<Box_imir>(*propIter)) {
switch (imir->get_mirror_direction()) {
case heif_transform_mirror_direction_horizontal:
- tile_x = tiling.num_columns - 1 - tile_x;
+ tile_x = cur_cols - 1 - tile_x;
break;
case heif_transform_mirror_direction_vertical:
- tile_y = tiling.num_rows - 1 - tile_y;
+ tile_y = cur_rows - 1 - tile_y;
break;
default:
assert(false);
diff --git a/tests/grid_tile_missing.cc b/tests/grid_tile_missing.cc
index 2de81ee2..b0025424 100644
--- a/tests/grid_tile_missing.cc
+++ b/tests/grid_tile_missing.cc
@@ -26,6 +26,7 @@
#include "catch_amalgamated.hpp"
#include "libheif/heif.h"
+#include "libheif/heif_tiling.h"
#include <cstdint>
#include <vector>
@@ -179,6 +180,120 @@ std::vector<uint8_t> build_heif_with_missing_grid_tile() {
return file;
}
+// Build a minimal HEIF with a 2x1 'grid' item (id=1) that has an associated
+// 'irot' rotation of 270° CCW. In the displayed image the grid is logically
+// 1 column by 2 rows. The grid references a missing tile (id 99), but the
+// test never gets that far: it passes (tile_x=1, tile_y=0) which is valid in
+// the file's 2x1 grid but out-of-range in the displayed 1x2 grid. The old
+// transform code did `num_rows - 1 - tile_x = 1 - 1 - 1` → unsigned underflow
+// (UBSan).
+std::vector<uint8_t> build_heif_grid_with_irot270() {
+ std::vector<uint8_t> ftyp_payload;
+ append_fourcc(ftyp_payload, "heic");
+ put_u32_be(ftyp_payload, 0);
+ append_fourcc(ftyp_payload, "mif1");
+ append_fourcc(ftyp_payload, "heic");
+ auto ftyp = make_box("ftyp", ftyp_payload);
+
+ std::vector<uint8_t> hdlr_payload;
+ put_u32_be(hdlr_payload, 0);
+ append_fourcc(hdlr_payload, "pict");
+ put_u32_be(hdlr_payload, 0);
+ put_u32_be(hdlr_payload, 0);
+ put_u32_be(hdlr_payload, 0);
+ hdlr_payload.push_back(0);
+ auto hdlr = make_box("hdlr", hdlr_payload, /*full=*/true);
+
+ std::vector<uint8_t> pitm_payload;
+ put_u16_be(pitm_payload, 1);
+ auto pitm = make_box("pitm", pitm_payload, /*full=*/true);
+
+ std::vector<uint8_t> infe_payload;
+ put_u16_be(infe_payload, 1);
+ put_u16_be(infe_payload, 0);
+ append_fourcc(infe_payload, "grid");
+ infe_payload.push_back(0);
+ auto infe = make_box("infe", infe_payload, /*full=*/true, /*version=*/2);
+
+ std::vector<uint8_t> iinf_payload;
+ put_u16_be(iinf_payload, 1);
+ append(iinf_payload, infe);
+ auto iinf = make_box("iinf", iinf_payload, /*full=*/true);
+
+ // ispe (prop 1): 128 (w) x 64 (h) -- the file/grid dimensions.
+ std::vector<uint8_t> ispe_payload;
+ put_u32_be(ispe_payload, 128);
+ put_u32_be(ispe_payload, 64);
+ auto ispe = make_box("ispe", ispe_payload, /*full=*/true);
+
+ // irot (prop 2): rotation = 3 * 90° = 270° CCW.
+ std::vector<uint8_t> irot_payload;
+ irot_payload.push_back(3);
+ auto irot = make_box("irot", irot_payload);
+
+ std::vector<uint8_t> ipco_payload;
+ append(ipco_payload, ispe);
+ append(ipco_payload, irot);
+ auto ipco = make_box("ipco", ipco_payload);
+
+ // ipma: item 1 associated with prop 1 (ispe) and prop 2 (irot).
+ std::vector<uint8_t> ipma_payload;
+ put_u32_be(ipma_payload, 1);
+ put_u16_be(ipma_payload, 1);
+ ipma_payload.push_back(2);
+ ipma_payload.push_back(0x80 | 1);
+ ipma_payload.push_back(0x80 | 2);
+ auto ipma = make_box("ipma", ipma_payload, /*full=*/true);
+
+ std::vector<uint8_t> iprp_payload;
+ append(iprp_payload, ipco);
+ append(iprp_payload, ipma);
+ auto iprp = make_box("iprp", iprp_payload);
+
+ // iloc v1: item 1 in idat, 8-byte ImageGrid header.
+ std::vector<uint8_t> iloc_payload;
+ put_u16_be(iloc_payload, (4 << 12) | (4 << 8) | (0 << 4) | 0);
+ put_u16_be(iloc_payload, 1);
+ put_u16_be(iloc_payload, 1);
+ put_u16_be(iloc_payload, 0x0001);
+ put_u16_be(iloc_payload, 0);
+ put_u16_be(iloc_payload, 1);
+ put_u32_be(iloc_payload, 0);
+ put_u32_be(iloc_payload, 8);
+ auto iloc = make_box("iloc", iloc_payload, /*full=*/true, /*version=*/1);
+
+ // iref: grid (item 1) references 2 missing tiles (ids 99 and 100). Two
+ // distinct ids are required because iref rejects duplicate references.
+ std::vector<uint8_t> dimg_payload;
+ put_u16_be(dimg_payload, 1);
+ put_u16_be(dimg_payload, 2);
+ put_u16_be(dimg_payload, 99);
+ put_u16_be(dimg_payload, 100);
+ auto dimg = make_box("dimg", dimg_payload);
+ auto iref = make_box("iref", dimg, /*full=*/true);
+
+ // idat: ImageGrid header — v=0, flags=0, rows-1=0, cols-1=1, w=128, h=64.
+ std::vector<uint8_t> idat_payload = {0, 0, 0, 1};
+ put_u16_be(idat_payload, 128);
+ put_u16_be(idat_payload, 64);
+ auto idat = make_box("idat", idat_payload);
+
+ std::vector<uint8_t> meta_payload;
+ append(meta_payload, hdlr);
+ append(meta_payload, pitm);
+ append(meta_payload, iinf);
+ append(meta_payload, iprp);
+ append(meta_payload, iloc);
+ append(meta_payload, iref);
+ append(meta_payload, idat);
+ auto meta = make_box("meta", meta_payload, /*full=*/true);
+
+ std::vector<uint8_t> file;
+ append(file, ftyp);
+ append(file, meta);
+ return file;
+}
+
} // namespace
@@ -242,10 +357,66 @@ TEST_CASE("grid: out-of-range tile coordinate returns error instead of OOB") {
heif_chroma_420,
nullptr, /*tile_x=*/1, /*tile_y=*/0);
- REQUIRE(err.code == heif_error_Invalid_input);
- REQUIRE(err.subcode == heif_suberror_Missing_grid_images);
+ // The out-of-range coordinate is now rejected up-front by the tile-position
+ // transform (see transform_requested_tile_position_to_original_tile_position).
+ REQUIRE(err.code == heif_error_Usage_error);
REQUIRE(tile == nullptr);
heif_image_handle_release(handle);
heif_context_free(ctx);
}
+
+
+// Regression for the UBSan crash in
+// ImageItem::transform_requested_tile_position_to_original_tile_position:
+// `tiling.num_rows - 1 - tile_x` underflowed when a 90°/270° irot was present
+// and the caller passed a tile coordinate that was valid in the file's grid
+// but out-of-range in the displayed (rotated) grid.
+TEST_CASE("grid: rotated grid rejects tile coord beyond displayed bounds") {
+ auto data = build_heif_grid_with_irot270();
+
+ heif_context* ctx = heif_context_alloc();
+ REQUIRE(ctx != nullptr);
+
+ heif_error err = heif_context_read_from_memory_without_copy(
+ ctx, data.data(), data.size(), nullptr);
+ REQUIRE(err.code == heif_error_Ok);
+
+ heif_image_handle* handle = nullptr;
+ err = heif_context_get_primary_image_handle(ctx, &handle);
+ REQUIRE(err.code == heif_error_Ok);
+ REQUIRE(handle != nullptr);
+
+ // Displayed tiling should reflect the 270° rotation: 1 column, 2 rows.
+ heif_image_tiling tiling = {};
+ err = heif_image_handle_get_image_tiling(handle, /*xform=*/1, &tiling);
+ REQUIRE(err.code == heif_error_Ok);
+ REQUIRE(tiling.num_columns == 1);
+ REQUIRE(tiling.num_rows == 2);
+
+ // (tile_x=1, tile_y=0) is in-range for the file's 2x1 grid but out-of-range
+ // for the displayed 1x2 grid. Pre-fix this hit unsigned underflow inside
+ // the inverse 270° transform. It must now be rejected as a usage error
+ // instead of crashing under UBSan.
+ heif_item_id tile_id = 0;
+ err = heif_image_handle_get_grid_image_tile_id(
+ handle, /*xform=*/1, /*tile_x=*/1, /*tile_y=*/0, &tile_id);
+ REQUIRE(err.code == heif_error_Usage_error);
+
+ heif_image* tile = nullptr;
+ err = heif_image_handle_decode_image_tile(handle, &tile,
+ heif_colorspace_YCbCr,
+ heif_chroma_420,
+ nullptr, /*tile_x=*/1, /*tile_y=*/0);
+ REQUIRE(err.code == heif_error_Usage_error);
+ REQUIRE(tile == nullptr);
+
+ // Sanity: an in-range displayed coordinate is *not* rejected by the bounds
+ // check (it reaches the missing-tile path).
+ err = heif_image_handle_get_grid_image_tile_id(
+ handle, /*xform=*/1, /*tile_x=*/0, /*tile_y=*/1, &tile_id);
+ REQUIRE(err.code == heif_error_Ok);
+
+ heif_image_handle_release(handle);
+ heif_context_free(ctx);
+}