[FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'

[Michael Niedermayer]

On Fri, May 26, 2017 at 06:07:35PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Fri, May 26, 2017 at 5:50 PM, Michael Niedermayer <michael at niedermayer.cc
> > wrote:
> 
> > On Fri, May 26, 2017 at 11:18:12PM +0200, Hendrik Leppkes wrote:
> > > On Fri, May 26, 2017 at 11:11 PM, Michael Niedermayer
> > > <michael at niedermayer.cc> wrote:
> > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov wrote:
> > > >> On 26 May 2017 at 12:21, wm4 <nfxjfg at googlemail.com> wrote:
> > > >>
> > > >> > On Thu, 25 May 2017 16:10:49 +0200
> > > >> > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > > >> >
> > > >> > > Fixes: 1735/clusterfuzz-testcase-minimized-5350472347025408
> > > >> > >
> > > >> > > Found-by: continuous fuzzing process
> > https://github.com/google/oss-
> > > >> > fuzz/tree/master/projects/ffmpeg
> > > >> > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > >> > > ---
> > > >> > >  libavcodec/fft_template.c | 50 +++++++++++++++++++++++-------
> > > >> > -----------------
> > > >> > >  1 file changed, 25 insertions(+), 25 deletions(-)
> > > >> > >
> > > >> > > diff --git a/libavcodec/fft_template.c b/libavcodec/fft_template.c
> > > >> > > index 480557f49f..e3a37e5d69 100644
> > > >> > > --- a/libavcodec/fft_template.c
> > > >> > > +++ b/libavcodec/fft_template.c
> > > >> > > @@ -249,7 +249,7 @@ static void fft_calc_c(FFTContext *s,
> > FFTComplex *z)
> > > >> > {
> > > >> > >
> > > >> > >      int nbits, i, n, num_transforms, offset, step;
> > > >> > >      int n4, n2, n34;
> > > >> > > -    FFTSample tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > > >> > > +    SUINT tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > > >> >
> > > >> > I want this SUINT thing gone, not have more of it.
> > > >> > _______________________________________________
> > > >> > ffmpeg-devel mailing list
> > > >> > ffmpeg-devel at ffmpeg.org
> > > >> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > >> >
> > > >>
> > > >> I agree, especially here.
> > > >
> > > >> Overflows should be left to happen in transforms if the input is
> > corrupt.
> > > >
> > > > signed int overflow is not allowed in C, if that is what you meant.
> > > >
> > > >
> > >
> > > Its "undefined", which means what the result will be is not defined -
> > > which in such a DSP function is irrelevant, if all it causes is
> > > corrupt output ... from corrupt input.
> >
> > no, this is not correct.
> > undefined behavior does not mean the effect will be limited to
> > the output.
> > It could cause the process to hard lockup, trigger an exception or
> > set a flag causing errors in later unrelated code.
> 
> 

> We've discussed this before, if you believe this to be exploitable, why
> allow it in our repository at all? I know of no other project that even
> remotely allows such ridiculous things. Please limit exploitable code to
> your personal tree, ffmpeg is not a rootkit.

please calm down, you make all kinds of statments and implications in
the text above which are not true.

This specific code in git triggers undefined behavior, the patch fixes
this undefined behavior.
If that is exploitable (which i neither claim it is nor that it isnt)
its a issue that exists before the patch but not afterwards.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170527/3f4edac6/attachment.sig>