[FFmpeg-devel] [PATCH] avcodec/wavpack: check for overflow

Paul B Mahol onemda at gmail.com
Wed Jul 3 12:57:17 CEST 2013


On 7/3/13, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Wed, Jul 03, 2013 at 12:13:45AM +0000, Paul B Mahol wrote:
>> On 7/3/13, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Tue, Jul 02, 2013 at 11:05:09PM +0000, Paul B Mahol wrote:
>> >> On 7/2/13, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >> > On Mon, Jun 17, 2013 at 05:49:47PM +0200, Michael Niedermayer wrote:
>> >> >> On Mon, Jun 17, 2013 at 10:54:28AM +0000, Paul B Mahol wrote:
>> >> >> > On 6/15/13, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >> >> > > Fix integer overflow in fate
>> >> >> > >
>> >> >> > > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
>> >> >> > > ---
>> >> >> > >  libavcodec/wavpack.c |   10 ++++++++--
>> >> >> > >  1 file changed, 8 insertions(+), 2 deletions(-)
>> >> >> > >
>> >> >> > > diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
>> >> >> > > index 47f598a..dd273f7 100644
>> >> >> > > --- a/libavcodec/wavpack.c
>> >> >> > > +++ b/libavcodec/wavpack.c
>> >> >> > > @@ -581,8 +581,14 @@ static inline int
>> >> >> > > wv_unpack_stereo(WavpackFrameContext
>> >> >> > > *s, GetBitContext *gb,
>> >> >> > >                      L2 = L + ((s->decorr[i].weightA *
>> >> >> > > (int64_t)A
>> >> >> > > +
>> >> >> > > 512) >>
>> >> >> > > 10);
>> >> >> > >                      R2 = R + ((s->decorr[i].weightB *
>> >> >> > > (int64_t)B
>> >> >> > > +
>> >> >> > > 512) >>
>> >> >> > > 10);
>> >> >> > >                  } else {
>> >> >> > > -                    L2 = L + ((s->decorr[i].weightA * A + 512)
>> >> >> > > >>
>> >> >> > > 10);
>> >> >> > > -                    R2 = R + ((s->decorr[i].weightB * B + 512)
>> >> >> > > >>
>> >> >> > > 10);
>> >> >> > > +                    int64_t Lt = s->decorr[i].weightA *
>> >> >> > > (int64_t)A
>> >> >> > > +
>> >> >> > > 512;
>> >> >> > > +                    int64_t Rt = s->decorr[i].weightB *
>> >> >> > > (int64_t)B
>> >> >> > > +
>> >> >> > > 512;
>> >> >> > > +                    if ((int32_t)Lt != Lt || (int32_t)Rt != Rt)
>> >> >> > > {
>> >> >> > > +                        av_log(s->avctx, AV_LOG_ERROR, "sample
>> >> >> > > overflow %d
>> >> >> >
>> >> >> > This looks extremly ugly.
>> >> >>
>> >> >> Iam quite aware of that, which is part of the reason why i posted
>> >> >> this
>> >> >> (aka do you know a less ugly solution?)
>> >> >
>> >> > ping
>> >> > anyone has a better solution ?
>> >> > do any objections to the original patch remain ?
>> >>
>> >> Yes, i see no point to apply this patch as is.
>> >
>> > what alternative solution do you suggest?
>>
>> There is no alternative solution yet, as you did not provided any real
>> solution to this problem.
>
> If i had a real solution i wouldnt ask for one.
> Repeating my question with different wording isnt going to solve this
> i suspect but then again yeah trying cant hurt. sadly no i still
> have only that ugly solution an alternativ or real or whatever you
> call it solution is still highly welcome
>
>
>>
>> Why only this overflow in decoder need checking?
>
>
>>
>> Why check is needed at all?
>
> It crashes with the right compiler flags, Its undefined behavior

Yes, its undefined behavior when decoding invalid files. Thing is, its
clear I'm against applying this kind of band aid.

>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> While the State exists there can be no freedom; when there is freedom there
> will be no State. -- Vladimir Lenin
>


More information about the ffmpeg-devel mailing list