From patchwork Sun Jan 12 21:56:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 17310 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 0F20444B190 for ; Mon, 13 Jan 2020 00:03:18 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DEDF568B0AA; Mon, 13 Jan 2020 00:03:17 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6EC6A68B0A3 for ; Mon, 13 Jan 2020 00:03:12 +0200 (EET) Received: by mail-wm1-f68.google.com with SMTP id t14so7586988wmi.5 for ; Sun, 12 Jan 2020 14:03:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=sPdE3FQBVIrv/2XecmbVEDOq8s/NXV/u0JMJDUGVVQI=; b=tNCaoicat46w3cET8PgMNnLNdc6H0rHZhJbtujux19PQUal37bqZtVLjwvRQqj7eFS wk1S/3SxWu9VzqliZTsgnPnOwDfmDCER7r5MmEYh3oLdkuS+FQo56twzWm/+rd1qNrFM K+oLwWK8qRnGi7OKWzv/1B0KrGzUiJht2TlzyclhW6lSF4gyGjQpJMZQoO6Il+hHRHos XsF+PYey/C6TmEKpyDqINVJE4jikJsSPFvK4VyE8h3uqLa1lz8KpuYwCisVu1lWdTjYs k1gkZlra5VtNJra6jRSH7x6XevG3lDpMWo5x6EGPxbDt8qfaj0BNYDQfbV/b6Roy9zoU oOYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=sPdE3FQBVIrv/2XecmbVEDOq8s/NXV/u0JMJDUGVVQI=; b=ZCsNiUoyhPOCT2p2mqgZ6IUHGiOI2UW6a21e5aBw5cGdcetlHuQGCeCsCr+4d6q6Lw KibryBMFnD0Fm5H8FRZNRn11DXMaXywtKGo1nKs0VcmuPZn670eBX+jG1I5EQ3DKrzAb 597q6XTb1RTNKwbm9PSKdalJ7RYRO9JHyNCfGy5a4VxDOoMAwnN1j6pXGIyIfbrwHUv+ 8CiExbLdq/MUrVpmJLbguqGUKEmIMUysHlOGLzJ92ov9/X0tum0jICSCjxrA3eQ+dBjG ScSDDBH76LNhFW5omjmHXT9KJU3EJvWM2PIikmL5KZ5wk7SXKqZjKRaEd3vBvoemqWQK e4SQ== X-Gm-Message-State: APjAAAXdUBjH707ahHChB1VtoM56aYrpF28XRj+Mt5mwO/D3dAucMiGo r2+33yNP7Ie5DYcBNW4l+y0Rj7Gm X-Google-Smtp-Source: APXvYqzs7x1ZbjBoc0uypiZn91PKX3zYa0dpM/IAUm8h401bvdCgJSE/TutcgwUvvdNrfua4n66Z3w== X-Received: by 2002:a7b:cb0a:: with SMTP id u10mr16010656wmj.165.1578866591715; Sun, 12 Jan 2020 14:03:11 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc08bbf.dynamic.kabel-deutschland.de. [188.192.139.191]) by smtp.gmail.com with ESMTPSA id c4sm11763741wml.7.2020.01.12.14.03.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 12 Jan 2020 14:03:11 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sun, 12 Jan 2020 22:56:50 +0100 Message-Id: <20200112215651.1258-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200112215651.1258-1-andreas.rheinhardt@gmail.com> References: <20200112215651.1258-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 2/3] avformat/apngdec: Fix size/overflow checks 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 Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" apng data consists of parts containing a small header (including a four-byte size field) and a data part; the size field does not account for everything and is actually twelve bytes short of the actual size. In order to make sure that the size fits into an int, the size field is checked for being > INT_MAX; yet this does not account for the + 12 and upon conversion to int (which happens when calling append_extradata()), the size parameter can still wrap around. Furthermore, append_extradata() appends the new data to the already existing extradata and therefore needs to make sure that the combined size of new and old data as well as padding fits into an int. The check used for this is "if (old_size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE - new_size)" and this does not work if new_size is > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE. Finally, the size field of a fcTL part has to be 26. This was enforced when reading packets, yet not when reading the header. All of these issues have been fixed. Signed-off-by: Andreas Rheinhardt --- libavformat/apngdec.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/libavformat/apngdec.c b/libavformat/apngdec.c index 36657877d2..e9c317efbf 100644 --- a/libavformat/apngdec.c +++ b/libavformat/apngdec.c @@ -121,13 +121,13 @@ end: return AVPROBE_SCORE_MAX; } -static int append_extradata(AVCodecParameters *par, AVIOContext *pb, int len) +static int append_extradata(AVCodecParameters *par, AVIOContext *pb, uint64_t len) { int previous_size = par->extradata_size; int new_size, ret; uint8_t *new_extradata; - if (previous_size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE - len) + if (previous_size + len > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE) return AVERROR_INVALIDDATA; new_size = previous_size + len; @@ -208,16 +208,11 @@ static int apng_read_header(AVFormatContext *s) goto fail; len = avio_rb32(pb); - if (len > 0x7fffffff) { - ret = AVERROR_INVALIDDATA; - goto fail; - } - tag = avio_rl32(pb); switch (tag) { case MKTAG('a', 'c', 'T', 'L'): if ((ret = avio_seek(pb, -8, SEEK_CUR)) < 0 || - (ret = append_extradata(st->codecpar, pb, len + 12)) < 0) + (ret = append_extradata(st->codecpar, pb, len + 12ULL)) < 0) goto fail; acTL_found = 1; ctx->num_frames = AV_RB32(st->codecpar->extradata + ret + 8); @@ -226,7 +221,7 @@ static int apng_read_header(AVFormatContext *s) ctx->num_frames, ctx->num_play); break; case MKTAG('f', 'c', 'T', 'L'): - if (!acTL_found) { + if (!acTL_found || len != 26) { ret = AVERROR_INVALIDDATA; goto fail; } @@ -235,7 +230,7 @@ static int apng_read_header(AVFormatContext *s) return 0; default: if ((ret = avio_seek(pb, -8, SEEK_CUR)) < 0 || - (ret = append_extradata(st->codecpar, pb, len + 12)) < 0) + (ret = append_extradata(st->codecpar, pb, len + 12ULL)) < 0) goto fail; } }