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

[Michael Niedermayer]

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


> +
> +    if ((ret = decode_zbuf(&bp, s->gb.buffer, s->gb.buffer + length) < 0))

() is placed incorrectly


> +        return ret;
> +
> +    av_bprint_finalize(&bp, (char **)&data);
> +
> +    if (!(sd = av_frame_new_side_data(f, AV_FRAME_DATA_ICC_PROFILE, bp.len)))
> +        return AVERROR(ENOMEM);
> +
> +    av_dict_set(&sd->metadata, "name", profile_name, 0);
> +    memcpy(sd->data, data, bp.len);
> +    av_free(data);
> +
> +    /* ICC compressed data and CRC */
> +    bytestream2_skip(&s->gb, length + 4);
> +
> +    return 0;
> +}
> +
>  static void handle_small_bpp(PNGDecContext *s, AVFrame *p)
>  {
>      if (s->bits_per_pixel == 1 && s->color_type == PNG_COLOR_TYPE_PALETTE) {
> @@ -1239,6 +1279,11 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
>              bytestream2_skip(&s->gb, 4); /* crc */
>              break;
>          }
> +        case MKTAG('i', 'C', 'C', 'P'): {
> +            if (decode_iccp_chunk(s, length, p) < 0)
> +                goto fail;
> +            break;
> +        }
>          case MKTAG('I', 'E', 'N', 'D'):
>              if (!(s->pic_state & PNG_ALLIMAGE))
>                  av_log(avctx, AV_LOG_ERROR, "IEND without all image\n");
> -- 
> 2.14.0.rc0.284.gd933b75aa4
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- 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/20170721/54c85ccb/attachment.sig>