[FFmpeg-devel] [PATCH 2/3] dca: fix reading over the end of the allocated buffer

[Kostya Shishkov]

On 6 January 2011 15:07, Anssi Hannula <anssi.hannula at iki.fi> wrote:
> 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.
> ---
> ?libavcodec/dca.c | ? 16 ++++++++++++++++
> ?1 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/libavcodec/dca.c b/libavcodec/dca.c
> index 83f529b..52eac77 100644
> --- a/libavcodec/dca.c
> +++ b/libavcodec/dca.c
> @@ -282,6 +282,10 @@ typedef struct {
>
> ? ? uint8_t dca_buffer[DCA_MAX_FRAME_SIZE];
> ? ? int dca_buffer_size; ? ? ? ?///< how much data is in the dca_buffer
> + ? ?/* NOTE: The decoder relies on there being at least 1kB of allocated
> + ? ? * memory after dca_buffer in case of rogue input causing the decoder
> + ? ? * to read over the end of the buffer (this is mostly provided by the
> + ? ? * large DSPContext below). */

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

> ? ? const int8_t* channel_order_tab; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ///< channel reordering table, lfe and non lfe
> ? ? GetBitContext gb;
> @@ -558,6 +562,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 +621,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 +659,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 +1019,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;
>

Those checks look ok