diff mbox series

[FFmpeg-devel,v3,2/5] avcodec/golomb: Prevent shift by negative number

Message ID 20200727090810.23794-2-andreas.rheinhardt@gmail.com
State Accepted
Commit 69636b443c4f40286135ad9658b5d44a9de4f3a4
Headers show
Series [FFmpeg-devel,v3,1/5] avcodec/golomb: Don't emit error message in get_ue_golomb
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt July 27, 2020, 9:08 a.m. UTC
This happened in get_ue_golomb() if the cached bitstream reader was in
use, because there was no check to handle the case of the read value
not being in the supported range.
For consistency with the uncached bitstream reader and for compliance
with the documentation, every value not in the 0-8190 range is treated as
error although the cached bitstream reader could actually read values in
the range 0..65534 without problems.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
v3: Besides accepting the same range as the uncached version this
version also consumes bits in case of error like the uncached version.
Unfortunately it is not possible to always consume the same number of
bits, because the uncached version will consume 63 bits as soon as all
valid bits are zero, whereas the cached version will only consume 63
bits if the next 31 bits are zero.

 libavcodec/golomb.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer July 28, 2020, 7:47 p.m. UTC | #1
On Mon, Jul 27, 2020 at 11:08:07AM +0200, Andreas Rheinhardt wrote:
> This happened in get_ue_golomb() if the cached bitstream reader was in
> use, because there was no check to handle the case of the read value
> not being in the supported range.
> For consistency with the uncached bitstream reader and for compliance
> with the documentation, every value not in the 0-8190 range is treated as
> error although the cached bitstream reader could actually read values in
> the range 0..65534 without problems.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> v3: Besides accepting the same range as the uncached version this
> version also consumes bits in case of error like the uncached version.
> Unfortunately it is not possible to always consume the same number of
> bits, because the uncached version will consume 63 bits as soon as all
> valid bits are zero, whereas the cached version will only consume 63
> bits if the next 31 bits are zero.
> 
>  libavcodec/golomb.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

probably ok

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
index 1f988d74aa..aed9b22471 100644
--- a/libavcodec/golomb.h
+++ b/libavcodec/golomb.h
@@ -66,9 +66,12 @@  static inline int get_ue_golomb(GetBitContext *gb)
         return ff_ue_golomb_vlc_code[buf];
     } else {
         int log = 2 * av_log2(buf) - 31;
+
+        skip_bits_long(gb, 32 - log);
+        if (log < 7)
+            return AVERROR_INVALIDDATA;
         buf >>= log;
         buf--;
-        skip_bits_long(gb, 32 - log);
 
         return buf;
     }