From patchwork Fri Apr 2 14:40:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Khirnov X-Patchwork-Id: 26711 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id C651D44AA9A for ; Fri, 2 Apr 2021 17:40:55 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id B4BEC68A628; Fri, 2 Apr 2021 17:40:55 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 0F66F68089B for ; Fri, 2 Apr 2021 17:40:49 +0300 (EEST) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id 2DF9C240693 for ; Fri, 2 Apr 2021 16:40:44 +0200 (CEST) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id Sy6UCE-g2lxx for ; Fri, 2 Apr 2021 16:40:43 +0200 (CEST) Received: from libav.khirnov.net (unknown [IPv6:2a00:c500:561:201:68fd:4bee:f4:c149]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "libav.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id 7F379240694 for ; Fri, 2 Apr 2021 16:40:39 +0200 (CEST) Received: by libav.khirnov.net (Postfix, from userid 1000) id BBF093A060C; Fri, 2 Apr 2021 16:40:37 +0200 (CEST) From: Anton Khirnov To: ffmpeg-devel@ffmpeg.org Date: Fri, 2 Apr 2021 16:40:33 +0200 Message-Id: <20210402144033.17410-7-anton@khirnov.net> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210402144033.17410-1-anton@khirnov.net> References: <20210402144033.17410-1-anton@khirnov.net> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 7/7] lavc/pngdec: use a separate bytestream reader for each chunk X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" This makes sure that reading a truncated chunk will never overflow into the following chunk. It also allows to remove many repeated lines skipping over the trailing crc checksum. --- libavcodec/pngdec.c | 166 +++++++++++++++++++------------------------- 1 file changed, 72 insertions(+), 94 deletions(-) diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c index 0ff81d740c..562c5ffea4 100644 --- a/libavcodec/pngdec.c +++ b/libavcodec/pngdec.c @@ -421,13 +421,12 @@ the_end:; } } -static int png_decode_idat(PNGDecContext *s, int length, +static int png_decode_idat(PNGDecContext *s, GetByteContext *gb, uint8_t *dst, ptrdiff_t dst_stride) { int ret; - s->zstream.avail_in = FFMIN(length, bytestream2_get_bytes_left(&s->gb)); - s->zstream.next_in = s->gb.buffer; - bytestream2_skip(&s->gb, length); + s->zstream.avail_in = bytestream2_get_bytes_left(gb); + s->zstream.next_in = gb->buffer; /* decode one line if possible */ while (s->zstream.avail_in > 0) { @@ -520,11 +519,11 @@ static uint8_t *iso88591_to_utf8(const uint8_t *in, size_t size_in) return out; } -static int decode_text_chunk(PNGDecContext *s, uint32_t length, int compressed) +static int decode_text_chunk(PNGDecContext *s, GetByteContext *gb, int compressed) { int ret, method; - const uint8_t *data = s->gb.buffer; - const uint8_t *data_end = data + length; + const uint8_t *data = gb->buffer; + const uint8_t *data_end = gb->buffer_end; const uint8_t *keyword = data; const uint8_t *keyword_end = memchr(keyword, 0, data_end - keyword); uint8_t *kw_utf8 = NULL, *text, *txt_utf8 = NULL; @@ -568,9 +567,9 @@ static int decode_text_chunk(PNGDecContext *s, uint32_t length, int compressed) } static int decode_ihdr_chunk(AVCodecContext *avctx, PNGDecContext *s, - uint32_t length) + GetByteContext *gb) { - if (length != 13) + if (bytestream2_get_bytes_left(gb) != 13) return AVERROR_INVALIDDATA; if (s->pic_state & PNG_IDAT) { @@ -583,28 +582,27 @@ static int decode_ihdr_chunk(AVCodecContext *avctx, PNGDecContext *s, return AVERROR_INVALIDDATA; } - s->width = s->cur_w = bytestream2_get_be32(&s->gb); - s->height = s->cur_h = bytestream2_get_be32(&s->gb); + s->width = s->cur_w = bytestream2_get_be32(gb); + s->height = s->cur_h = bytestream2_get_be32(gb); if (av_image_check_size(s->width, s->height, 0, avctx)) { s->cur_w = s->cur_h = s->width = s->height = 0; av_log(avctx, AV_LOG_ERROR, "Invalid image size\n"); return AVERROR_INVALIDDATA; } - s->bit_depth = bytestream2_get_byte(&s->gb); + s->bit_depth = bytestream2_get_byte(gb); if (s->bit_depth != 1 && s->bit_depth != 2 && s->bit_depth != 4 && s->bit_depth != 8 && s->bit_depth != 16) { av_log(avctx, AV_LOG_ERROR, "Invalid bit depth\n"); goto error; } - s->color_type = bytestream2_get_byte(&s->gb); - s->compression_type = bytestream2_get_byte(&s->gb); + s->color_type = bytestream2_get_byte(gb); + s->compression_type = bytestream2_get_byte(gb); if (s->compression_type) { av_log(avctx, AV_LOG_ERROR, "Invalid compression method %d\n", s->compression_type); goto error; } - s->filter_type = bytestream2_get_byte(&s->gb); - s->interlace_type = bytestream2_get_byte(&s->gb); - bytestream2_skip(&s->gb, 4); /* crc */ + s->filter_type = bytestream2_get_byte(gb); + s->interlace_type = bytestream2_get_byte(gb); s->hdr_state |= PNG_IHDR; if (avctx->debug & FF_DEBUG_PICT_INFO) av_log(avctx, AV_LOG_DEBUG, "width=%d height=%d depth=%d color_type=%d " @@ -619,24 +617,24 @@ error: return AVERROR_INVALIDDATA; } -static int decode_phys_chunk(AVCodecContext *avctx, PNGDecContext *s) +static int decode_phys_chunk(AVCodecContext *avctx, PNGDecContext *s, + GetByteContext *gb) { if (s->pic_state & PNG_IDAT) { av_log(avctx, AV_LOG_ERROR, "pHYs after IDAT\n"); return AVERROR_INVALIDDATA; } - avctx->sample_aspect_ratio.num = bytestream2_get_be32(&s->gb); - avctx->sample_aspect_ratio.den = bytestream2_get_be32(&s->gb); + avctx->sample_aspect_ratio.num = bytestream2_get_be32(gb); + avctx->sample_aspect_ratio.den = bytestream2_get_be32(gb); if (avctx->sample_aspect_ratio.num < 0 || avctx->sample_aspect_ratio.den < 0) avctx->sample_aspect_ratio = (AVRational){ 0, 1 }; - bytestream2_skip(&s->gb, 1); /* unit specifier */ - bytestream2_skip(&s->gb, 4); /* crc */ + bytestream2_skip(gb, 1); /* unit specifier */ return 0; } static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s, - uint32_t length, AVFrame *p) + GetByteContext *gb, AVFrame *p) { int ret; size_t byte_depth = s->bit_depth > 8 ? 2 : 1; @@ -773,7 +771,7 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s, if (s->has_trns && s->color_type != PNG_COLOR_TYPE_PALETTE) s->bpp -= byte_depth; - ret = png_decode_idat(s, length, p->data[0], p->linesize[0]); + ret = png_decode_idat(s, gb, p->data[0], p->linesize[0]); if (s->has_trns && s->color_type != PNG_COLOR_TYPE_PALETTE) s->bpp += byte_depth; @@ -781,14 +779,13 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s, if (ret < 0) return ret; - bytestream2_skip(&s->gb, 4); /* crc */ - return 0; } static int decode_plte_chunk(AVCodecContext *avctx, PNGDecContext *s, - uint32_t length) + GetByteContext *gb) { + int length = bytestream2_get_bytes_left(gb); int n, i, r, g, b; if ((length % 3) != 0 || length > 256 * 3) @@ -796,22 +793,22 @@ static int decode_plte_chunk(AVCodecContext *avctx, PNGDecContext *s, /* read the palette */ n = length / 3; for (i = 0; i < n; i++) { - r = bytestream2_get_byte(&s->gb); - g = bytestream2_get_byte(&s->gb); - b = bytestream2_get_byte(&s->gb); + r = bytestream2_get_byte(gb); + g = bytestream2_get_byte(gb); + b = bytestream2_get_byte(gb); s->palette[i] = (0xFFU << 24) | (r << 16) | (g << 8) | b; } for (; i < 256; i++) s->palette[i] = (0xFFU << 24); s->hdr_state |= PNG_PLTE; - bytestream2_skip(&s->gb, 4); /* crc */ return 0; } static int decode_trns_chunk(AVCodecContext *avctx, PNGDecContext *s, - uint32_t length) + GetByteContext *gb) { + int length = bytestream2_get_bytes_left(gb); int v, i; if (!(s->hdr_state & PNG_IHDR)) { @@ -829,7 +826,7 @@ static int decode_trns_chunk(AVCodecContext *avctx, PNGDecContext *s, return AVERROR_INVALIDDATA; for (i = 0; i < length; i++) { - unsigned v = bytestream2_get_byte(&s->gb); + unsigned v = bytestream2_get_byte(gb); s->palette[i] = (s->palette[i] & 0x00ffffff) | (v << 24); } } else if (s->color_type == PNG_COLOR_TYPE_GRAY || s->color_type == PNG_COLOR_TYPE_RGB) { @@ -840,7 +837,7 @@ static int decode_trns_chunk(AVCodecContext *avctx, PNGDecContext *s, for (i = 0; i < length / 2; i++) { /* only use the least significant bits */ - v = av_mod_uintp2(bytestream2_get_be16(&s->gb), s->bit_depth); + v = av_mod_uintp2(bytestream2_get_be16(gb), s->bit_depth); if (s->bit_depth > 8) AV_WB16(&s->transparent_color_be[2 * i], v); @@ -851,35 +848,30 @@ static int decode_trns_chunk(AVCodecContext *avctx, PNGDecContext *s, return AVERROR_INVALIDDATA; } - bytestream2_skip(&s->gb, 4); /* crc */ s->has_trns = 1; return 0; } -static int decode_iccp_chunk(PNGDecContext *s, int length, AVFrame *f) +static int decode_iccp_chunk(PNGDecContext *s, GetByteContext *gb, AVFrame *f) { int ret, cnt = 0; AVBPrint bp; - while ((s->iccp_name[cnt++] = bytestream2_get_byte(&s->gb)) && cnt < 81); + while ((s->iccp_name[cnt++] = bytestream2_get_byte(gb)) && cnt < 81); if (cnt > 80) { av_log(s->avctx, AV_LOG_ERROR, "iCCP with invalid name!\n"); ret = AVERROR_INVALIDDATA; goto fail; } - length = FFMAX(length - cnt, 0); - - if (bytestream2_get_byte(&s->gb) != 0) { + if (bytestream2_get_byte(gb) != 0) { av_log(s->avctx, AV_LOG_ERROR, "iCCP with invalid compression!\n"); ret = AVERROR_INVALIDDATA; goto fail; } - length = FFMAX(length - 1, 0); - - if ((ret = decode_zbuf(&bp, s->gb.buffer, s->gb.buffer + length)) < 0) + if ((ret = decode_zbuf(&bp, gb->buffer, gb->buffer_end)) < 0) return ret; av_freep(&s->iccp_data); @@ -888,9 +880,6 @@ static int decode_iccp_chunk(PNGDecContext *s, int length, AVFrame *f) return ret; s->iccp_data_len = bp.len; - /* ICC compressed data and CRC */ - bytestream2_skip(&s->gb, length + 4); - return 0; fail: s->iccp_name[0] = 0; @@ -971,12 +960,12 @@ static void handle_small_bpp(PNGDecContext *s, AVFrame *p) } static int decode_fctl_chunk(AVCodecContext *avctx, PNGDecContext *s, - uint32_t length) + GetByteContext *gb) { uint32_t sequence_number; int cur_w, cur_h, x_offset, y_offset, dispose_op, blend_op; - if (length != 26) + if (bytestream2_get_bytes_left(gb) != 26) return AVERROR_INVALIDDATA; if (!(s->hdr_state & PNG_IHDR)) { @@ -995,15 +984,14 @@ static int decode_fctl_chunk(AVCodecContext *avctx, PNGDecContext *s, s->last_y_offset = s->y_offset; s->last_dispose_op = s->dispose_op; - sequence_number = bytestream2_get_be32(&s->gb); - cur_w = bytestream2_get_be32(&s->gb); - cur_h = bytestream2_get_be32(&s->gb); - x_offset = bytestream2_get_be32(&s->gb); - y_offset = bytestream2_get_be32(&s->gb); - bytestream2_skip(&s->gb, 4); /* delay_num (2), delay_den (2) */ - dispose_op = bytestream2_get_byte(&s->gb); - blend_op = bytestream2_get_byte(&s->gb); - bytestream2_skip(&s->gb, 4); /* crc */ + sequence_number = bytestream2_get_be32(gb); + cur_w = bytestream2_get_be32(gb); + cur_h = bytestream2_get_be32(gb); + x_offset = bytestream2_get_be32(gb); + y_offset = bytestream2_get_be32(gb); + bytestream2_skip(gb, 4); /* delay_num (2), delay_den (2) */ + dispose_op = bytestream2_get_byte(gb); + blend_op = bytestream2_get_byte(gb); if (sequence_number == 0 && (cur_w != s->width || @@ -1194,6 +1182,8 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, int i, ret; for (;;) { + GetByteContext gb_chunk; + length = bytestream2_get_bytes_left(&s->gb); if (length <= 0) { @@ -1233,8 +1223,7 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, goto fail; } av_log(avctx, AV_LOG_ERROR, ", skipping\n"); - bytestream2_skip(&s->gb, 4); /* tag */ - goto skip_tag; + bytestream2_skip(&s->gb, length + 8); /* tag */ } } tag = bytestream2_get_le32(&s->gb); @@ -1242,6 +1231,9 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, av_log(avctx, AV_LOG_DEBUG, "png: tag=%s length=%u\n", av_fourcc2str(tag), length); + bytestream2_init(&gb_chunk, s->gb.buffer, length); + bytestream2_skip(&s->gb, length + 4); + if (avctx->codec_id == AV_CODEC_ID_PNG && avctx->skip_frame == AVDISCARD_ALL) { switch(tag) { @@ -1252,62 +1244,57 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, case MKTAG('t', 'R', 'N', 'S'): break; default: - goto skip_tag; + continue; } } switch (tag) { case MKTAG('I', 'H', 'D', 'R'): - if ((ret = decode_ihdr_chunk(avctx, s, length)) < 0) + if ((ret = decode_ihdr_chunk(avctx, s, &gb_chunk)) < 0) goto fail; break; case MKTAG('p', 'H', 'Y', 's'): - if ((ret = decode_phys_chunk(avctx, s)) < 0) + if ((ret = decode_phys_chunk(avctx, s, &gb_chunk)) < 0) goto fail; break; case MKTAG('f', 'c', 'T', 'L'): if (!CONFIG_APNG_DECODER || avctx->codec_id != AV_CODEC_ID_APNG) - goto skip_tag; - if ((ret = decode_fctl_chunk(avctx, s, length)) < 0) + continue; + if ((ret = decode_fctl_chunk(avctx, s, &gb_chunk)) < 0) goto fail; decode_next_dat = 1; break; case MKTAG('f', 'd', 'A', 'T'): if (!CONFIG_APNG_DECODER || avctx->codec_id != AV_CODEC_ID_APNG) - goto skip_tag; - if (!decode_next_dat || length < 4) { + continue; + if (!decode_next_dat || bytestream2_get_bytes_left(&gb_chunk) < 4) { ret = AVERROR_INVALIDDATA; goto fail; } - bytestream2_get_be32(&s->gb); - length -= 4; + bytestream2_get_be32(&gb_chunk); /* fallthrough */ case MKTAG('I', 'D', 'A', 'T'): if (CONFIG_APNG_DECODER && avctx->codec_id == AV_CODEC_ID_APNG && !decode_next_dat) - goto skip_tag; - if ((ret = decode_idat_chunk(avctx, s, length, p)) < 0) + continue; + if ((ret = decode_idat_chunk(avctx, s, &gb_chunk, p)) < 0) goto fail; break; case MKTAG('P', 'L', 'T', 'E'): - if (decode_plte_chunk(avctx, s, length) < 0) - goto skip_tag; + decode_plte_chunk(avctx, s, &gb_chunk); break; case MKTAG('t', 'R', 'N', 'S'): - if (decode_trns_chunk(avctx, s, length) < 0) - goto skip_tag; + decode_trns_chunk(avctx, s, &gb_chunk); break; case MKTAG('t', 'E', 'X', 't'): - if (decode_text_chunk(s, length, 0) < 0) + if (decode_text_chunk(s, &gb_chunk, 0) < 0) av_log(avctx, AV_LOG_WARNING, "Broken tEXt chunk\n"); - bytestream2_skip(&s->gb, length + 4); break; case MKTAG('z', 'T', 'X', 't'): - if (decode_text_chunk(s, length, 1) < 0) + if (decode_text_chunk(s, &gb_chunk, 1) < 0) av_log(avctx, AV_LOG_WARNING, "Broken zTXt chunk\n"); - bytestream2_skip(&s->gb, length + 4); break; case MKTAG('s', 'T', 'E', 'R'): { - int mode = bytestream2_get_byte(&s->gb); + int mode = bytestream2_get_byte(&gb_chunk); if (mode == 0 || mode == 1) { s->stereo_mode = mode; @@ -1315,33 +1302,31 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, av_log(avctx, AV_LOG_WARNING, "Unknown value in sTER chunk (%d)\n", mode); } - bytestream2_skip(&s->gb, 4); /* crc */ break; } case MKTAG('i', 'C', 'C', 'P'): { - if ((ret = decode_iccp_chunk(s, length, p)) < 0) + if ((ret = decode_iccp_chunk(s, &gb_chunk, p)) < 0) goto fail; break; } case MKTAG('c', 'H', 'R', 'M'): { s->have_chrm = 1; - s->white_point[0] = bytestream2_get_be32(&s->gb); - s->white_point[1] = bytestream2_get_be32(&s->gb); + s->white_point[0] = bytestream2_get_be32(&gb_chunk); + s->white_point[1] = bytestream2_get_be32(&gb_chunk); /* RGB Primaries */ for (i = 0; i < 3; i++) { - s->display_primaries[i][0] = bytestream2_get_be32(&s->gb); - s->display_primaries[i][1] = bytestream2_get_be32(&s->gb); + s->display_primaries[i][0] = bytestream2_get_be32(&gb_chunk); + s->display_primaries[i][1] = bytestream2_get_be32(&gb_chunk); } - bytestream2_skip(&s->gb, 4); /* crc */ break; } case MKTAG('g', 'A', 'M', 'A'): { AVBPrint bp; char *gamma_str; - int num = bytestream2_get_be32(&s->gb); + int num = bytestream2_get_be32(&gb_chunk); av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED); av_bprintf(&bp, "%i/%i", num, 100000); @@ -1351,7 +1336,6 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, av_dict_set(&s->frame_metadata, "gamma", gamma_str, AV_DICT_DONT_STRDUP_VAL); - bytestream2_skip(&s->gb, 4); /* crc */ break; } case MKTAG('I', 'E', 'N', 'D'): @@ -1361,13 +1345,7 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, ret = AVERROR_INVALIDDATA; goto fail; } - bytestream2_skip(&s->gb, 4); /* crc */ goto exit_loop; - default: - /* skip tag */ -skip_tag: - bytestream2_skip(&s->gb, length + 4); - break; } } exit_loop: