[FFmpeg-devel] [PATCH 4/4] avcodec/dca: improve band data parsing robustness

[foo86]

Limit the maximum length of unary part of Rice code by the number of
available bits instead of using an arbitrary constant that happens to be
just large enough to work.

This effectively limits amount of data that can be overread per segment
by maximum length of binary code per sample multiplied by maximum
segment size.

Increase size of padding area after the end of input buffer according to
this limit and add some extra overread checks to make reading past end
of buffer safe even when internal overread checks are disabled in
bitstream reader code.
---
 libavcodec/dca_xll.c | 25 +++++++++++++++----------
 libavcodec/dcadec.h  |  2 +-
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/libavcodec/dca_xll.c b/libavcodec/dca_xll.c
index 5e6cf35..46bef3d 100644
--- a/libavcodec/dca_xll.c
+++ b/libavcodec/dca_xll.c
@@ -32,7 +32,7 @@ static int get_linear(GetBitContext *gb, int n)
 
 static int get_rice_un(GetBitContext *gb, int k)
 {
-    unsigned int v = get_unary(gb, 1, 128);
+    unsigned int v = get_unary(gb, 1, get_bits_left(gb));
     return (v << k) | get_bits_long(gb, k);
 }
 
@@ -512,7 +512,7 @@ static int chs_parse_band_data(DCAXllDecoder *s, DCAXllChSet *c, int band, int s
         nsamples_part_b = s->nsegsamples - c->nsamples_part_a[k];
 
         if (get_bits_left(&s->gb) < 0)
-            return AVERROR_INVALIDDATA;
+            goto overread;
 
         if (!c->rice_code_flag[k]) {
             // Linear codes
@@ -534,6 +534,9 @@ static int chs_parse_band_data(DCAXllDecoder *s, DCAXllChSet *c, int band, int s
                 // Unpack the number of isolated samples
                 int nisosamples = get_bits(&s->gb, s->nsegsamples_log2);
 
+                if (get_bits_left(&s->gb) < nisosamples * s->nsegsamples_log2)
+                    goto overread;
+
                 // Set all locations to 0
                 memset(part_b, 0, sizeof(*part_b) * nsamples_part_b);
 
@@ -573,14 +576,14 @@ static int chs_parse_band_data(DCAXllDecoder *s, DCAXllChSet *c, int band, int s
     // Start unpacking LSB portion of the segment
     if (b->lsb_section_size) {
         // Skip to the start of LSB portion
-        if (ff_dca_seek_bits(&s->gb, band_data_end - b->lsb_section_size * 8)) {
-            av_log(s->avctx, AV_LOG_ERROR, "Read past end of XLL band data\n");
-            return AVERROR_INVALIDDATA;
-        }
+        if (ff_dca_seek_bits(&s->gb, band_data_end - b->lsb_section_size * 8))
+            goto overread;
 
         // Unpack all LSB parts of residuals of this segment
         for (i = 0; i < c->nchannels; i++) {
             if (b->nscalablelsbs[i]) {
+                if (get_bits_left(&s->gb) < 0)
+                    goto overread;
                 get_array(&s->gb,
                           b->lsb_sample_buffer[i] + seg * s->nsegsamples,
                           s->nsegsamples, b->nscalablelsbs[i]);
@@ -589,12 +592,14 @@ static int chs_parse_band_data(DCAXllDecoder *s, DCAXllChSet *c, int band, int s
     }
 
     // Skip to the end of band data
-    if (ff_dca_seek_bits(&s->gb, band_data_end)) {
-        av_log(s->avctx, AV_LOG_ERROR, "Read past end of XLL band data\n");
-        return AVERROR_INVALIDDATA;
-    }
+    if (ff_dca_seek_bits(&s->gb, band_data_end))
+        goto overread;
 
     return 0;
+
+overread:
+    av_log(s->avctx, AV_LOG_ERROR, "Read past end of XLL band data\n");
+    return AVERROR_INVALIDDATA;
 }
 
 static av_cold void chs_clear_band_data(DCAXllDecoder *s, DCAXllChSet *c, int band, int seg)
diff --git a/libavcodec/dcadec.h b/libavcodec/dcadec.h
index 6726121..2805a0e 100644
--- a/libavcodec/dcadec.h
+++ b/libavcodec/dcadec.h
@@ -32,7 +32,7 @@
 #include "dca_exss.h"
 #include "dca_xll.h"
 
-#define DCA_BUFFER_PADDING_SIZE     1024
+#define DCA_BUFFER_PADDING_SIZE     2048
 
 #define DCA_PACKET_CORE         0x01
 #define DCA_PACKET_EXSS         0x02
-- 
2.1.4