strukturag / libheif

libheif is an HEIF and AVIF file format decoder and encoder.
Other
1.75k stars 302 forks source link

heif_image_get_plane_readonly sometimes returns a buffer that is too small to contain the image data #417

Open Chris-R-R opened 3 years ago

Chris-R-R commented 3 years ago

The following snippet shows the problem. It only occurs with some images.

int stride=0;
const uint8_t* decoded_pic = heif_image_get_plane_readonly(img, heif_channel_interleaved, &stride);

int Height = heif_image_handle_get_height(handle);
int Width = heif_image_handle_get_width(handle);

Accessing decoded_pic[stride*Height-1] sometimes gives an access violation on Windows 10. It sometimes happens with much smaller indices too. This is with libheif ver 1.9.1 and libde265 frame-parallel branch. Unfortunately I cannot upload a sample image as it comes from a client and I cannot share it.

I have noticed that the stride returned is sometimes larger than 3 times the width, even when rounding upwards to the nearest 32-bit boundary.

Is there a method to query the size of the returned buffer?

silverbacknet commented 3 years ago

libheif uses 16 byte (128 bit) alignment for strides. Buffer size is always [m_mem_height stride + alignment - 1] (though it may not start exactly on the first byte, for alignment reasons, from user code assume heightstride), or 0. If you're getting anything else, then I wonder if the handles are getting mixed up, like reusing for thumbnails, or something like that? Can you at least load the image into MediaInfo and see if anything looks different from ones that work?

Chris-R-R commented 3 years ago

The image in question did not have an embedded thumbnail. I'll give MediaInfo a try on thursday. I was not aware of the 16-byte alignment issue, the code I am working with is assuming the image data starts directly on the first byte of the buffer returned by heif_image_get_plane_readonly() and that seems to be true for all of the examples I have seen so far but perhaps the buffer returned has always been correctly aligned. Would switching out the libde265 version possibly affect things?

silverbacknet commented 3 years ago

I mean internally, the buffer is not exactly the same as the buffer returned, if it's not allocated aligned, but that's purely internal bookkeeping. The buffer you get returned always starts at the first byte of the image data.

Chris-R-R commented 3 years ago

Ok I understand. I'm mostly interested in using this for converting to other formats so I haven't really looked at the inner workings of the library. I will see if MediaInfo shows anything special about the problematic files and update the issue. I believe the behaviour has changed since earlier versions of the library - it seems some files that worked in an older version are causing problems in 1.9.1 but it's possible that the access violation was just avoided there because of the heap layout.

Chris-R-R commented 3 years ago

This is the info returned by MediaInfo: General Complete name : D:\heifcrash\IMG_2209.HEIC Format : heic Codec ID : heic (heic/mif1) File size : 182 KiB

Image ID : 1 Format : HEVC Format/Info : High Efficiency Video Coding Format profile : Main Still@L3.1@Main Codec ID : hvc1 Codec ID/Info : High Efficiency Video Coding Width : 814 pixels Height : 860 pixels Color space : YUV Chroma subsampling : 4:2:0 Bit depth : 8 bits Stream size : 181 KiB (99%) Matrix coefficients : BT.601 Color range : Full Describes : 2 Codec configuration box : hvcC

Chris-R-R commented 3 years ago

I have done some debugging. The height returned by heif_image_handle_get_height does not match ImagePlane::m_height

Chris-R-R commented 3 years ago

heif_image_handle_get_height returns 0x32e and the other height is 0x15c

Chris-R-R commented 3 years ago

It seems heif_image_get_height() retuirns the correct height. I was not aware of the differences between these 2 functions or that they could return differing sizes.

Chris-R-R commented 3 years ago

I can avoid the crash using heif_image_get_height() but then I only get half of the image. Decoding with the CopyTrans HEIC codec allows the whole image to be viewed for example, with this image.

Chris-R-R commented 3 years ago

Using libde265-1.0.8 seems to give the same result

farindk commented 3 years ago

Hi Chris, the size returned by heif_image_handle_get_height() is from the metadata stored in the HEIF while heif_image_get_height() is from the encoded image itself. Hence, I assume that the values stored in the metadata or simply wrong or that the image is composed of tiles and the primary image is set wrong.

Too bad that you cannot send an example image, but you could paste the output of heif-info -d FILENAME here to show the inner structure of the image file.

kleisauke commented 3 years ago

@farindk I think processing the image corresponding to CVE-2020-10251 with the code above causes an out-of-bounds read. I've mirrored this image here: https://t0.nl/poc.heic.

