From patchwork Sat Oct 31 14:16:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 23300 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 A7DB544B236 for ; Sat, 31 Oct 2020 16:17:03 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8F03068AF83; Sat, 31 Oct 2020 16:17:03 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 2EEF7687FA8 for ; Sat, 31 Oct 2020 16:16:57 +0200 (EET) Received: by mail-wr1-f68.google.com with SMTP id n15so9518776wrq.2 for ; Sat, 31 Oct 2020 07:16:57 -0700 (PDT) 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:reply-to :mime-version:content-transfer-encoding; bh=XlcWUcPW2QSTScLU9irhi2u2wgcCOCBZ4zN/OcVtmSA=; b=iKBnAPwWxK+Vin7X5wgFGb7cN1ObfW+exIL0plPXEJ6PmMf1TEGW8N8Uuk6Go+NXh4 vlZ9qKiigmTlfBgIGg1HG7IZt9rlxXI2WUpAs2nf30EcbccEl2Qgxlf1X1tR+x6wLqx7 4S4E4kZfLNx7LqxSHn/G2kp5dgbS6veyEwukGwXlb+Gbya/TZ/KC6huxq2lNnIStnfZm tHrclux+odqF4ENMV/a4YfKGC3afeNNd/A2sx6PfzW8o2YIkhoW5qwjkun9buS/gKKHp 2tElVx+KdNN69IrUGaAE+xPK39eEd4wB9iNB7bJWBZoxMvn7L/jXOjeTh2IwlnOXiYcV PfZA== 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:reply-to:mime-version:content-transfer-encoding; bh=XlcWUcPW2QSTScLU9irhi2u2wgcCOCBZ4zN/OcVtmSA=; b=CrQKmCuv1M5dKkwPkssMZq2uNf1BrFU0n6RdddO+4TzheLKe+Gfl/vGlcxXj1N0ZCF VnTEZMrGVvqS72rNPXejMawZxqHHdciqkpuyf5P4VbRo6Z4zHy9zyQSOnXrLSbGevkG7 i7VQ/z34l9lSrWRSYGtcZPoJxe6hTBDEJZ7717ZbSlAQratsMd3N/xT9z/nyYZdJYxox Qt1bHLMEIru82tQL/jWjbhcF5V6bHy4BpeJZIAqMnGRQRMgLPEO9l9sCGhMcLBO/eKCa PvaudR87i7zrbaCTP1DyWB4wQV1j7TSmkxN3qv7dilM1xjd4Xf+wPUJTjIWOgJczP8rZ AQ2Q== X-Gm-Message-State: AOAM532vEXCFGQUXRa5DiVT8moNRedI4Yu8a8UXdFTVOf3D7SMDTbBNo 7epQ6gArSs8s3vSgN+OBDIhY3ANIr6o= X-Google-Smtp-Source: ABdhPJwQw2jmnJvLEjFxJh+hQmmKZLARYP+EGFu77bDEUOacev+mwTr7BqriOw20X+JgMXeqQEDEHQ== X-Received: by 2002:adf:e643:: with SMTP id b3mr9652303wrn.408.1604153816461; Sat, 31 Oct 2020 07:16:56 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1aa4b.dynamic.kabel-deutschland.de. [188.193.170.75]) by smtp.gmail.com with ESMTPSA id r1sm15522273wro.18.2020.10.31.07.16.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 31 Oct 2020 07:16:55 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sat, 31 Oct 2020 15:16:24 +0100 Message-Id: <20201031141626.727000-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20201031141626.727000-1-andreas.rheinhardt@gmail.com> References: <20201031141626.727000-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v3 2/4] 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. In this case the currently used check would lead to undefined signed integer overflow. 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)". If new_size is > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE the right side becomes negative if the types are signed (as they are now); yet changing this to "if (new_size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE - old_size)" is better as this also works for unsigned types (where it is of course presumed that INT_MAX is replaced by the corresponding maximum for the new type). Both of these issues have been fixed. Signed-off-by: Andreas Rheinhardt --- libavformat/apngdec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavformat/apngdec.c b/libavformat/apngdec.c index 23d7e15393..d8d0de190f 100644 --- a/libavformat/apngdec.c +++ b/libavformat/apngdec.c @@ -127,7 +127,7 @@ static int append_extradata(AVCodecParameters *par, AVIOContext *pb, int len) int new_size, ret; uint8_t *new_extradata; - if (previous_size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE - len) + if (len > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE - previous_size) return AVERROR_INVALIDDATA; new_size = previous_size + len; @@ -208,7 +208,7 @@ static int apng_read_header(AVFormatContext *s) goto fail; len = avio_rb32(pb); - if (len > 0x7fffffff) { + if (len > INT_MAX - 12) { ret = AVERROR_INVALIDDATA; goto fail; }