[FFmpeg-devel] [PATCH 1/5] avcodec/atrac9dec: Clamp band_ext_data to max that can be read if skipped.

Michael Niedermayer michael at niedermayer.cc
Sat Dec 28 15:58:53 EET 2019


On Sat, Dec 28, 2019 at 02:03:37PM +0100, Lynne wrote:
> Dec 27, 2019, 21:35 by michael at niedermayer.cc:
> 
> > On Thu, Dec 26, 2019 at 05:24:31PM +0100, Lynne wrote:
> >
> >> Dec 26, 2019, 13:57 by dev at lynne.ee:
> >>
> >> > Dec 16, 2019, 23:19 by michael at niedermayer.cc:
> >> >
> >> >> Fixes: out of array read
> >> >> Fixes: 19327/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ATRAC9_fuzzer-5679823087468544
> >> >>
> >> >> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >> >> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc> >> >>
> >>
> >> Actually nevermind, patch is good as-is. I think a 0 len just means to reuse the parameters from the previous. So clipping them should be fine.
> >> You should replace the FFMIN(val, (len << 1) - 1) with a av_clip_uintp2(val, len) which does exactly that.
> >>
> >
> > i think both the FFMIN and av_clip_uintp2 do the same thing here, unless
> > iam missing something. But i agree
> > The later is a bit prettier, and as you prefer it too, ill change it and will apply with that
> >
> 
> Why did you use av_clip_uintp2_c() instead of av_clip_uintp2()?

Because len is not a constant, av_clip_uintp2() only works with constants
(it will fail to build even on ARM for example if av_clip_uintp2() is used)

Thx

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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- 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/20191228/a02b9861/attachment.sig>


More information about the ffmpeg-devel mailing list