[FFmpeg-devel] [PATCH] avformat/icodec: Fix crash probing fuzzed file

[Michael Niedermayer]

On Mon, Feb 15, 2016 at 11:27:20AM -0800, Mark Harris wrote:
> On Mon, Feb 15, 2016 at 11:02 AM, Michael Niedermayer
> <michael at niedermayer.cc> wrote:
> > On Mon, Feb 15, 2016 at 09:57:51AM -0800, Mark Harris wrote:
> >> Avoid invalid memory read/crash when ico offset >= 0xfffffff8.
> >> Base64-encoded example: AAABADAwMDAwMAAAMAAwMDAw/P///w==
> >> ---
> >>  libavformat/icodec.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavformat/icodec.c b/libavformat/icodec.c
> >> index 6ddb901..8f84337 100644
> >> --- a/libavformat/icodec.c
> >> +++ b/libavformat/icodec.c
> >> @@ -60,7 +60,7 @@ static int probe(AVProbeData *p)
> >>          offset = AV_RL32(p->buf + 18 + i * 16);
> >>          if (offset < 22)
> >>              return FFMIN(i, AVPROBE_SCORE_MAX / 4);
> >> -        if (offset + 8 > p->buf_size)
> >> +        if (offset > p->buf_size - 8)
> >
> > buf_size - 8 can underflow or more precissely is not guranteed to be
> > representable as unsigned while the compare is using unsigned
> >
> 
> If p->buf_size was less than 8, would it not have returned before
> this?  AV_RL32(p->buf + 14) would be 0 and offset = AV_RL32(p->buf +
> 18) would be 0, due to the zero padding of the probe buffer.

you are correct, i didnt think about that but I would suggest to
make the check work independant of that, who knows, the code could
be factored out or the array might not be zero padded for whatever
reason.
If the code depends on such assumtations that should be clearly
documented so one changing the zero padding would know that this
needs updating. But its easier to use 8LL instead of 8, which should
avoid the problem or to make offset uint64_t

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

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160216/3ef45422/attachment.sig>