CVE-2019-1000016: Improper Validation of Array Index (CWE-129)
value: FFMPEG version 4.1 contains a CWE-129: Improper Validation of Array Index vulnerability in libavcodec/cbs_av1.c that can result in Denial of service. This attack appears to be exploitable via specially crafted AV1 file has to be provided as input. This vulnerability appears to have been fixed in after commit b97a4b658814b2de8b9f2a3bce491c002d34de31.
CWE description: The product uses untrusted input when calculating or using an array index, but the product does not validate or incorrectly validates the index to ensure the index references a valid position within the array.
CVSS v3 base score: 6.5
Published: 2019-02-04T21:29Z
Repository: https://github.com/FFmpeg/FFmpeg
Language: C
Files changed: libavcodec/cbs_av1.c
Commit date: 2018-12-22 18:17:03 +0000
Commit message: cbs_av1: Fix reading of overlong uvlc codes

The specification allows 2^32-1 to be encoded as any number of zeroes
greater than 31, followed by a one.  This previously failed because the
trace code would overflow the array containing the string representation
of the bits if there were more than 63 zeroes.  Fix that by splitting the
trace output into batches, and at the same time move it out of the default
path.

(While this seems likely to be a specification error, libaom does support
it so we probably should as well.)

From a test case by keval shah <skeval65@gmail.com>.

Reviewed-by: Michael Niedermayer <michael@niedermayer.cc>

Diff: diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
index b3f81ae93a53..f70ff1c94fcd 100644
--- a/libavcodec/cbs_av1.c
+++ b/libavcodec/cbs_av1.c
@@ -27,49 +27,71 @@
 
 static int cbs_av1_read_uvlc(CodedBitstreamContext *ctx, GetBitContext *gbc,
                              const char *name, uint32_t *write_to,
                              uint32_t range_min, uint32_t range_max)
 {
-    uint32_t value;
-    int position, zeroes, i, j;
-    char bits[65];
+    uint32_t zeroes, bits_value, value;
+    int position;
 
     if (ctx->trace_enable)
         position = get_bits_count(gbc);
 
-    zeroes = i = 0;
+    zeroes = 0;
     while (1) {
-        if (get_bits_left(gbc) < zeroes + 1) {
+        if (get_bits_left(gbc) < 1) {
             av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid uvlc code at "
                    "%s: bitstream ended.\n", name);
             return AVERROR_INVALIDDATA;
         }
 
-        if (get_bits1(gbc)) {
-            bits[i++] = '1';
+        if (get_bits1(gbc))
             break;
-        } else {
-            bits[i++] = '0';
-            ++zeroes;
-        }
+        ++zeroes;
     }
 
     if (zeroes >= 32) {
         value = MAX_UINT_BITS(32);
     } else {
-        value = get_bits_long(gbc, zeroes);
-
-        for (j = 0; j < zeroes; j++)
-            bits[i++] = (value >> (zeroes - j - 1) & 1) ? '1' : '0';
+        if (get_bits_left(gbc) < zeroes) {
+            av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid uvlc code at "
+                   "%s: bitstream ended.\n", name);
+            return AVERROR_INVALIDDATA;
+        }
 
-        value += (1 << zeroes) - 1;
+        bits_value = get_bits_long(gbc, zeroes);
+        value = bits_value + (UINT32_C(1) << zeroes) - 1;
     }
 
     if (ctx->trace_enable) {
+        char bits[65];
+        int i, j, k;
+
+        if (zeroes >= 32) {
+            while (zeroes > 32) {
+                k = FFMIN(zeroes - 32, 32);
+                for (i = 0; i < k; i++)
+                    bits[i] = '0';
+                bits[i] = 0;
+                ff_cbs_trace_syntax_element(ctx, position, name,
+                                            NULL, bits, 0);
+                zeroes -= k;
+                position += k;
+            }
+        }
+
+        for (i = 0; i < zeroes; i++)
+            bits[i] = '0';
+        bits[i++] = '1';
+
+        if (zeroes < 32) {
+            for (j = 0; j < zeroes; j++)
+                bits[i++] = (bits_value >> (zeroes - j - 1) & 1) ? '1' : '0';
+        }
+
         bits[i] = 0;
-        ff_cbs_trace_syntax_element(ctx, position, name, NULL,
-                                    bits, value);
+        ff_cbs_trace_syntax_element(ctx, position, name,
+                                    NULL, bits, value);
     }
 
     if (value < range_min || value > range_max) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
                "%"PRIu32", but must be in [%"PRIu32",%"PRIu32"].\n",

Vulnerable code:     uint32_t value;
    int position, zeroes, i, j;
    char bits[65];
    zeroes = i = 0;
        if (get_bits_left(gbc) < zeroes + 1) {
        if (get_bits1(gbc)) {
            bits[i++] = '1';
        } else {
            bits[i++] = '0';
            ++zeroes;
        }
        value = get_bits_long(gbc, zeroes);

        for (j = 0; j < zeroes; j++)
            bits[i++] = (value >> (zeroes - j - 1) & 1) ? '1' : '0';
        value += (1 << zeroes) - 1;
        ff_cbs_trace_syntax_element(ctx, position, name, NULL,
                                    bits, value);
Fixed code:     uint32_t zeroes, bits_value, value;
    int position;
    zeroes = 0;
        if (get_bits_left(gbc) < 1) {
        if (get_bits1(gbc))
        ++zeroes;
        if (get_bits_left(gbc) < zeroes) {
            av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid uvlc code at "
                   "%s: bitstream ended.\n", name);
            return AVERROR_INVALIDDATA;
        }
        bits_value = get_bits_long(gbc, zeroes);
        value = bits_value + (UINT32_C(1) << zeroes) - 1;
        char bits[65];
        int i, j, k;

        if (zeroes >= 32) {
            while (zeroes > 32) {
                k = FFMIN(zeroes - 32, 32);
                for (i = 0; i < k; i++)
                    bits[i] = '0';
                bits[i] = 0;
                ff_cbs_trace_syntax_element(ctx, position, name,
                                            NULL, bits, 0);
                zeroes -= k;
                position += k;
            }
        }

        for (i = 0; i < zeroes; i++)
            bits[i] = '0';
        bits[i++] = '1';

        if (zeroes < 32) {
            for (j = 0; j < zeroes; j++)
                bits[i++] = (bits_value >> (zeroes - j - 1) & 1) ? '1' : '0';
        }

        ff_cbs_trace_syntax_element(ctx, position, name,
                                    NULL, bits, value);