[FFmpeg-devel] [libav-devel] [PATCH] wmavoice: limit wmavoice_decode_packet return value to packet size

[Andreas Cadhalpun]

On 28.06.2015 13:13, Ronald S. Bultje wrote:
> On Sun, Jun 28, 2015 at 5:28 AM, Andreas Cadhalpun
>> Treating one like an error, but not the other seems strange as well.
>> One could add an explode mode for both. Would that be better?
> 
> 
> In the first case, it's an error. If the frame size is 2 bits, the header
> is 1, and it specifies a spillover bits of 2, then the frame is clearly
> corrupt. Returning an error is fine. The ffmin() isn't necessary. I also
> don't think an explode mode check is necessary here, it's a clear error
> that is unrecoverable for this frame.

I've no strong preference. Attached is a variant just returning an error.

> In the second case, does that actually happen?

Yes.

> Wmavoice is one of the
> limited number of decoders that internally checks for overreads.
> get_bits_count() should never overread.

It doesn't check everywhere:
    /* Statistics? FIXME - we don't check for length, a slight overrun
     * will be caught by internal buffer padding, and anything else
     * will be skipped, not read. */
    if (get_bits1(gb)) {
        res = get_bits(gb, 4);
        skip_bits(gb, 10 * (res + 1));
    }

> Do you have samples for which this happens?

Yes.

> We currently basically return an error on any possible overread
> signified in the bitstream (without actually overreading), so doing so here
> also would make sense (if it really happens at all).

OK.

> (We could also remove all the overread checks in the decoder, make it use
> the safe bitstream reader mode, and then check for overreads at the end of
> synth_superframe or in the caller, and then return an error. I have no
> specific preference, and this may lead to less code overall.)

That should work as well, but would be a larger change.

Best regards,
Andreas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-wmavoice-limit-wmavoice_decode_packet-return-value-t.patch
Type: text/x-diff
Size: 2070 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150628/51a814d2/attachment.bin>