Re: [FFmpeg-devel] [PATCH] alsdec: validate time diff index

[Michael Niedermayer]

On Sat, Apr 18, 2015 at 08:47:08PM +0200, Michael Niedermayer wrote:
> On Sat, Apr 18, 2015 at 08:42:58PM +0200, Michael Niedermayer wrote:
> > On Sat, Apr 18, 2015 at 08:13:30PM +0200, Andreas Cadhalpun wrote:
> > > On 18.04.2015 20:05, Michael Niedermayer wrote:
> > > > On Sat, Apr 18, 2015 at 06:58:30PM +0200, Andreas Cadhalpun wrote:
> > > >> If begin is smaller than t, the subtraction 'begin -= t' wraps around,
> > > >> because begin is unsigned. The same applies for end < t.
> > > >>
> > > >> This causes segmentation faults.
> > > >>
> > > >> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
> > > >> ---
> > > >>  libavcodec/alsdec.c | 8 ++++++++
> > > >>  1 file changed, 8 insertions(+)
> > > >>
> > > >> diff --git a/libavcodec/alsdec.c b/libavcodec/alsdec.c
> > > >> index f85f1e8..ff6b6cf 100644
> > > >> --- a/libavcodec/alsdec.c
> > > >> +++ b/libavcodec/alsdec.c
> > > >> @@ -1290,8 +1290,16 @@ static int 
> > > >> revert_channel_correlation(ALSDecContext *ctx, ALSBlockData *bd,
> > > >>  
> > > >>              if (ch[dep].time_diff_sign) {
> > > >>                  t      = -t;
> > > >> +                if (begin < t) {
> > > >> +                    av_log(ctx->avctx, AV_LOG_ERROR, "begin %u 
> > > >> smaller than time diff index %d.\n", begin, t);
> > > >> +                    return AVERROR_INVALIDDATA;
> > > >> +                }
> > > > 
> > > > begin is 1, t < 0 also the comparission is unsigned so t will overflow
> > > > in it
> > > 
> > > Sorry, I sent the wrong version of the patch. Fixed one attached.
> > > The t < 0 case is not problematic for the subtraction.
> > [...]
> > > @@ -1290,8 +1290,16 @@ static int 
> > > revert_channel_correlation(ALSDecContext *ctx, ALSBlockData *bd,
> > >  
> > >              if (ch[dep].time_diff_sign) {
> > >                  t      = -t;
> > > +                if (t > 0 && begin < t) {
> > 
> > time_diff_index is always positive, so t is always negative here
> > so this cant be true unless the context got corrupted or iam missing
> > something
> 
> or its uninitialized

i see these get allocated without zeroing, no idea if thats related
to the issue but ill change the alloc as a precautionary meassure

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"

signature.asc
Description: Digital signature

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