Vitor Sessak <[email protected]> writes: > On 02/29/2012 10:47 PM, Måns Rullgård wrote: >> Vitor Sessak<[email protected]> 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. > > Those codecs are supposed to consume a fixed number of bits to output > a fixed number of samples and all the code use this assumption.
Key word being "supposed". If there is any chance that a malformed input can cause an over-read, the checked bitstream reader should be used. > If a malicious file is over-reading the input, the over-read itself is > the least of your problems: there is a good chance the decoder will > also write beyond the end of buffers in an exploitable way - and this > is not prevented by the checked reader. That's an unrelated problem, irrelevant to this discussion. -- Måns Rullgård [email protected] _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
