[FFmpeg-devel] [PATCH 2/2] pngdec: decode and expose iCCP chunks as side data

[Michael Niedermayer]

On Sat, Jul 22, 2017 at 09:15:53PM +0100, Rostislav Pehlivanov wrote:
> On 21 July 2017 at 14:11, Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > On Thu, Jul 20, 2017 at 09:46:22PM +0100, Rostislav Pehlivanov wrote:
> > > Signed-off-by: Rostislav Pehlivanov <atomnuker at gmail.com>
> > > ---
> > >  libavcodec/pngdec.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 45 insertions(+)
> > >
> > > diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> > > index 083f61f4f8..64811c6fc3 100644
> > > --- a/libavcodec/pngdec.c
> > > +++ b/libavcodec/pngdec.c
> > > @@ -836,6 +836,46 @@ static int decode_trns_chunk(AVCodecContext
> > *avctx, PNGDecContext *s,
> > >      return 0;
> > >  }
> > >
> > > +static int decode_iccp_chunk(PNGDecContext *s, uint32_t length,
> > AVFrame *f)
> > > +{
> > > +    int ret, cnt = 0;
> > > +    uint8_t *data, profile_name[82];
> > > +    AVBPrint bp;
> > > +    AVFrameSideData *sd;
> > > +
> > > +    while ((profile_name[cnt++] = bytestream2_get_byte(&s->gb)) && cnt
> > < 81);
> > > +    if (cnt > 80) {
> > > +        av_log(s->avctx, AV_LOG_ERROR, "iCCP with invalid name!\n");
> > > +        return AVERROR_INVALIDDATA;
> > > +    }
> > > +
> > > +    length -= cnt;
> > > +
> > > +    if (bytestream2_get_byte(&s->gb) != 0) {
> > > +        av_log(s->avctx, AV_LOG_ERROR, "iCCP with invalid
> > compression!\n");
> > > +        return AVERROR_INVALIDDATA;
> > > +    }
> > > +
> > > +    length -= 1;
> >
> > length could have overflowed and become rather big from one of the 2
> > subtractions
> > the following code would then misbehave
> >
> >
> Thanks to pointing this out
> 
> Changed to:
> + length = FFMAX(length - cnt, 0);

if length is unsigned int and cnt signed int then length - cnt is unsigned
so the FFMAX will not work as intended


> and
> + length = FFMAX(length - 1, 0);
> 

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

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- 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/20170723/fe93621e/attachment.sig>