[FFmpeg-devel] [PATCH] dca: fix reading over the end of the allocated buffer (v2)

[Anssi Hannula]

I noticed that the existing core DTS decoder doesn't seem to do
any sanity checks and can easily read over the end of the buffer (a
quick calculation suggests a rogue stream may cause the decoder to
read up to around 120 kilobytes from the 16 kilobyte buffer).

Since the dca_buffer resides inside DCAContext, there are several
kilobytes of extra allocated memory following the buffer, but that is
not enough.

Fix that by adding several checks to the decoder, making sure it never
reads more than 1 kilobyte over the end of the buffer.
---

Kostya wrote:
> Ahem, add those several kbs to dca_buffer size instead, I would
> not rely on having something else in context as padding.

OK, I had assumed the implicit padding was originally intended. Anyway,
here's a new one adding the 1 kB to the dca_buffer instead.

Note that my selection of 1024 bytes as padding is somewhat arbitrary,
and it could be more bigger or smaller if wanted, thus decreasing or
increasing the amount of sanity checks needed.


 libavcodec/dca.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/libavcodec/dca.c b/libavcodec/dca.c
index 83f529b..6f0f9c1 100644
--- a/libavcodec/dca.c
+++ b/libavcodec/dca.c
@@ -184,6 +184,8 @@ static const int8_t dca_channel_reorder_nolfe_xch[][9] = {
 
 #define DCA_MAX_FRAME_SIZE 16384
 
+#define DCA_BUFFER_PADDING_SIZE 1024
+
 /** Bit allocation */
 typedef struct {
     int offset;                 ///< code values offset
@@ -280,7 +282,7 @@ typedef struct {
     DECLARE_ALIGNED(16, float, samples)[(DCA_PRIM_CHANNELS_MAX+1)*256];
     const float *samples_chanptr[DCA_PRIM_CHANNELS_MAX+1];
 
-    uint8_t dca_buffer[DCA_MAX_FRAME_SIZE];
+    uint8_t dca_buffer[DCA_MAX_FRAME_SIZE + DCA_BUFFER_PADDING_SIZE];
     int dca_buffer_size;        ///< how much data is in the dca_buffer
 
     const int8_t* channel_order_tab;                             ///< channel reordering table, lfe and non lfe
@@ -558,6 +560,9 @@ static int dca_subframe_header(DCAContext * s, int base_channel, int block_index
     /* Primary audio coding side information */
     int j, k;
 
+    if (get_bits_left(&s->gb) < 0)
+        return -1;
+
     if (!base_channel) {
         s->subsubframes[s->current_subframe] = get_bits(&s->gb, 2) + 1;
         s->partial_samples[s->current_subframe] = get_bits(&s->gb, 3);
@@ -614,6 +619,9 @@ static int dca_subframe_header(DCAContext * s, int base_channel, int block_index
         }
     }
 
+    if (get_bits_left(&s->gb) < 0)
+        return -1;
+
     for (j = base_channel; j < s->prim_channels; j++) {
         const uint32_t *scale_table;
         int scale_sum;
@@ -649,6 +657,9 @@ static int dca_subframe_header(DCAContext * s, int base_channel, int block_index
             s->joint_huff[j] = get_bits(&s->gb, 3);
     }
 
+    if (get_bits_left(&s->gb) < 0)
+        return -1;
+
     /* Scale factors for joint subband coding */
     for (j = base_channel; j < s->prim_channels; j++) {
         int source_channel;
@@ -1006,6 +1017,9 @@ static int dca_subsubframe(DCAContext * s, int base_channel, int block_index)
         quant_step_table = lossy_quant_d;
 
     for (k = base_channel; k < s->prim_channels; k++) {
+        if (get_bits_left(&s->gb) < 0)
+            return -1;
+
         for (l = 0; l < s->vq_start_subband[k]; l++) {
             int m;
 
-- 
1.7.3