From 6751e6f594b0e0cba6fb0fbfdb7b0ab2c30c8512 Mon Sep 17 00:00:00 2001 From: John Rummell Date: Mon, 23 Mar 2020 15:48:33 -0700 Subject: [PATCH] Check return value from avio_read() to verify data actually read If the buffer doesn't contain enough bytes when reading a stream, fail rather than continuing on with unitialized data. One attempt caught by Chromium fuzzers (crbug.com/1054229), rest done by looking for calls to avio_read() that don't check the result in Chromium code search. --- libavformat/mov.c | 34 ++++++++++++++++++++++++++-------- libavformat/oggdec.c | 3 ++- libavformat/wavdec.c | 12 ++++++++---- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index f280f360b6..b66a72484c 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -1012,10 +1012,24 @@ static int mov_read_adrm(MOVContext *c, AVIOContext *pb, MOVAtom atom) } /* drm blob processing */ - avio_read(pb, output, 8); // go to offset 8, absolute position 0x251 - avio_read(pb, input, DRM_BLOB_SIZE); - avio_read(pb, output, 4); // go to offset 4, absolute position 0x28d - avio_read(pb, file_checksum, 20); + /* go to offset 8, absolute position 0x251 */ + if (avio_read(pb, output, 8) != 8) { + ret = AVERROR_INVALIDDATA; + goto fail; + } + if (avio_read(pb, input, DRM_BLOB_SIZE) != DRM_BLOB_SIZE) { + ret = AVERROR_INVALIDDATA; + goto fail; + } + /* go to offset 4, absolute position 0x28d */ + if (avio_read(pb, output, 4) != 4) { + ret = AVERROR_INVALIDDATA; + goto fail; + } + if (avio_read(pb, file_checksum, 20) != 20) { + ret = AVERROR_INVALIDDATA; + goto fail; + } av_log(c->fc, AV_LOG_INFO, "[aax] file checksum == "); // required by external tools for (i = 0; i < 20; i++) @@ -1876,7 +1890,8 @@ static int mov_read_wave(MOVContext *c, AVIOContext *pb, MOVAtom atom) AV_WB32(st->codecpar->extradata , ALAC_EXTRADATA_SIZE); AV_WB32(st->codecpar->extradata + 4, MKTAG('a','l','a','c')); AV_WB64(st->codecpar->extradata + 12, buffer); - avio_read(pb, st->codecpar->extradata + 20, 16); + if (avio_read(pb, st->codecpar->extradata + 20, 16) != 16) + return AVERROR_INVALIDDATA; avio_skip(pb, atom.size - 24); return 0; } @@ -4376,7 +4391,8 @@ static int mov_read_keys(MOVContext *c, AVIOContext *pb, MOVAtom atom) c->meta_keys[i] = av_mallocz(key_size + 1); if (!c->meta_keys[i]) return AVERROR(ENOMEM); - avio_read(pb, c->meta_keys[i], key_size); + if (avio_read(pb, c->meta_keys[i], key_size) != key_size) + return AVERROR_INVALIDDATA; } return 0; @@ -6547,7 +6563,8 @@ static int mov_read_dfla(MOVContext *c, AVIOContext *pb, MOVAtom atom) avio_rb24(pb); /* Flags */ - avio_read(pb, buf, sizeof(buf)); + if (avio_read(pb, buf, sizeof(buf)) != sizeof(buf)) + return AVERROR_INVALIDDATA; flac_parse_block_header(buf, &last, &type, &size); if (type != FLAC_METADATA_TYPE_STREAMINFO || size != FLAC_STREAMINFO_SIZE) { @@ -6715,7 +6732,8 @@ static int mov_read_dops(MOVContext *c, AVIOContext *pb, MOVAtom atom) AV_WL32(st->codecpar->extradata, MKTAG('O','p','u','s')); AV_WL32(st->codecpar->extradata + 4, MKTAG('H','e','a','d')); AV_WB8(st->codecpar->extradata + 8, 1); /* OpusHead version */ - avio_read(pb, st->codecpar->extradata + 9, size - 9); + if (avio_read(pb, st->codecpar->extradata + 9, size - 9) != size - 9) + return AVERROR_INVALIDDATA; /* OpusSpecificBox is stored in big-endian, but OpusHead is little-endian; aside from the preceeding magic and version they're diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c index 4f4b5fe386..de2f24b967 100644 --- a/libavformat/oggdec.c +++ b/libavformat/oggdec.c @@ -216,7 +216,8 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, int nsegs) uint8_t magic[8]; int64_t pos = avio_tell(s->pb); avio_skip(s->pb, nsegs); - avio_read(s->pb, magic, sizeof(magic)); + if (avio_read(s->pb, magic, sizeof(magic)) != sizeof(magic)) + return AVERROR_INVALIDDATA; avio_seek(s->pb, pos, SEEK_SET); codec = ogg_find_codec(magic, sizeof(magic)); if (!codec) { diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index 575c667452..bdb7431d66 100644 --- a/libavformat/wavdec.c +++ b/libavformat/wavdec.c @@ -611,7 +611,8 @@ static int64_t find_guid(AVIOContext *pb, const uint8_t guid1[16]) int64_t size; while (!avio_feof(pb)) { - avio_read(pb, guid, 16); + if (avio_read(pb, guid, 16) != 16) + return AVERROR_INVALIDDATA; size = avio_rl64(pb); if (size <= 24) return AVERROR_INVALIDDATA; @@ -797,7 +798,8 @@ static int w64_read_header(AVFormatContext *s) uint8_t guid[16]; int ret; - avio_read(pb, guid, 16); + if (avio_read(pb, guid, 16) != 16) + return AVERROR_INVALIDDATA; if (memcmp(guid, ff_w64_guid_riff, 16)) return AVERROR_INVALIDDATA; @@ -805,7 +807,8 @@ static int w64_read_header(AVFormatContext *s) if (avio_rl64(pb) < 16 + 8 + 16 + 8 + 16 + 8) return AVERROR_INVALIDDATA; - avio_read(pb, guid, 16); + if (avio_read(pb, guid, 16) != 16) + return AVERROR_INVALIDDATA; if (memcmp(guid, ff_w64_guid_wave, 16)) { av_log(s, AV_LOG_ERROR, "could not find wave guid\n"); return AVERROR_INVALIDDATA; @@ -862,7 +865,8 @@ static int w64_read_header(AVFormatContext *s) break; chunk_key[4] = 0; - avio_read(pb, chunk_key, 4); + if (avio_read(pb, chunk_key, 4) != 4) + return AVERROR_INVALIDDATA; chunk_size = avio_rl32(pb); if (chunk_size == UINT32_MAX) return AVERROR_INVALIDDATA; -- 2.26.0.rc2.310.g2932bb562d-goog