[FFmpeg-devel] [PATCH] tiffdec: support embedded ICC profiles

[Lynne]

Jan 13, 2020, 20:20 by derek.buitenhuis at gmail.com:

> On 13/01/2020 17:50, Lynne wrote:
>
>> Actually the entire condition needs to be gone. count is uint32_t. The length is already checked below.
>> Copied this from ff_tadd_shorts_metadata which has an int count.
>>
>
> Ah.
>
>> And the offset value isn't taken into account. TIFF allows the ICC profile to be placed pretty much anywhere within the file (the 32 bit offset points from the start of the file, not the field). So this would only work with files where the ICC profile immediately follows the tag.
>>
>
> Oh, that's a bit wild. I did a quick Google search for the ICC profile tag
> spec, and it wasn't obvious to me where it is defined, so I assumed it was
> directly after the tag.
>

Yeah, I thought it was after the tag too with the offset being used to align to the nearest 16bits or maybe for some padding. But for the sample I have the position of the offset and the current byte in the bytestream reader match, and the ICC spec (which defines TIFF encapsulation) says the start of the file, so I can't argue with that.



>> Attached a new patch.
>> +        gb_temp = s->gb;
>> +        bytestream2_seek(&gb_temp, SEEK_SET, off);
>> +
>> +        if (bytestream2_get_bytes_left(&gb_temp) < count)
>> +            return AVERROR_INVALIDDATA;
>>
>
> Is it worth checking the bytestream2_seek return value too, or will that
> be handled by bytestream2_get_bytes_left anyway? If it is handled, patch
> seems OK.
>

bytestream2_seek returns the amount of bytes since the start of the buffer after seeking. It clips to the size of the buffer so you can't seek past the end.