[FFmpeg-devel] [PATCH 1/7] avformat/avc: Fix undefined shift and assert when reading exp-golomb num

[Andreas Rheinhardt]

The current code for parsing unsigned exponential-golomb codes is not
suitable for long codes: If there are enough leading zeroes, it
left-shifts 1 by 32 places and uses get_bitsz to read 32 bits, although
it is only suitable to read 0-25 bits. There is an av_assert2 to
ensure this when using the uncached bitstream reader; with valid input
one can still run into the assert and left-shift 1 by 31 which is
undefined.

This commit changes this. All valid data is parsed correctly; invalid
data does no longer lead to undefined behaviour or to asserts. Parsing
all valid data correctly also necessitated changing the return value to
unsigned; also an intermediate variable used for parsing signed
exponential-golomb codes needed to be made unsigned, too, in order to
parse long signed codes correctly.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
Moving the function to the beginning is done in preparation for another
commit that I'll send soon.

 libavformat/avc.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/libavformat/avc.c b/libavformat/avc.c
index b5e2921388..55494eb08a 100644
--- a/libavformat/avc.c
+++ b/libavformat/avc.c
@@ -27,6 +27,21 @@
 #include "avc.h"
 #include "avio_internal.h"
 
+static inline unsigned get_ue_golomb(GetBitContext *gb)
+{
+    int i;
+    for (i = 1; i <= 32; i++) {
+        if (get_bits_left(gb) < i)
+            return 0;
+        if (show_bits1(gb))
+            break;
+        skip_bits1(gb);
+    }
+    if (i > 32)
+        return 0;
+    return get_bits_long(gb, i) - 1;
+}
+
 static const uint8_t *avc_find_startcode_internal(const uint8_t *p, const uint8_t *end)
 {
     const uint8_t *a = p + 4 - ((intptr_t)p & 3);
@@ -318,15 +333,8 @@ static const AVRational avc_sample_aspect_ratio[17] = {
     {   2,  1 },
 };
 
-static inline int get_ue_golomb(GetBitContext *gb) {
-    int i;
-    for (i = 0; i < 32 && !get_bits1(gb); i++)
-        ;
-    return get_bitsz(gb, i) + (1 << i) - 1;
-}
-
 static inline int get_se_golomb(GetBitContext *gb) {
-    int v = get_ue_golomb(gb) + 1;
+    unsigned v = get_ue_golomb(gb) + 1;
     int sign = -(v & 1);
     return ((v >> 1) ^ sign) - sign;
 }
-- 
2.20.1