On 02/29/2012 10:50 PM, Ronald S. Bultje wrote:
Hi,
2012/2/29 Måns Rullgård<m...@mansr.com>:
Vitor Sessak<vitor1...@gmail.com> writes:
On 02/29/2012 04:28 PM, Janne Grunau wrote:
On 2012-02-26 09:52:44 +0100, Vitor Sessak wrote:
---
libavcodec/ra144dec.c | 2 ++
libavcodec/ra288.c | 2 ++
libavcodec/sipr.c | 2 ++
libavcodec/twinvq.c | 2 ++
4 files changed, 8 insertions(+), 0 deletions(-)
Why?
Have you proofed that each of the decoder can't overread?
Of course I did. I concede didn't do it with the AMRNB in my first
patch. I was almost sure I saw the check when I reviewed it, but I was
wrong.
[...]
I would say the decoders are not important enough and speed penalty
for audio doesn't matter enough to disable the safe bitstream reader.
How hard is it to check a single constant value correctly? What is the
use of the safe bitstream reader if the check is done right?
There's much more to it than that. Almost anything using
variable-length codes will need more than a simple packet size check, or
a damaged/malicious bitstream may cause over-reads.
This is the concern that I have also... We really want almost-academic
sort of proof that the decoder can not possibly ever consume more than
X bits of data from /dev/random per single decoding iteration before
unsetting the safe bitstream reader flag.
Sorry, but this is not worth the trouble (its way harder to convince
everyone a codec is safe than actually checking it). Do you mind to have
a look at the r144dec.c code to check (at least for this codec) how
trivial it is?
For ra288.c, it is the same thing, with the caveat that it depends on
the demuxer setting block_align correctly (BTW, wmavoice.c has the same
problem). It's trivial to fix.
For the sipr, there are four decoding "modes". The bit consumption
depend only on the mode (it's trivial to see in the code). Since all the
modes are tested by FATE and didn't brake when the checked bitstream
reader were activated (and the code does check if the frame has the
expected number of bits), it would surprise me that they are wrong.
TwinVQ is in the same situation as SIPR, but with more modes.
-Vitor
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
