Re: [FFmpeg-devel] [libav-devel] [PATCH] opus_silk: fix out of array read in silk_lsf2lpc

2015-12-16 Thread Andreas Cadhalpun 
On 16.12.2015 02:02, Michael Niedermayer wrote:
> On Tue, Dec 15, 2015 at 10:41:15PM +0100, Andreas Cadhalpun wrote:
>>  opus_silk.c |2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 253c6f7d87dcadf712840f45dde18f5698b9c1d4  
>> 0001-opus_silk-fix-typo-causing-overflow-in-silk_stabiliz.patch
>> From d3b2c24d391d7871b978b3372f0f740730996ded Mon Sep 17 00:00:00 2001
>> From: Andreas Cadhalpun 
>> Date: Tue, 15 Dec 2015 22:00:31 +0100
>> Subject: [PATCH] opus_silk: fix typo causing overflow in silk_stabilize_lsf
>>
>> Due to this typo max_center can be too large, causing nlsf to be set to
>> too large values, which in turn can cause nlsf[i - 1] + min_delta[i] to
>> overflow to a negative value, which is not allowed for nlsf and can
>> cause an out of bounds read in silk_lsf2lpc.
> 
> this looks nicer then the previous fixes

Indeed, pushed.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [libav-devel] [PATCH] opus_silk: fix out of array read in silk_lsf2lpc

2015-12-15 Thread Andreas Cadhalpun 
On 15.12.2015 08:17, Anton Khirnov wrote:
> Can you share the sample that shows the problem?

I could, but it's of no use for comparing with libopus, because their
decoder errors out in an unrelated check.

> From what I can see, libopus does not do any clipping at that point, so
> something else must ensure that there is no overflow.

Indeed, the real problem is just a silly typo...
It might have been caused by the fact that the specification uses
i and k in exactly the opposite way than the code.

New patch attached.

Best regards,
Andreas


>From d3b2c24d391d7871b978b3372f0f740730996ded Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun 
Date: Tue, 15 Dec 2015 22:00:31 +0100
Subject: [PATCH] opus_silk: fix typo causing overflow in silk_stabilize_lsf

Due to this typo max_center can be too large, causing nlsf to be set to
too large values, which in turn can cause nlsf[i - 1] + min_delta[i] to
overflow to a negative value, which is not allowed for nlsf and can
cause an out of bounds read in silk_lsf2lpc.

Signed-off-by: Andreas Cadhalpun 
---
 libavcodec/opus_silk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/opus_silk.c b/libavcodec/opus_silk.c
index 841d1ed..73526f9 100644
--- a/libavcodec/opus_silk.c
+++ b/libavcodec/opus_silk.c
@@ -824,7 +824,7 @@ static inline void silk_stabilize_lsf(int16_t nlsf[16], int order, const uint16_
 
 /* upper extent */
 for (i = order; i > k; i--)
-max_center -= min_delta[k];
+max_center -= min_delta[i];
 max_center -= min_delta[k] >> 1;
 
 /* move apart */
-- 
2.6.2

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

Re: [FFmpeg-devel] [libav-devel] [PATCH] opus_silk: fix out of array read in silk_lsf2lpc

2015-12-15 Thread Michael Niedermayer 
On Tue, Dec 15, 2015 at 10:41:15PM +0100, Andreas Cadhalpun wrote:
> On 15.12.2015 08:17, Anton Khirnov wrote:
> > Can you share the sample that shows the problem?
> 
> I could, but it's of no use for comparing with libopus, because their
> decoder errors out in an unrelated check.
> 
> > From what I can see, libopus does not do any clipping at that point, so
> > something else must ensure that there is no overflow.
> 
> Indeed, the real problem is just a silly typo...
> It might have been caused by the fact that the specification uses
> i and k in exactly the opposite way than the code.
> 
> New patch attached.
> 
> Best regards,
> Andreas
> 
> 

>  opus_silk.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 253c6f7d87dcadf712840f45dde18f5698b9c1d4  
> 0001-opus_silk-fix-typo-causing-overflow-in-silk_stabiliz.patch
> From d3b2c24d391d7871b978b3372f0f740730996ded Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun 
> Date: Tue, 15 Dec 2015 22:00:31 +0100
> Subject: [PATCH] opus_silk: fix typo causing overflow in silk_stabilize_lsf
> 
> Due to this typo max_center can be too large, causing nlsf to be set to
> too large values, which in turn can cause nlsf[i - 1] + min_delta[i] to
> overflow to a negative value, which is not allowed for nlsf and can
> cause an out of bounds read in silk_lsf2lpc.

this looks nicer then the previous fixes

thx

[...]


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel