[FFmpeg-devel] [PATCH 1/2] avcodec/mpeg12dec: Fix invalid shift in mpeg2_fast_decode_block_intra()

[Michael Niedermayer]

On Thu, Dec 26, 2019 at 02:13:59AM +0000, Kieran Kunhya wrote:
> On Thu, 26 Dec 2019 at 00:27, Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > Fixes: left shift of negative value -695
> > Fixes:
> > 19232/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG1VIDEO_fuzzer-5702856963522560
> > Fixes:
> > 19555/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG1VIDEO_fuzzer-5741218147598336
> >
> > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by
> > <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>:
> > Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/mpeg12dec.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> > index 775579f9f0..4643992d28 100644
> > --- a/libavcodec/mpeg12dec.c
> > +++ b/libavcodec/mpeg12dec.c
> > @@ -586,7 +586,7 @@ static inline int
> > mpeg2_fast_decode_block_intra(MpegEncContext *s,
> 
> 
[...]

> Also it has the following comment associated with it:
> 
> /**
>  * Note: this function can read out of range and crash for corrupt streams.
>  * Changing this would eat up any speed benefits it has.
>  * Do not use "fast" flag if you need the code to be robust.
>  */
> 
> If you want to make it robust you might as well just use the real decode
> function

People wanted to maximize code coverage of the fuzzer. So it fuzzes such
cases too now. 
I dont see any harm in fixing issues like the one this patch is about.
and that the codepath is inherently not robust as part of what its intended
for shouldnt be an argument to not fix issues we can easily fix.

About removing it, its very easy to fix (the case here) but removing it from old releases
would not be easy and also i dont see how that could be justified to its
users.

So i think this patch should be applied

Thanks


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200130/efdb6b63/attachment.sig>