[FFmpeg-soc] [soc]: r2578 - mlp/mlpdec.c

[Michael Niedermayer]

On Thu, Jun 26, 2008 at 07:26:20PM +0100, Ramiro Polla wrote:
> Hello,
> 
> ramiro wrote:
> > Author: ramiro
> > Date: Thu Jun 26 19:33:28 2008
> > New Revision: 2578
> > 
> > Log:
> > More checks to see if there is enough data.
> > 
> > Modified:
> >    mlp/mlpdec.c
> > 
> > Modified: mlp/mlpdec.c
> > ==============================================================================
> > --- mlp/mlpdec.c	(original)
> > +++ mlp/mlpdec.c	Thu Jun 26 19:33:28 2008
> > @@ -1009,6 +1009,9 @@ static int read_access_unit(AVCodecConte
> >      for (substr = 0; substr < m->num_substreams; substr++) {
> >          int extraword_present, checkdata_present, end;
> >  
> > +        if (bytes_left < 2)
> > +            return -1;
> > +
> >          extraword_present = get_bits1(&gb);
> >          skip_bits1(&gb);
> >          checkdata_present = get_bits1(&gb);
> > @@ -1022,6 +1025,8 @@ static int read_access_unit(AVCodecConte
> >          bytes_left -= 2;
> >  
> >          if (extraword_present) {
> > +            if (bytes_left < 2)
> > +                return -1;
> >              skip_bits(&gb, 16);
> >              parity_bits ^= *buf++;
> >              parity_bits ^= *buf++;
> 
> This seems like a long and painful road... While we're still reading 
> headers with easily calculated size, this isn't too hard, and there 
> won't be much overhead in the checks. But on more complicated headers 
> and specially the VLC values it's not easy to see if we still have 
> enough data.
> 
> Lots of other codecs I've looked at seem to just trust init_get_bits() 
> with the remaining bytes.
> 
> What's the best practice here? Should the header checksum && coded 
> lengths be enough to validate the input size?

All _writes_ must be checked, the reads are not critical.
also dont forget FF_INPUT_BUFFER_PADDING_SIZE

about other codecs
mpeg & h26* are designed so 23 zero bits can never occur in a valid bitstream
that way the 64 zero bit at the end are guranteed to trigger some kind of
error.

I definitly do not want the code to be salted with input buffer checks in
every second line!

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

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20080626/ff10b3b1/attachment.pgp>