On 28.04.2015 03:18, Michael Niedermayer wrote:
> On Mon, Apr 27, 2015 at 11:56:15PM +0200, Andreas Cadhalpun wrote:
>> s->decoded_buffer is allocated with a min_size of:
>>     2 * FFALIGN(blockstodecode, 8) * sizeof(*s->decoded_buffer)
>>
>> Then it is assigned to s->decoded[0], which is passed as out buffer to
>> decode_array_0000.
>>
>> In this function 64 elements of the out buffer are written
>> unconditionally and outside the array if blocksdecode is too small.
>>
>> This causes memory corruption, leading to segmentation faults or other 
>> crashes.
>>
>> Thus check that FFALIGN(blockstodecode, 8) is at least 32, i. e. the
>> decoded_buffer has at least 64 components.
> 
> the stereo case would need a check against 64 i think

Yes.

> also if this is specifific to decode_array_0000(), then the others
> should not fail with a short array

OK.

> or decode_array_0000() could be made to just write less or error
> out

decode_array_0000 is void so error out would require more changes,
but just writing less seems like a better fix anyway. New patch attached.

Best regards,
Andreas

>From 969592cc6c04571afa0d8b32be31caf78ca52517 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Tue, 28 Apr 2015 11:13:43 +0200
Subject: [PATCH] apedec: prevent out of array writes in decode_array_0000

s->decoded_buffer is allocated with a min_size of:
    2 * FFALIGN(blockstodecode, 8) * sizeof(*s->decoded_buffer)

Then it is assigned to s->decoded[0] (and s->decoded_buffer + FFALIGN(blockstodecode, 8)
to s->decoded[1]) and passed as out buffer to decode_array_0000.

In this function 64 elements of the out buffer are written
unconditionally and outside the array if blockstodecode is too small.

This causes memory corruption, leading to segmentation faults or other
crashes.

Thus change decode_array_0000 to write at most blockstodecode elements
of the out buffer.

Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
---
 libavcodec/apedec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
index ffd54c1..03afd75 100644
--- a/libavcodec/apedec.c
+++ b/libavcodec/apedec.c
@@ -592,14 +592,14 @@ static void decode_array_0000(APEContext *ctx, GetBitContext *gb,
     int ksummax, ksummin;
 
     rice->ksum = 0;
-    for (i = 0; i < 5; i++) {
+    for (i = 0; i < FFMIN(blockstodecode, 5); i++) {
         out[i] = get_rice_ook(&ctx->gb, 10);
         rice->ksum += out[i];
     }
     rice->k = av_log2(rice->ksum / 10) + 1;
     if (rice->k >= 24)
         return;
-    for (; i < 64; i++) {
+    for (; i < FFMIN(blockstodecode, 64); i++) {
         out[i] = get_rice_ook(&ctx->gb, rice->k);
         rice->ksum += out[i];
         rice->k = av_log2(rice->ksum / ((i + 1) * 2)) + 1;
-- 
2.1.4

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to