[FFmpeg-devel] [PATCH] IFF: Add error checking to byterun1 decoder

[Sebastian Vater]

Michael Niedermayer a ?crit :
>
>>  iff.c |   58 ++++++++++++++++++++++++++++++++++++++++------------------
>>  1 file changed, 40 insertions(+), 18 deletions(-)
>> 8519ee6c6a67610cb02eb831b3261e324bcee6ed  iff-byterun1-error.patch
>> diff --git a/libavcodec/iff.c b/libavcodec/iff.c
>> index bcf4929..5e4c55a 100644
>> --- a/libavcodec/iff.c
>> +++ b/libavcodec/iff.c
>> @@ -226,27 +226,43 @@ static void decodeplane32(uint32_t *dst, const uint8_t *buf, int buf_size, int p
>>   * @param dst_size the destination plane size in bytes
>>   * @param buf the source byterun1 compressed bitstream
>>   * @param buf_end the EOF of source byterun1 compressed bitstream
>> - * @return number of consumed bytes in byterun1 compressed bitstream
>> + * @return consumed bytes in compressed bitstream or negative error code otherwise
>>  */
>> -static int decode_byterun(uint8_t *dst, int dst_size,
>> -                          const uint8_t *buf, const uint8_t *const buf_end) {
>> +static av_always_inline int decode_byterun(AVCodecContext *const avctx,
>> +                                           uint8_t *dst, int dst_size,
>> +                                           const uint8_t *buf,
>> +                                           const uint8_t *const buf_end) {
>>      const uint8_t *const buf_start = buf;
>> -    unsigned x;
>> -    for (x = 0; x < dst_size && buf < buf_end;) {
>> -        unsigned length;
>> +    if (buf >= buf_end) {
>> +        av_log(avctx, AV_LOG_ERROR, "IFF byterun1 stream buffer overflow\n");
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +    do {
>>          const int8_t value = *buf++;
>>          if (value >= 0) {
>> -            length = value + 1;
>> -            memcpy(dst + x, buf, FFMIN3(length, dst_size - x, buf_end - buf));
>> +            const int length = (unsigned) value + 1;
>>     
>
> whats that cast good for?
>   

Preventing it to create a sign-extend instruction (at least on m68k)
which isn't necessary because value is always positive.

-- 

Best regards,
                   :-) Basty/CDGS (-: