On 10.01.2016 12:13, Mats Peterson wrote:
> On 01/10/2016 11:56 AM, Andreas Cadhalpun wrote:
>> This fixes segmentation faults due to out of bounds writes, when
>> color_start is interpreted as negative number.
>>
>> This regression was introduced in commit 57631f.
>>
>> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
>> ---
>>
>> Seriously, changing the code behavior when "factoring out" is a
>> very bad practice.
>>
>> ---
>> libavformat/qtpalette.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavformat/qtpalette.c b/libavformat/qtpalette.c
>> index a78b6af..666c6b7 100644
>> --- a/libavformat/qtpalette.c
>> +++ b/libavformat/qtpalette.c
>> @@ -48,7 +48,7 @@ int ff_get_qtpalette(int codec_id, AVIOContext *pb,
>> uint32_t *palette)
>>
>> /* If the depth is 1, 2, 4, or 8 bpp, file is palettized. */
>> if ((bit_depth == 1 || bit_depth == 2 || bit_depth == 4 || bit_depth
>> == 8)) {
>> - int color_count, color_start, color_end;
>> + uint32_t color_count, color_start, color_end;
>> uint32_t a, r, g, b;
>>
>> /* Ignore the greyscale bit for 1-bit video and sample
>>
>
> As far as I remember, those variables were ints in the original code in mov.c,
You remember wrongly:
$ git show 57631f
[...]
- unsigned int color_start, color_count, color_end;
- unsigned int a, r, g, b;
[...]
+ int color_count, color_start, color_end;
+ uint32_t a, r, g, b;
> and they will hardly reach a value where they could be interpreted as ints.
Wrong again, as they can be arbitrary values read from the input file:
color_start = avio_rb32(pb);
> But it's of course better to be on the safe side.
Indeed, as otherwise it is a security vulnerability.
Best regards,
Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
