| Message ID | 20230416164830.15664-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,01/11] avcodec/adpcm: Fix integer overflow in intermediate in ADPCM_XMD | expand |
| Context | Check | Description |
|---|---|---|
| andriy/commit_msg_x86 | warning | Please wrap lines in the body of the commit message between 60 and 72 characters. |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
NAK, breaks decoding.
On Sun, Apr 16, 2023 at 6:48 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > Fixes: runtime error: signed integer overflow: 2140143616 + 254665816 > cannot be represented in type 'int' > Fixes: > 45982/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ADPCM_XMD_fuzzer-6690181676924928 > > As a sideeffect this simplifies the equation, the high bits are different > after this but only > the low 16bits are stored and used in later steps. > The change is untested as there are no fate testcases, no sample files on > the server, no links on > the mailing list and no reports on trac referencing this format that i > could find. > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by > <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>: > Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/adpcm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c > index 451696932d1..d8f334cf5a0 100644 > --- a/libavcodec/adpcm.c > +++ b/libavcodec/adpcm.c > @@ -1579,11 +1579,11 @@ static int adpcm_decode_frame(AVCodecContext > *avctx, AVFrame *frame, > nibble[0] = sign_extend(byte & 15, 4); > nibble[1] = sign_extend(byte >> 4, 4); > > - out[2+n*2] = (nibble[0]*(scale<<14) + > (history[0]*29336) - (history[1]*13136)) >> 14; > + out[2+n*2 ] = nibble[0]*scale + ((history[0]*3667 - > history[1]*1642) >> 11); > Please commit this with no extra spaces added. Here is sample: https://0x0.st/H8Le.xmd > history[1] = history[0]; > history[0] = out[2+n*2]; > > - out[2+n*2+1] = (nibble[1]*(scale<<14) + > (history[0]*29336) - (history[1]*13136)) >> 14; > + out[2+n*2+1] = nibble[1]*scale + ((history[0]*3667 - > history[1]*1642) >> 11); > history[1] = history[0]; > history[0] = out[2+n*2+1]; > } > -- > 2.17.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Mon, Apr 17, 2023 at 09:27:03AM +0200, Paul B Mahol wrote: > On Sun, Apr 16, 2023 at 6:48 PM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > Fixes: runtime error: signed integer overflow: 2140143616 + 254665816 > > cannot be represented in type 'int' > > Fixes: > > 45982/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ADPCM_XMD_fuzzer-6690181676924928 > > > > As a sideeffect this simplifies the equation, the high bits are different > > after this but only > > the low 16bits are stored and used in later steps. > > The change is untested as there are no fate testcases, no sample files on > > the server, no links on > > the mailing list and no reports on trac referencing this format that i > > could find. > > > > Found-by: continuous fuzzing process > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by > > <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>: > > Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/adpcm.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c > > index 451696932d1..d8f334cf5a0 100644 > > --- a/libavcodec/adpcm.c > > +++ b/libavcodec/adpcm.c > > @@ -1579,11 +1579,11 @@ static int adpcm_decode_frame(AVCodecContext > > *avctx, AVFrame *frame, > > nibble[0] = sign_extend(byte & 15, 4); > > nibble[1] = sign_extend(byte >> 4, 4); > > > > - out[2+n*2] = (nibble[0]*(scale<<14) + > > (history[0]*29336) - (history[1]*13136)) >> 14; > > + out[2+n*2 ] = nibble[0]*scale + ((history[0]*3667 - > > history[1]*1642) >> 11); > > > > Please commit this with no extra spaces added. ok > > Here is sample: https://0x0.st/H8Le.xmd thanks alot, i will test with this before applying [...]
On Mon, Apr 17, 2023 at 09:04:48AM +0200, Paul B Mahol wrote:
> NAK, breaks decoding.
The file you posted decodes the same before and after the patch
is there some other issue ?
or is tha patch ok with the whitespace change removed ?
thx
[...]
On Tue, Apr 18, 2023 at 12:45:01AM +0200, Michael Niedermayer wrote: > On Mon, Apr 17, 2023 at 09:04:48AM +0200, Paul B Mahol wrote: > > NAK, breaks decoding. > > The file you posted decodes the same before and after the patch > is there some other issue ? > or is tha patch ok with the whitespace change removed ? will apply patchset ill wait a little more for patch #1 before applying thx [...]
On Sat, May 6, 2023 at 12:15 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Tue, Apr 18, 2023 at 12:45:01AM +0200, Michael Niedermayer wrote: > > On Mon, Apr 17, 2023 at 09:04:48AM +0200, Paul B Mahol wrote: > > > NAK, breaks decoding. > > > > The file you posted decodes the same before and after the patch > > is there some other issue ? > > or is tha patch ok with the whitespace change removed ? > > will apply patchset > ill wait a little more for patch #1 before applying > I sent another mail that approved it, after I tested changes. No need to wait. > > thx > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > He who knows, does not speak. He who speaks, does not know. -- Lao Tsu > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Sat, May 06, 2023 at 12:24:15AM +0200, Paul B Mahol wrote: > On Sat, May 6, 2023 at 12:15 AM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > On Tue, Apr 18, 2023 at 12:45:01AM +0200, Michael Niedermayer wrote: > > > On Mon, Apr 17, 2023 at 09:04:48AM +0200, Paul B Mahol wrote: > > > > NAK, breaks decoding. > > > > > > The file you posted decodes the same before and after the patch > > > is there some other issue ? > > > or is tha patch ok with the whitespace change removed ? > > > > will apply patchset > > ill wait a little more for patch #1 before applying > > > > I sent another mail that approved it, after I tested changes. No need to > wait. ok thx [...]
diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c index 451696932d1..d8f334cf5a0 100644 --- a/libavcodec/adpcm.c +++ b/libavcodec/adpcm.c @@ -1579,11 +1579,11 @@ static int adpcm_decode_frame(AVCodecContext *avctx, AVFrame *frame, nibble[0] = sign_extend(byte & 15, 4); nibble[1] = sign_extend(byte >> 4, 4); - out[2+n*2] = (nibble[0]*(scale<<14) + (history[0]*29336) - (history[1]*13136)) >> 14; + out[2+n*2 ] = nibble[0]*scale + ((history[0]*3667 - history[1]*1642) >> 11); history[1] = history[0]; history[0] = out[2+n*2]; - out[2+n*2+1] = (nibble[1]*(scale<<14) + (history[0]*29336) - (history[1]*13136)) >> 14; + out[2+n*2+1] = nibble[1]*scale + ((history[0]*3667 - history[1]*1642) >> 11); history[1] = history[0]; history[0] = out[2+n*2+1]; }
Fixes: runtime error: signed integer overflow: 2140143616 + 254665816 cannot be represented in type 'int' Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ADPCM_XMD_fuzzer-6690181676924928 As a sideeffect this simplifies the equation, the high bits are different after this but only the low 16bits are stored and used in later steps. The change is untested as there are no fate testcases, no sample files on the server, no links on the mailing list and no reports on trac referencing this format that i could find. Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/adpcm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)