Skip to content

Commit

Permalink
Do not check maximum image size while merely parsing the file. Do the…
Browse files Browse the repository at this point in the history 
… test when actually decoding the image (#1220)
  • Loading branch information
@farindk
farindk committed Jul 4, 2024
1 parent d432310 commit 29bc4c3
Showing 1 changed file with 10 additions and 7 deletions.
17 changes: 10 additions & 7 deletions libheif/context.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -609,13 +609,6 @@ Error HeifContext::interpret_heif_file()
uint32_t width = ispe->get_width();
uint32_t height = ispe->get_height();


// --- check whether the image size is "too large"
err = check_resolution(width, height);
if (err) {
return err;
}

image->set_resolution(width, height);
ispe_read = true;
}
Expand Down Expand Up @@ -1362,6 +1355,16 @@ Error HeifContext::decode_image_planar(heif_item_id ID,
Error error;


// --- check whether image size exceeds maximum (according to 'ispe')

auto ispe = m_heif_file->get_property<Box_ispe>(ID);
if (ispe) {
error = check_resolution(ispe->get_width(), ispe->get_height());
if (error) {
return error;
}
}

// --- decode image, depending on its type

if (image_type == "hvc1" ||
Expand Down

3 comments on commit 29bc4c3

@fancycode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This triggers an overflow with the fuzzing file from https://github.com/strukturag/libheif/actions/runs/9793699446:

$ ./fuzzing/file_fuzzer ../fuzzing/data/corpus/crash-22ce65742e65f2e18e145511f47e1265634d6d48 
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3422022759
INFO: Loaded 2 modules   (38550 inline 8-bit counters): 38458 [0x7f61cabf7f70, 0x7f61cac015aa), 92 [0x5b5ae0, 0x5b5b3c), 
INFO: Loaded 2 PC tables (38550 PCs): 38458 [0x7f61cac015b0,0x7f61cac97950), 92 [0x5b5b40,0x5b6100), 
./fuzzing/file_fuzzer: Running 1 inputs 1 time(s) each.
Running: ../fuzzing/data/corpus/crash-22ce65742e65f2e18e145511f47e1265634d6d48
/path/to/libheif/libheif/context.cc:612:31: runtime error: implicit conversion from type 'uint32_t' (aka 'unsigned int') of value 3741341181 (32-bit, unsigned) to type 'int' changed the value to -553626115 (32-bit, signed)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/jojo/devel/struktur/libheif/libheif/context.cc:612:31 in

Also fails the CIfuzzer jobs, e.g. https://github.com/strukturag/libheif/actions/runs/9793699446/job/27042172552

(wondering why it did not fail the CI fuzzer with the similar corpus file from #1204)

@fancycode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(wondering why it did not fail the CI fuzzer with the similar corpus file from #1204)

I can answer this myself: the file_fuzzer only runs on *.heic files in the corpus:

libheif/scripts/run-ci.sh

Line 272 in 29bc4c3

./fuzzing/file_fuzzer ./fuzzing/data/corpus/*.heic

@fancycode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix in #1222

Please sign in to comment.