Chris-R-R commented 3 years ago

MIME type: image/heic Box: ftyp ----- size: 24 (header size: 8) major brand: heic minor version: 0 compatible brands: heic,mif1

Box: meta ----- size: 1021 (header size: 12) version: 0 flags: 0 Box: hdlr ----- size: 34 (header size: 12) version: 0 flags: 0 pre_defined: 0 handler_type: pict name:
Box: dinf -----
size: 36 (header size: 8)
Box: dref -----
size: 28 (header size: 12)
version: 0
flags: 0
Box: url -----
size: 12 (header size: 12)
version: 0
flags: 1
location:
Box: pitm -----
size: 14 (header size: 12)
version: 0
flags: 0
item_ID: 1
Box: iinf -----
size: 56 (header size: 12)
version: 0
flags: 0
Box: infe -----
size: 21 (header size: 12)
version: 2
flags: 0
item_ID: 1
item_protection_index: 0
item_type: hvc1
item_name:
content_type:
content_encoding:
item uri type:
hidden item: false
Box: infe -----
size: 21 (header size: 12)
version: 2
flags: 1
item_ID: 2
item_protection_index: 0
item_type: Exif
item_name:
content_type:
content_encoding:
item uri type:
hidden item: true
Box: iref -----
size: 26 (header size: 12)
version: 0
flags: 0
reference with type 'cdsc' from ID: 2 to IDs: 1
Box: iprp -----
size: 799 (header size: 8)
Box: ipco -----
size: 766 (header size: 8)
Box: colr -----
size: 560 (header size: 8)
colour_type: prof
profile size: 548
Box: hvcC -----
size: 113 (header size: 8)
configuration_version: 1
general_profile_space: 0
general_tier_flag: 0
general_profile_idc: 3
general_profile_compatibility_flags: 0111.0000 0000.0000 0000.0000 0000.0000
general_constraint_indicator_flags: 10110000 00000000 00000000 00000000 00000000 00000000
general_level_idc: 93
min_spatial_segmentation_idc: 0
parallelism_type: 0
chroma_format: 1
bit_depth_luma: 8
bit_depth_chroma: 8
avg_frame_rate: 0
constant_frame_rate: 0
num_temporal_layers: 1
temporal_id_nested: 0
length_size: 4
array_completeness: 0
NAL_unit_type: 32
40 01 0c 01 ff ff 03 70 00 00 03 00 b0 00 00 03 00 00 03 00 5d 70 24
array_completeness: 0
NAL_unit_type: 33
42 01 01 03 70 00 00 03 00 b0 00 00 03 00 00 03 00 5d a0 06 62 00 d8 75 78 87 b9 16 55 37 02 02 06 00 80
array_completeness: 0
NAL_unit_type: 34
44 01 c0 61 72 c8 40 53 24
Box: ispe -----
size: 20 (header size: 12)
version: 0
flags: 0
image width: 814
image height: 860
Box: clap -----
size: 40 (header size: 8)
clean_aperture: 814/1 x 859/1
offset: 0/1 ; -4194304/8388608
Box: irot -----
size: 9 (header size: 8)
rotation: 0 degrees (CCW)
Box: pixi -----
size: 16 (header size: 12)
version: 0
flags: 0
bits_per_channel: 8,8,8
Box: ipma -----
size: 25 (header size: 12)
version: 0
flags: 0
associations for item ID: 1
property index: 1 (essential: true)
property index: 2 (essential: true)
property index: 3 (essential: false)
property index: 4 (essential: true)
property index: 5 (essential: true)
property index: 6 (essential: false)
Box: iloc -----
size: 44 (header size: 12)
version: 0
flags: 0
item ID: 1
construction method: 0
data_reference_index: 0
base_offset: 0
extents: 1191,185559
item ID: 2
construction method: 0
data_reference_index: 0
base_offset: 0
extents: 1061,130

Box: mdat ----- size: 185705 (header size: 16)

Chris-R-R commented 3 years ago

The Microsoft heic codec can also display the image I've been having problems with on Windows 10.

farindk commented 3 years ago

@Chris-R-R The file appears broken. The ispe box claims that the image size is 814x860 while, in reality, it is cropped to 814x815 (clap box). Also the vertical offset in the clap box is very strange (but valid).

Chris-R-R commented 3 years ago

It only seems to be able to decode 0x15c lines (348 decimal) so it's not even decoding the 815 cropped lines you mention.