Re: [FFmpeg-devel] [PATCH 1/4] avcodec/jpeg2000htdec: Check M_b / magp before using it in a shift

["Tomas Härdin" <git@haerdin.se>]

ons 2024-03-20 klockan 03:59 +0100 skrev Michael Niedermayer:
> Fixes: shift exponent -1 is negative
> Fixes: 65378/clusterfuzz-testcase-minimized-
> ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5457678193197056
> 
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/jpeg2000htdec.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c
> index 6b9898d3ff..0b94bb5da2 100644
> --- a/libavcodec/jpeg2000htdec.c
> +++ b/libavcodec/jpeg2000htdec.c
> @@ -1193,6 +1193,9 @@ ff_jpeg2000_decode_htj2k(const
> Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *c
>  
>      int32_t M_b = magp;
>  
> +    if (magp >= 31)
> +        return AVERROR_INVALIDDATA;

This isn't where the error is, assuming it even is an error. It's
either expn or nguardbits that are wrong, and they should be detected
and reported as such in jpeg2000dec.c. Checking this in every call to
ff_jpeg2000_decode_htj2k() is wasteful.

nguardbits can be 0..7 and expn can be 0..31. Table A.11 indicates that
Ssize can be up to 38 bits, so M_b >= 31 is in fact perfectly valid. A
more appropriate error might be AVERROR_PATCHWELCOME.

While we're on the topic, it seems get_qcx() could be more strict when
it comes to the size of JPEG2000_QCD.

I see some TODOs around this stuff as well, init_band_stepsize()

/Tomas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".