| Message ID | 20240912233337.2444412-5-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/7] avformat/mov_chan: Check for FF_SANE_NB_CHANNELS | 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-09-13 klockan 01:33 +0200 skrev Michael Niedermayer: > Fixes: signed integer overflow: 9223372036854775807 - - > 1927491430256034080 cannot be represented in type 'long' > Fixes: 70607/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer- > 5282235077951488 > > 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 | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > index 8eae9f87afa..41281c5196d 100644 > --- a/libavformat/mxfdec.c > +++ b/libavformat/mxfdec.c > @@ -1924,6 +1924,11 @@ static int > mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_t > return mxf_absolute_bodysid_offset(mxf, index_table- > >body_sid, offset_temp, offset_out, partition_out); > } else { > /* EditUnitByteCount == 0 for VBR indexes, which is fine > since they use explicit StreamOffsets */ > + if (s->edit_unit_byte_count && s->index_duration > > INT64_MAX / s->edit_unit_byte_count || > + s->edit_unit_byte_count * s->index_duration > > INT64_MAX - offset_temp > + ) > + return AVERROR_INVALIDDATA; Looks OK /Tomas
fre 2024-09-13 klockan 01:33 +0200 skrev Michael Niedermayer: > Fixes: signed integer overflow: 9223372036854775807 - - > 1927491430256034080 cannot be represented in type 'long' > Fixes: 70607/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer- > 5282235077951488 > > 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 | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > index 8eae9f87afa..41281c5196d 100644 > --- a/libavformat/mxfdec.c > +++ b/libavformat/mxfdec.c > @@ -1924,6 +1924,11 @@ static int > mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_t > return mxf_absolute_bodysid_offset(mxf, index_table- > >body_sid, offset_temp, offset_out, partition_out); > } else { > /* EditUnitByteCount == 0 for VBR indexes, which is fine > since they use explicit StreamOffsets */ > + if (s->edit_unit_byte_count && s->index_duration > > INT64_MAX / s->edit_unit_byte_count || > + s->edit_unit_byte_count * s->index_duration > > INT64_MAX - offset_temp > + ) > + return AVERROR_INVALIDDATA; Actually there's one minor issue: kdevelop warns about lack of parenthesis around the && and || /Tomas
sön 2024-09-15 klockan 22:28 +0200 skrev Tomas Härdin: > fre 2024-09-13 klockan 01:33 +0200 skrev Michael Niedermayer: > > Fixes: signed integer overflow: 9223372036854775807 - - > > 1927491430256034080 cannot be represented in type 'long' > > Fixes: 70607/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer- > > 5282235077951488 > > > > 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 | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > > index 8eae9f87afa..41281c5196d 100644 > > --- a/libavformat/mxfdec.c > > +++ b/libavformat/mxfdec.c > > @@ -1924,6 +1924,11 @@ static int > > mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable > > *index_t > > return mxf_absolute_bodysid_offset(mxf, index_table- > > > body_sid, offset_temp, offset_out, partition_out); > > } else { > > /* EditUnitByteCount == 0 for VBR indexes, which is > > fine > > since they use explicit StreamOffsets */ > > + if (s->edit_unit_byte_count && s->index_duration > > > INT64_MAX / s->edit_unit_byte_count || > > + s->edit_unit_byte_count * s->index_duration > > > INT64_MAX - offset_temp > > + ) > > + return AVERROR_INVALIDDATA; > > Actually there's one minor issue: kdevelop warns about lack of > parenthesis around the && and || It seems where the () are makes little difference, but I'm guessing we want them around the || terms simply because that saves a few cycles /Tomas
On Mon, Sep 16, 2024 at 09:59:11AM +0200, Tomas Härdin wrote: > sön 2024-09-15 klockan 22:28 +0200 skrev Tomas Härdin: > > fre 2024-09-13 klockan 01:33 +0200 skrev Michael Niedermayer: > > > Fixes: signed integer overflow: 9223372036854775807 - - > > > 1927491430256034080 cannot be represented in type 'long' > > > Fixes: 70607/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer- > > > 5282235077951488 > > > > > > 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 | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > > > index 8eae9f87afa..41281c5196d 100644 > > > --- a/libavformat/mxfdec.c > > > +++ b/libavformat/mxfdec.c > > > @@ -1924,6 +1924,11 @@ static int > > > mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable > > > *index_t > > > return mxf_absolute_bodysid_offset(mxf, index_table- > > > > body_sid, offset_temp, offset_out, partition_out); > > > } else { > > > /* EditUnitByteCount == 0 for VBR indexes, which is > > > fine > > > since they use explicit StreamOffsets */ > > > + if (s->edit_unit_byte_count && s->index_duration > > > > INT64_MAX / s->edit_unit_byte_count || > > > + s->edit_unit_byte_count * s->index_duration > > > > INT64_MAX - offset_temp > > > + ) > > > + return AVERROR_INVALIDDATA; > > > > Actually there's one minor issue: kdevelop warns about lack of > > parenthesis around the && and || > > It seems where the () are makes little difference, but I'm guessing we > want them around the || terms simply because that saves a few cycles will apply with it around the || thx [...]
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c index 8eae9f87afa..41281c5196d 100644 --- a/libavformat/mxfdec.c +++ b/libavformat/mxfdec.c @@ -1924,6 +1924,11 @@ static int mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_t return mxf_absolute_bodysid_offset(mxf, index_table->body_sid, offset_temp, offset_out, partition_out); } else { /* EditUnitByteCount == 0 for VBR indexes, which is fine since they use explicit StreamOffsets */ + if (s->edit_unit_byte_count && s->index_duration > INT64_MAX / s->edit_unit_byte_count || + s->edit_unit_byte_count * s->index_duration > INT64_MAX - offset_temp + ) + return AVERROR_INVALIDDATA; + offset_temp += s->edit_unit_byte_count * s->index_duration; } }
Fixes: signed integer overflow: 9223372036854775807 - -1927491430256034080 cannot be represented in type 'long' Fixes: 70607/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer-5282235077951488 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 | 5 +++++ 1 file changed, 5 insertions(+)