[FFmpeg-devel] [PATCH 1/3] avformat/av1dec: Fix padding in obu_get_packet()

[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 <michael at 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.