[FFmpeg-devel] bad arguments to init_put_bits

[Dyami Caliri]

Do you want sample code that generates the crash?

For "the code shouldn't really crash even without the patch," are you
saying that the code, as it's written, doesn't look it would crash?
Or are you saying that we need to change the code to make sure it doesn't
crash, even without the patch?

Here is the short answer for how it happens: bit-shifting (or multiplying)
the buffer_size argument to init_put_bits() can cause it to be interpreted
as a negative number, which sets the buf_ptr to NULL, which leads to
dereferencing null in later calls to put_bits().

Here is a longer explanation:
prores_encode_frame() calculates a frame_size variable like this:
int frame_size = FFALIGN(avctx->width, 16) * FFALIGN(avctx->height, 16)*16
+ 500 + FF_MIN_BUFFER_SIZE;

In encode_slice_plane, "unsigned buf_size" is passed in. This is related to
the frame_size value from earlier.

Then init_put_bits() is called with (buf_size << 3).
The argument for init_put_bits is a regular, signed int.
So, for certain width+height, we end up with a buf_size that, when shifted
left 3 times and converted to a signed int, becomes a negative value.

In init_put_bits(), if buffer_size < 0, the buffer_size is set to zero and
the buffer is set to 0/NULL.

Thereafter, any calls to put_bits will crash, because it accesses
'buf_ptr', which is NULL.

On Thu, Feb 26, 2015 at 11:45 AM, Michael Niedermayer <michaelni at gmx.at>
wrote:

> Hi
>
>
> On Thu, Feb 26, 2015 at 10:42:06AM -0800, Dyami Caliri wrote:
> > The init_put_bits() function (in libavcodec/put_bits.h) takes a buffer
> and
> > a buffer size (in bytes). Several of the encoders are passing the buffer
> > size in bits, by multiplying the buffer size by 8. This is incorrect.
> >
> > We saw this problem when encoding a ProRes (Anatoliy) file at size
> > 4096x4096. Debugging showed that the buffer size was very large, and when
> > multiplied by 8, it was interpreted as a negative number. This caused
> > the init_put_bits()
> > to zero out the buffer, leading to a crash.
> >
> > The attached patch fixes the argument to init_put_bits in all of the
> cases
> > where the buffer size was multiplied by 8.
> >
> > You could use the patch or make the same change.
>
> patch applied
>
> but how can the crash be reproduced? the code shouldnt really crash
> even without the patch
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The real ebay dictionary, page 3
> "Rare item" - "Common item with rare defect or maybe just a lie"
> "Professional" - "'Toy' made in china, not functional except as doorstop"
> "Experts will know" - "The seller hopes you are not an expert"
>