From patchwork Thu Aug 20 08:50:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Niedermayer X-Patchwork-Id: 21764 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 7C8AB440B43 for ; Thu, 20 Aug 2020 11:57:27 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 6241B68B60B; Thu, 20 Aug 2020 11:57:27 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from vie01a-dmta-pe03-2.mx.upcmail.net (vie01a-dmta-pe03-2.mx.upcmail.net [62.179.121.161]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id C099968B4FD for ; Thu, 20 Aug 2020 11:57:21 +0300 (EEST) Received: from [172.31.216.235] (helo=vie01a-pemc-psmtp-pe12.mail.upcmail.net) by vie01a-dmta-pe03.mx.upcmail.net with esmtp (Exim 4.92) (envelope-from ) id 1k8gIV-000A4w-0T for ffmpeg-devel@ffmpeg.org; Thu, 20 Aug 2020 10:51:47 +0200 Received: from localhost ([213.47.68.29]) by vie01a-pemc-psmtp-pe12.mail.upcmail.net with ESMTP id 8gHXkvtmuIr7G8gHXknzeI; Thu, 20 Aug 2020 10:50:47 +0200 X-Env-Mailfrom: michael@niedermayer.cc X-Env-Rcptto: ffmpeg-devel@ffmpeg.org X-SourceIP: 213.47.68.29 X-CNFS-Analysis: v=2.3 cv=QN4WuTDL c=1 sm=1 tr=0 a=2hcxjKEKjp0CzLx6oWAm4g==:117 a=2hcxjKEKjp0CzLx6oWAm4g==:17 a=MKtGQD3n3ToA:10 a=1oJP67jkp3AA:10 a=GEAsPZ9sns4A:10 a=ZZnuYtJkoWoA:10 a=nZOtpAppAAAA:20 a=HK7NmiA44IjD81REnJMA:9 a=1fhp2MxaeJtTNGEnv6mo:22 a=Z5ABNNGmrOfJ6cZ5bIyy:22 a=UDnyf2zBuKT2w-IlGP_r:22 From: Michael Niedermayer To: FFmpeg development discussions and patches Date: Thu, 20 Aug 2020 10:50:45 +0200 Message-Id: <20200820085046.5628-3-michael@niedermayer.cc> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200820085046.5628-1-michael@niedermayer.cc> References: <20200820085046.5628-1-michael@niedermayer.cc> X-CMAE-Envelope: MS4wfA3AzLZSh0utreumQiDDTO8Z2jIJ1LINg4eBmVGxcrbDg2SjSxk4lFD2Jm3fyW0EjQU3sFT4972fuRKWGsoYoAXuQjSmRPVheroXFWb2ZmEEgzI0UDEv mUY7BLfcm1uvI1mQJ4Nshzn4oh6XElJwmE2gmEINBOw9ol0BFoK0vlrU Subject: [FFmpeg-devel] [PATCH 3/4] avcodec/tiff: Restrict tag order based on specification 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 MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" "The entries in an IFD must be sorted in ascending order by Tag. Note that this is not the order in which the fields are described in this document." This way various dimensions, sample and bit sizes cannot be changed at arbitrary times which reduces the potential for bugs. The tag reading code also on various places assumes that numerically previous tags have already been parsed, so this needs to be enforced one way or another. If this commit causes problems with real world files which are not easy to fix then some other form of checks are needed to ensure the various dependencies in the tag reading are not violated. Fixes: out of array access Fixes: 24825/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TIFF_fuzzer-6326925027704832 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/tiff.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c index 8a5f6e030d..86f8487086 100644 --- a/libavcodec/tiff.c +++ b/libavcodec/tiff.c @@ -79,6 +79,7 @@ typedef struct TiffContext { int fill_order; uint32_t res[4]; int is_thumbnail; + unsigned last_tag; int is_bayer; uint8_t pattern[4]; @@ -1252,6 +1253,12 @@ static int tiff_decode_tag(TiffContext *s, AVFrame *frame) if (ret < 0) { goto end; } + if (tag <= s->last_tag) + return AVERROR_INVALIDDATA; + + // We ignore TIFF_STRIP_SIZE as it is sometimes in the logic but wrong order around TIFF_STRIP_OFFS + if (tag != TIFF_STRIP_SIZE) + s->last_tag = tag; off = bytestream2_tell(&s->gb); if (count == 1) { @@ -1807,6 +1814,7 @@ again: s->is_tiled = 0; s->is_jpeg = 0; s->cur_page = 0; + s->last_tag = 0; for (i = 0; i < 65536; i++) s->dng_lut[i] = i;