| Message ID | 20240329193221.11522-3-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/jpeg2000htdec: Check magp before using it in a shift | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
fre 2024-03-29 klockan 20:32 +0100 skrev Michael Niedermayer: > This is kind of ugly > Fixes: signed integer overflow: 255 * 1157565362826411919 cannot be > represented in type 'long' > Fixes: 67313/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer- > 6250434245230592 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/mxfdec.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > index c9af4628555..fe86f516630 100644 > --- a/libavformat/mxfdec.c > +++ b/libavformat/mxfdec.c > @@ -1891,9 +1891,14 @@ static int > mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_t > if (edit_unit < s->index_start_position + s->index_duration) > { > int64_t index = edit_unit - s->index_start_position; > > - if (s->edit_unit_byte_count) > + if (s->edit_unit_byte_count) { > + if (s->edit_unit_byte_count * (uint64_t)index / s- > >edit_unit_byte_count != index || Don't we already have a function for testing these kinds of overflows for av_calloc()? Or do it manually less uglily like so: index > INT64_MAX / s->edit_unit_byte_count > + s->edit_unit_byte_count * index > INT64_MAX - > offset_temp > + ) Nit: looks a bit weird to have the ) there rather than at the end of the previous line /Tomas
On Sat, Mar 30, 2024 at 10:01:32AM +0100, Tomas Härdin wrote: > fre 2024-03-29 klockan 20:32 +0100 skrev Michael Niedermayer: > > This is kind of ugly > > Fixes: signed integer overflow: 255 * 1157565362826411919 cannot be > > represented in type 'long' > > Fixes: 67313/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer- > > 6250434245230592 > > > > Found-by: continuous fuzzing process > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/mxfdec.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > > index c9af4628555..fe86f516630 100644 > > --- a/libavformat/mxfdec.c > > +++ b/libavformat/mxfdec.c > > @@ -1891,9 +1891,14 @@ static int > > mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_t > > if (edit_unit < s->index_start_position + s->index_duration) > > { > > int64_t index = edit_unit - s->index_start_position; > > > > - if (s->edit_unit_byte_count) > > + if (s->edit_unit_byte_count) { > > + if (s->edit_unit_byte_count * (uint64_t)index / s- > > >edit_unit_byte_count != index || > > Don't we already have a function for testing these kinds of overflows We have av_size_mult() thats for size_t > for av_calloc()? Or do it manually less uglily like so: > > index > INT64_MAX / s->edit_unit_byte_count ok > > > + s->edit_unit_byte_count * index > INT64_MAX - > > offset_temp > > + ) > > Nit: looks a bit weird to have the ) there rather than at the end of > the previous line will push with this changed thx [...]
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c index c9af4628555..fe86f516630 100644 --- a/libavformat/mxfdec.c +++ b/libavformat/mxfdec.c @@ -1891,9 +1891,14 @@ static int mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_t if (edit_unit < s->index_start_position + s->index_duration) { int64_t index = edit_unit - s->index_start_position; - if (s->edit_unit_byte_count) + if (s->edit_unit_byte_count) { + if (s->edit_unit_byte_count * (uint64_t)index / s->edit_unit_byte_count != index || + s->edit_unit_byte_count * index > INT64_MAX - offset_temp + ) + return AVERROR_INVALIDDATA; + offset_temp += s->edit_unit_byte_count * index; - else { + } else { if (s->nb_index_entries == 2 * s->index_duration + 1) index *= 2; /* Avid index */
This is kind of ugly Fixes: signed integer overflow: 255 * 1157565362826411919 cannot be represented in type 'long' Fixes: 67313/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer-6250434245230592 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/mxfdec.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)