On 10/16/2020 10:35 AM, Andreas Rheinhardt wrote: > James Almer: >> On 10/16/2020 10:23 AM, Andreas Rheinhardt wrote: >>> James Almer: >>>> On 10/16/2020 7:46 AM, Michael Niedermayer wrote: >>>>> Fixes: stack buffer overflow (read) >>>>> Fixes: >>>>> 26369/clusterfuzz-testcase-minimized-ffmpeg_dem_VIVIDAS_fuzzer-5721057325219840 >>>>> >>>>> Found-by: continuous fuzzing process >>>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>>>> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> >>>>> --- >>>>> libavformat/av1dec.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/libavformat/av1dec.c b/libavformat/av1dec.c >>>>> index 10c4560968..395eef6522 100644 >>>>> --- a/libavformat/av1dec.c >>>>> +++ b/libavformat/av1dec.c >>>>> @@ -382,7 +382,7 @@ static int obu_read_header(AVFormatContext *s) >>>>> static int obu_get_packet(AVFormatContext *s, AVPacket *pkt) >>>>> { >>>>> ObuContext *c = s->priv_data; >>>>> - uint8_t header[MAX_OBU_HEADER_SIZE]; >>>>> + uint8_t header[MAX_OBU_HEADER_SIZE + AV_INPUT_BUFFER_PADDING_SIZE]; >>>>> int64_t obu_size; >>>>> int size = av_fifo_space(c->fifo); >>>>> int ret, len, type; >>>> >>>> Where is header being overread? All reads and writes are always >>>> constrained to MAX_OBU_HEADER_SIZE bytes at most by the fifo. >>> >>> read_obu_with_size() reads it via a GetBitContext which overreads (even >>> when not using the unchecked bitstream reader). >> >> I thought about that too, which would mean this fuzzer forcefully >> disables the checked bitstream reader at configure time? (Why do we even >> have such a configure option anyway? It breaks all kinds of assumptions. >> It should be done internally at the module level exclusively). >> >> Defining UNCHECKED_BITSTREAM_READER to 0 in av1dec.c before including >> get_bits.h would be a better fix. > > You misunderstood: Even the checked bitstream reader overreads
How useful and expected. It's not like the get_bits.h doxy says the checked bitstream reader "ensures that we don't read past input buffer boundaries" or anything like that. Guess the padding works, then. > (otherwise every get_bits() call would need special code to handle the > case in which less than four bytes are available). The only difference > between the checked and the unchecked bitstream reader is that the > former checks when updating the counter: > > #if UNCHECKED_BITSTREAM_READER > # define SKIP_COUNTER(name, gb, num) name ## _index += (num) > #else > # define SKIP_COUNTER(name, gb, num) \ > name ## _index = FFMIN(name ## _size_plus8, name ## _index + (num)) > #endif > > - Andreas > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
