[FFmpeg-devel] [PATCH 19/19] lavc: sanitize decoder return value.

[Michael Niedermayer]

On Tue, Jul 31, 2012 at 06:40:54PM +0200, Nicolas George wrote:
> Le duodi 12 thermidor, an CCXX, Michael Niedermayer a écrit :
> > > +/* Note: we are still vulnerable to decoders decoding a frame and
> > > +   returning 0 for success instead of the used bytes. */
> > > +#define SANITIZE_DECODED_SIZE(ret, pkt, got_frame) {                   \
> > > +    av_assert2((ret) <= (pkt).size);                                   \
> > > +    (ret) = FFMIN((ret), (pkt).size);                                  \
> > > +    av_assert2(!(pkt).size || (got_frame) || (ret));                   \
> > > +    if ((pkt).size && !(got_frame) && !(ret))                          \
> > > +        ret = AVERROR_BUG;                                             \
> > > +}
> > I think the asserts alone should be enough
> 
> Are you sure? The asserts only affect lavc built for debugging. For normal
> builds, the applications would not be able to trust the return value of
> avcodec_decode_<type>() unconditionally, because there may still be codecs
> that give a bogus return value and are not yet fixed (like the three that
> overread), or even not yet detected.

IMHO either add no assert at all or add just a assert and we fix the
cases where it fails if any exist (and maybe a av_assert0 would be
better here at leastr until its all fixed then it could be changed to
av_assert2 if there are speed concerns)

[...]


-- 
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: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120802/33049aa9/attachment.asc>