[FFmpeg-devel] [PATCH] svq1enc: fix out of bounds reads

[Andreas Cadhalpun]

On 22.01.2016 00:57, Michael Niedermayer wrote:
> On Thu, Jan 21, 2016 at 11:04:14PM +0100, Andreas Cadhalpun wrote:
>> level can be up to 5, but there are only four codebooks.
>>
>> Fixes ubsan runtime error: index 5 out of bounds for type 'int8_t
>> [4][96]'
>>
>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>> ---
>>  libavcodec/svq1enc.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
>> index 1e1745e..7ff72b4 100644
>> --- a/libavcodec/svq1enc.c
>> +++ b/libavcodec/svq1enc.c
>> @@ -104,7 +104,7 @@ static int encode_block(SVQ1EncContext *s, uint8_t *src, uint8_t *ref,
>>      best_score = 0;
>>      // FIXME: Optimize, this does not need to be done multiple times.
>>      if (intra) {
>> -        codebook_sum   = svq1_intra_codebook_sum[level];
>> +        codebook_sum   = level < 4 ? svq1_intra_codebook_sum[level] : NULL;
>>          codebook       = ff_svq1_intra_codebooks[level];
>>          mean_vlc       = ff_svq1_intra_mean_vlc;
>>          multistage_vlc = ff_svq1_intra_multistage_vlc[level];
>> @@ -117,7 +117,7 @@ static int encode_block(SVQ1EncContext *s, uint8_t *src, uint8_t *ref,
>>              }
>>          }
>>      } else {
>> -        codebook_sum   = svq1_inter_codebook_sum[level];
>> +        codebook_sum   = level < 4 ? svq1_inter_codebook_sum[level] : NULL;
>>          codebook       = ff_svq1_inter_codebooks[level];
>>          mean_vlc       = ff_svq1_inter_mean_vlc + 256;
>>          multistage_vlc = ff_svq1_inter_multistage_vlc[level];
> 
>> @@ -143,7 +143,7 @@ static int encode_block(SVQ1EncContext *s, uint8_t *src, uint8_t *ref,
>>              const int8_t *vector;
>>  
>>              for (i = 0; i < 16; i++) {
>> -                int sum = codebook_sum[stage * 16 + i];
>> +                int sum = codebook_sum ? codebook_sum[stage * 16 + i] : 0;
>>                  int sqr, diff, score;
> 
> this is uneeded, it cannot be NULL

Indeed. That explains how FATE could pass...

> the other 2 checks should be commented to say something about the
>> = 4 case being unused

Attached patch has comments there.

Best regards,
Andreas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-svq1enc-fix-out-of-bounds-reads.patch
Type: text/x-diff
Size: 1809 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160126/2b4b025d/attachment.patch>