[FFmpeg-devel] [PATCH] some length validation for mjpegdec

[Michael Niedermayer]

On Sat, Jul 24, 2010 at 10:25:17AM +0200, Reimar D?ffinger wrote:
> On Sat, Jul 24, 2010 at 02:54:58AM +0200, Michael Niedermayer wrote:
> > > @@ -225,6 +232,15 @@
> > >      if (nb_components <= 0 ||
> > >          nb_components > MAX_COMPONENTS)
> > >          return -1;
> > > +    if (len != 8 + 3*nb_components)
> > > +    {
> > > +        av_log(s->avctx, AV_LOG_DEBUG, "decode_sof0: error, len(%d) mismatch\n", len);
> > > +        len = 8 + 3*nb_components;
> > > +    }
> > > +    if (len > (get_bits_left(&s->gb) + 4*16) >> 3) {
> > > +        av_log(s->avctx, AV_LOG_ERROR, "decode_sof: not enough data\n");
> > > +        return -1;
> > > +    }
> > 
> > this is hmm
> > len is not used anywhere, setting it to a wrong value
> > is just wrong
> > the actual amount read could be tested without messing with len
> 
> Sure, it's just that the len check was already there and I thought
> allowing an inconsistent len value would be a bad idea.

i dont see how a larger len value would be inconsistent
and a smaller would indicate a non standard format/ommision of something


> However I also didn't want to fail on it, since having a check there
> that did not fail seemed like an indication that there might
> be files out there that are broken that way.
> Is there any alternative you like particularly?

FFMAX(what we read, len)<<3 > get_bits_left(&s->gb)

also AV_LOG_DEBUG and "error" in the string dont match


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100724/bf064fd1/attachment.pgp>