[FFmpeg-devel] [PATCH 1/2] avcodec/vp9: Check in decode_tiles() if there is data remaining

[Ronald S. Bultje]

Hi Michael,

On Thu, Aug 9, 2018 at 8:49 PM, Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Wed, Aug 08, 2018 at 10:00:42PM -0300, James Almer wrote:
>
> Apply this patch with changes to allow that specific condition and lets
> > not waste more time on this.
>
> this is the only change the patch does. Without it there is no patch.
>
> Either we stop when the input ends -> that might break decoding a file
> that was designed so as to expect a decoder not to stop.
> or we do not stop then that allows doing denial of service
>

OK, ok, hold on. I'll try to explain my problem with the patch and I will
suggest a possible solution. Please store your objections in the closet for
a second. I'm not a terrible person.

The situation you're fixing and not breaking:
let's say there is a file that is 1 byte long (8 bits), but we claim it's a
16k x 16k file. This will take ages to decode, even though it's likely
broken. Right? A one-byte file is unlikely anyway, but sure, it will run
out of data after a few symbols. I get it. I really do. And I agree that
this must be fixed. Yes.

Also, if a valid file of 1 byte (8 bits) has only 1 symbol of approximately
4 real bits, then at the end, there's still 4 bits left in the arithcoder.
So nothing breaks. Great!

My objection:
if a file has exactly symbols of 8 bits in arithdata, then after all this,
the arithcoder will signal empty and EOF, even though no error occured.
Imagine a bitcoder (non-arith) of this situation. After get_bits(gb, 8),
the data pointer will have reached the end, and the bits_left is 0, but
that does not indicate an error, quite the contrary. It just means that the
byte boundary happened to align with the exact end of the file. This can
happen.

My suggestion:
add an eof flag to the arithcoder. When we have reached the above condition
where new data is needed but not present, simply set the EOF flag, and
check that for errors. If it's set, you can error out.

Ronald