On 25 July 2017 at 17:33, Rostislav Pehlivanov <atomnu...@gmail.com> wrote:
>
>
> On 23 July 2017 at 13:36, Michael Niedermayer <mich...@niedermayer.cc>
> wrote:
>
>> On Sat, Jul 22, 2017 at 09:15:53PM +0100, Rostislav Pehlivanov wrote:
>> > On 21 July 2017 at 14:11, Michael Niedermayer <mich...@niedermayer.cc>
>> > wrote:
>> >
>> > > On Thu, Jul 20, 2017 at 09:46:22PM +0100, Rostislav Pehlivanov wrote:
>> > > > Signed-off-by: Rostislav Pehlivanov <atomnu...@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.
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>
> Changed to an int as discussed on IRC
>
> I'll give it a few more hours before I push both patches
>
Thanks for the review, pushed with your suggestions.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
