diff mbox

[FFmpeg-devel,2/2] avcodec/hevc_cabac: Check prefix so as to avoid invalid shifts in coeff_abs_level_remaining_decode()

Message ID 20180115233728.15279-2-michael@niedermayer.cc
State Accepted
Commit a026a3efaeb9c2026668dccbbda339a21ab3206b
Headers show

Commit Message

Michael Niedermayer Jan. 15, 2018, 11:37 p.m. UTC
I suspect that this can be limited tighter, but i failed to find anything
in the spec that would confirm that.

Fixes: 4833/clusterfuzz-testcase-minimized-5302840101699584
Fixes: runtime error: left shift of 134217730 by 4 places cannot be represented in type 'int'

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/hevc_cabac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ronald S. Bultje Jan. 16, 2018, 12:33 a.m. UTC | #1
Hi,

On Mon, Jan 15, 2018 at 6:37 PM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

> I suspect that this can be limited tighter, but i failed to find anything
> in the spec that would confirm that.
>
> Fixes: 4833/clusterfuzz-testcase-minimized-5302840101699584
> Fixes: runtime error: left shift of 134217730 by 4 places cannot be
> represented in type 'int'
>
> Found-by: continuous fuzzing process https://github.com/google/oss-
> fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/hevc_cabac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/hevc_cabac.c b/libavcodec/hevc_cabac.c
> index 743168500c..faa36d5459 100644
> --- a/libavcodec/hevc_cabac.c
> +++ b/libavcodec/hevc_cabac.c
> @@ -998,7 +998,7 @@ static av_always_inline int coeff_abs_level_remaining_decode(HEVCContext
> *s, int
>      } else {
>          int prefix_minus3 = prefix - 3;
>
> -        if (prefix == CABAC_MAX_BIN) {
> +        if (prefix == CABAC_MAX_BIN || prefix_minus3 + rc_rice_param >=
> 31) {
>              av_log(s->avctx, AV_LOG_ERROR, "CABAC_MAX_BIN : %d\n",
> prefix);
>              return 0;
>          }


I understand this is unrelated to the patch, but I once again want to point
out how utterly useless this error message is for end users :-(.

Ronald
James Almer Jan. 16, 2018, 1:38 a.m. UTC | #2
On 1/15/2018 9:33 PM, Ronald S. Bultje wrote:
> Hi,
> 
> On Mon, Jan 15, 2018 at 6:37 PM, Michael Niedermayer <michael@niedermayer.cc
>> wrote:
> 
>> I suspect that this can be limited tighter, but i failed to find anything
>> in the spec that would confirm that.
>>
>> Fixes: 4833/clusterfuzz-testcase-minimized-5302840101699584
>> Fixes: runtime error: left shift of 134217730 by 4 places cannot be
>> represented in type 'int'
>>
>> Found-by: continuous fuzzing process https://github.com/google/oss-
>> fuzz/tree/master/projects/ffmpeg
>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>> ---
>>  libavcodec/hevc_cabac.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/hevc_cabac.c b/libavcodec/hevc_cabac.c
>> index 743168500c..faa36d5459 100644
>> --- a/libavcodec/hevc_cabac.c
>> +++ b/libavcodec/hevc_cabac.c
>> @@ -998,7 +998,7 @@ static av_always_inline int coeff_abs_level_remaining_decode(HEVCContext
>> *s, int
>>      } else {
>>          int prefix_minus3 = prefix - 3;
>>
>> -        if (prefix == CABAC_MAX_BIN) {
>> +        if (prefix == CABAC_MAX_BIN || prefix_minus3 + rc_rice_param >=
>> 31) {
>>              av_log(s->avctx, AV_LOG_ERROR, "CABAC_MAX_BIN : %d\n",
>> prefix);
>>              return 0;
>>          }
> 
> 
> I understand this is unrelated to the patch, but I once again want to point
> out how utterly useless this error message is for end users :-(.
> 
> Ronald

This one is particularly bizarre as well. It prints "CABAC_MAX_BIN"
followed by it's constant value, as if it could change.
That apparently will not be the case anymore after this patch, though,
with the new check.
Michael Niedermayer Jan. 23, 2018, 6:44 p.m. UTC | #3
On Tue, Jan 16, 2018 at 12:37:28AM +0100, Michael Niedermayer wrote:
> I suspect that this can be limited tighter, but i failed to find anything
> in the spec that would confirm that.
> 
> Fixes: 4833/clusterfuzz-testcase-minimized-5302840101699584
> Fixes: runtime error: left shift of 134217730 by 4 places cannot be represented in type 'int'
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/hevc_cabac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

will apply this in a few days unless someone wants me to wait

[...]
Michael Niedermayer Jan. 27, 2018, 2:51 a.m. UTC | #4
On Tue, Jan 23, 2018 at 07:44:24PM +0100, Michael Niedermayer wrote:
> On Tue, Jan 16, 2018 at 12:37:28AM +0100, Michael Niedermayer wrote:
> > I suspect that this can be limited tighter, but i failed to find anything
> > in the spec that would confirm that.
> > 
> > Fixes: 4833/clusterfuzz-testcase-minimized-5302840101699584
> > Fixes: runtime error: left shift of 134217730 by 4 places cannot be represented in type 'int'
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/hevc_cabac.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> will apply this in a few days unless someone wants me to wait

applied


[...]
diff mbox

Patch

diff --git a/libavcodec/hevc_cabac.c b/libavcodec/hevc_cabac.c
index 743168500c..faa36d5459 100644
--- a/libavcodec/hevc_cabac.c
+++ b/libavcodec/hevc_cabac.c
@@ -998,7 +998,7 @@  static av_always_inline int coeff_abs_level_remaining_decode(HEVCContext *s, int
     } else {
         int prefix_minus3 = prefix - 3;
 
-        if (prefix == CABAC_MAX_BIN) {
+        if (prefix == CABAC_MAX_BIN || prefix_minus3 + rc_rice_param >= 31) {
             av_log(s->avctx, AV_LOG_ERROR, "CABAC_MAX_BIN : %d\n", prefix);
             return 0;
         }