Commit 3e01ff32 for libheif
commit 3e01ff32b5eb8fd1570cb33e4ed89e0a426fc732
Author: Dirk Farin <dirk.farin@gmail.com>
Date: Sat Sep 5 18:00:57 2026 +0200
Reject items with an item-error in the depth/aux/thumbnail handle getters
heif_image_handle_get_depth_image_handle(), _get_thumbnail() and
_get_auxiliary_image_handle() handed out a handle for an item that failed to
parse (an ImageItem_Error, e.g. an unsupported codec), so the error only
surfaced later at heif_decode_image(). heif_context_get_image_handle() and
heif_context_get_primary_image_handle() already reject such items up front; do
the same here by returning the item's error at handle-creation time.
Updates the error-item regression test: the depth-aux case now expects the error
from the depth-handle getter, and a second case decodes a primary whose alpha
auxiliary is an error item to keep covering the verify_decodable() reference
walk (which must not dereference a null context).
diff --git a/libheif/api/libheif/heif_aux_images.cc b/libheif/api/libheif/heif_aux_images.cc
index d36e3ef1..214ebfb5 100644
--- a/libheif/api/libheif/heif_aux_images.cc
+++ b/libheif/api/libheif/heif_aux_images.cc
@@ -85,6 +85,14 @@ heif_error heif_image_handle_get_depth_image_handle(const heif_image_handle* han
return err.error_struct(handle->image.get());
}
+ // Reject an item that could not be parsed (e.g. an unsupported codec) here,
+ // like heif_context_get_image_handle() does, rather than handing out a handle
+ // that can only fail at decode.
+ if (Error error = depth_image->get_item_error()) {
+ *out_depth_handle = nullptr;
+ return error.error_struct(handle->image.get());
+ }
+
*out_depth_handle = new heif_image_handle();
(*out_depth_handle)->image = depth_image;
(*out_depth_handle)->context = handle->context;
@@ -167,6 +175,10 @@ heif_error heif_image_handle_get_thumbnail(const heif_image_handle* handle,
auto thumbnails = handle->image->get_thumbnails();
for (const auto& thumb : thumbnails) {
if (thumb->get_id() == thumbnail_id) {
+ if (Error error = thumb->get_item_error()) {
+ *out_thumbnail_handle = nullptr;
+ return error.error_struct(handle->image.get());
+ }
*out_thumbnail_handle = new heif_image_handle();
(*out_thumbnail_handle)->image = thumb;
(*out_thumbnail_handle)->context = handle->context;
@@ -313,6 +325,9 @@ heif_error heif_image_handle_get_auxiliary_image_handle(const heif_image_handle*
auto auxImages = main_image_handle->image->get_aux_images();
for (const auto& aux : auxImages) {
if (aux->get_id() == auxiliary_id) {
+ if (Error error = aux->get_item_error()) {
+ return error.error_struct(main_image_handle->image.get());
+ }
*out_auxiliary_handle = new heif_image_handle();
(*out_auxiliary_handle)->image = aux;
(*out_auxiliary_handle)->context = main_image_handle->context;
diff --git a/tests/error_item_decode.cc b/tests/error_item_decode.cc
index ce53e5b9..f99b9e8e 100644
--- a/tests/error_item_decode.cc
+++ b/tests/error_item_decode.cc
@@ -24,32 +24,35 @@
SOFTWARE.
*/
-// Regression test for a null-pointer dereference reachable from a fuzzer
-// (OSS-Fuzz / CIFuzz). An item whose type libheif cannot handle (here 'lhv1',
-// layered HEVC) is represented internally by an ImageItem_Error placeholder.
-// These placeholders used to be constructed with a null HeifContext, so any
-// code that dereferenced their context crashed. ImageItem::decode_image() gained
-// a verify_decodable() pre-pass that calls get_file() (-> context->get_heif_file())
-// before decoding, so decoding such an item segfaulted.
+// Regression tests around unsupported items, which are represented internally by
+// an ImageItem_Error placeholder (here triggered with 'lhv1', layered HEVC).
//
-// The depth-image handle getter does not reject items with an item-error, so an
-// error item attached as a depth auxiliary can be handed to heif_decode_image()
-// and reach the crash. The fix gives error items their real context, so decoding
-// one returns its error instead of dereferencing null.
+// 1. These placeholders used to be constructed with a null HeifContext, so any
+// code dereferencing their context crashed. ImageItem::decode_image() gained
+// a verify_decodable() pre-pass that walks the reference graph and calls
+// get_file() (-> context->get_heif_file()); with the error item as an alpha
+// auxiliary, that walk dereferenced null (OSS-Fuzz / CIFuzz). Error items now
+// carry their real context.
+//
+// 2. The depth/aux/thumbnail handle getters used not to reject an item with an
+// item-error, unlike heif_context_get_image_handle(), so they handed out a
+// handle that could only fail at decode. They now surface the item's error at
+// handle-creation time.
#include "catch_amalgamated.hpp"
#include "libheif/heif.h"
#include "test_utils.h"
#include <cstdint>
+#include <string>
#include <vector>
namespace {
-// A minimal file: item 1 is a decodable 16x16 'mski' mask (the primary); item 2
-// is an 'lhv1' item (unsupported -> ImageItem_Error) declared as the depth
-// auxiliary of item 1 via 'auxl' + an 'auxC' depth type.
-std::vector<uint8_t> build_file_with_error_depth_item() {
+// item 1: decodable 16x16 'mski' primary. item 2: an 'lhv1' item (unsupported ->
+// ImageItem_Error) attached to item 1 as an auxiliary of the given type (via
+// 'auxl' + an 'auxC' of `aux_urn`).
+std::vector<uint8_t> build_file_with_error_aux(const std::string& aux_urn) {
const uint32_t W = 16, H = 16;
std::vector<uint8_t> ftyp_payload;
@@ -91,7 +94,7 @@ std::vector<uint8_t> build_file_with_error_depth_item() {
}
auto iinf = make_box("iinf", iinf_payload, /*full=*/true);
- // ipco: 1=ispe, 2=mskC, 3=auxC(depth)
+ // ipco: 1=ispe, 2=mskC, 3=auxC(aux_urn)
std::vector<uint8_t> ispe_payload;
put_u32_be(ispe_payload, W);
put_u32_be(ispe_payload, H);
@@ -102,7 +105,7 @@ std::vector<uint8_t> build_file_with_error_depth_item() {
auto mskC = make_box("mskC", mskC_payload, /*full=*/true);
std::vector<uint8_t> auxC_payload;
- append_cstr(auxC_payload, "urn:mpeg:mpegB:cicp:systems:auxiliary:depth");
+ append_cstr(auxC_payload, aux_urn.c_str());
auto auxC = make_box("auxC", auxC_payload, /*full=*/true);
std::vector<uint8_t> ipco_payload;
@@ -117,7 +120,7 @@ std::vector<uint8_t> build_file_with_error_depth_item() {
ipma_payload.push_back(2);
ipma_payload.push_back(0x80 | 1);
ipma_payload.push_back(0x80 | 2);
- put_u16_be(ipma_payload, 2); // item 2: ispe + auxC(depth)
+ put_u16_be(ipma_payload, 2); // item 2: ispe + auxC
ipma_payload.push_back(2);
ipma_payload.push_back(0x80 | 1);
ipma_payload.push_back(0x80 | 3);
@@ -128,21 +131,19 @@ std::vector<uint8_t> build_file_with_error_depth_item() {
append(iprp_payload, ipma);
auto iprp = make_box("iprp", iprp_payload);
- std::vector<uint8_t> idat_payload(W * H + 16, 0x7F); // item 1 mask + item 2 dummy
+ std::vector<uint8_t> idat_payload(W * H + 16, 0x7F);
auto idat = make_box("idat", idat_payload);
std::vector<uint8_t> iloc_payload;
iloc_payload.push_back((4 << 4) | 4);
iloc_payload.push_back(0);
put_u16_be(iloc_payload, 2);
- // item 1
put_u16_be(iloc_payload, 1);
- put_u16_be(iloc_payload, 0x0001); // idat
+ 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, W * H);
- // item 2
put_u16_be(iloc_payload, 2);
put_u16_be(iloc_payload, 0x0001);
put_u16_be(iloc_payload, 0);
@@ -151,7 +152,7 @@ std::vector<uint8_t> build_file_with_error_depth_item() {
put_u32_be(iloc_payload, 16);
auto iloc = make_box("iloc", iloc_payload, /*full=*/true, /*version=*/1);
- // iref: 'auxl' from item 2 (aux) to item 1 (master)
+ // 'auxl' from item 2 (aux) to item 1 (master)
std::vector<uint8_t> auxl_payload;
put_u16_be(auxl_payload, 2);
put_u16_be(auxl_payload, 1);
@@ -177,41 +178,55 @@ std::vector<uint8_t> build_file_with_error_depth_item() {
} // namespace
-// Decoding an error item (reached here through its depth-image handle) must
-// return that item's error, not crash on a null context.
-TEST_CASE("error item: decoding an unsupported depth aux item does not crash") {
- auto data = build_file_with_error_depth_item();
+// The depth-image handle getter must reject an item that has an item-error at
+// handle-creation time, not hand out a handle that only fails at decode.
+TEST_CASE("error item: getting the handle of an unsupported depth aux fails early") {
+ auto data = build_file_with_error_aux("urn:mpeg:mpegB:cicp:systems:auxiliary:depth");
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);
+ REQUIRE(heif_context_read_from_memory_without_copy(ctx, data.data(), data.size(), nullptr).code
+ == heif_error_Ok);
heif_image_handle* primary = nullptr;
- err = heif_context_get_primary_image_handle(ctx, &primary);
- REQUIRE(err.code == heif_error_Ok);
- REQUIRE(primary != nullptr);
+ REQUIRE(heif_context_get_primary_image_handle(ctx, &primary).code == heif_error_Ok);
- int num_depth = heif_image_handle_get_number_of_depth_images(primary);
- REQUIRE(num_depth == 1);
+ REQUIRE(heif_image_handle_get_number_of_depth_images(primary) == 1);
heif_item_id depth_id = 0;
heif_image_handle_get_list_of_depth_image_IDs(primary, &depth_id, 1);
heif_image_handle* depth_handle = nullptr;
- err = heif_image_handle_get_depth_image_handle(primary, depth_id, &depth_handle);
- REQUIRE(err.code == heif_error_Ok);
- REQUIRE(depth_handle != nullptr);
+ heif_error err = heif_image_handle_get_depth_image_handle(primary, depth_id, &depth_handle);
+
+ REQUIRE(err.code != heif_error_Ok); // the item's error is surfaced here
+ REQUIRE(depth_handle == nullptr);
+
+ heif_image_handle_release(primary);
+ heif_context_free(ctx);
+}
+
+// Decoding a primary whose alpha auxiliary is an unsupported (error) item must
+// not crash: verify_decodable() walks the reference graph, including the alpha
+// edge into the error item, and dereferences its context. (Error items now
+// carry a real context.)
+TEST_CASE("error item: decoding a primary with an unsupported alpha aux does not crash") {
+ auto data = build_file_with_error_aux("urn:mpeg:mpegB:cicp:systems:auxiliary:alpha");
+
+ heif_context* ctx = heif_context_alloc();
+ REQUIRE(ctx != nullptr);
+ REQUIRE(heif_context_read_from_memory_without_copy(ctx, data.data(), data.size(), nullptr).code
+ == heif_error_Ok);
+
+ heif_image_handle* primary = nullptr;
+ REQUIRE(heif_context_get_primary_image_handle(ctx, &primary).code == heif_error_Ok);
- // The crash was here: decode_image() -> verify_decodable() -> get_file() on a
- // null context. Reaching the assert at all proves it no longer segfaults.
+ // Reaching this call at all (no segfault in verify_decodable) is the point.
heif_image* img = nullptr;
- err = heif_decode_image(depth_handle, &img, heif_colorspace_undefined, heif_chroma_undefined, nullptr);
- REQUIRE(err.code != heif_error_Ok); // the item's own error is surfaced
+ heif_error err = heif_decode_image(primary, &img, heif_colorspace_undefined, heif_chroma_undefined, nullptr);
+ (void) err; // may succeed (ignoring the broken alpha) or fail; must not crash
if (img) { heif_image_release(img); }
- heif_image_handle_release(depth_handle);
heif_image_handle_release(primary);
heif_context_free(ctx);
}