From 8fbc110d89bf9213db752fae460717a40726cdcc Mon Sep 17 00:00:00 2001 From: Michael Niedermayer Date: Thu, 20 Aug 2020 01:05:35 +0200 Subject: [PATCH] avcodec/tiff: Restrict tag order based on specification "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 (cherry picked from commit ad29f9e47cb848e11ee1d358d2bae15cd35ef04b) 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 260152c207..50fcd73bba 100644 --- a/libavcodec/tiff.c +++ b/libavcodec/tiff.c @@ -64,6 +64,7 @@ typedef struct TiffContext { int predictor; int fill_order; uint32_t res[4]; + unsigned last_tag; int strips, rps, sstype; int sot; @@ -789,6 +790,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) { @@ -1221,6 +1228,7 @@ static int decode_frame(AVCodecContext *avctx, s->photometric = TIFF_PHOTOMETRIC_NONE; s->compr = TIFF_RAW; s->fill_order = 0; + s->last_tag = 0; free_geotags(s); // Reset these offsets so we can tell if they were set this frame