| Message ID | 20240401160835.19523-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/mxfdec: Check that edit_unit_byte_count is not negative | 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 |
On Mon, 1 Apr 2024, Michael Niedermayer wrote: > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/mxfdec.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > index e484db052ef..37446963369 100644 > --- a/libavformat/mxfdec.c > +++ b/libavformat/mxfdec.c > @@ -1245,9 +1245,13 @@ static int mxf_read_index_entry_array(AVIOContext *pb, MXFIndexTableSegment *seg > static int mxf_read_index_table_segment(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) > { > MXFIndexTableSegment *segment = arg; > + int tmp; > switch(tag) { > case 0x3F05: > - segment->edit_unit_byte_count = avio_rb32(pb); Why not simply make segment->edit_unit_byte_count unsigned? Thanks, Marton > + tmp = avio_rb32(pb); > + if (tmp < 0) > + return AVERROR_INVALIDDATA; > + segment->edit_unit_byte_count = tmp; > av_log(NULL, AV_LOG_TRACE, "EditUnitByteCount %d\n", segment->edit_unit_byte_count); > break; > case 0x3F06: > -- > 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 01, 2024 at 06:22:47PM +0200, Marton Balint wrote: > > > On Mon, 1 Apr 2024, Michael Niedermayer wrote: > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/mxfdec.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > > index e484db052ef..37446963369 100644 > > --- a/libavformat/mxfdec.c > > +++ b/libavformat/mxfdec.c > > @@ -1245,9 +1245,13 @@ static int mxf_read_index_entry_array(AVIOContext *pb, MXFIndexTableSegment *seg > > static int mxf_read_index_table_segment(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) > > { > > MXFIndexTableSegment *segment = arg; > > + int tmp; > > switch(tag) { > > case 0x3F05: > > - segment->edit_unit_byte_count = avio_rb32(pb); > > Why not simply make segment->edit_unit_byte_count unsigned? ok will apply that instead thx [...]
mån 2024-04-01 klockan 18:22 +0200 skrev Marton Balint: > > > On Mon, 1 Apr 2024, Michael Niedermayer wrote: > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/mxfdec.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > > index e484db052ef..37446963369 100644 > > --- a/libavformat/mxfdec.c > > +++ b/libavformat/mxfdec.c > > @@ -1245,9 +1245,13 @@ static int > > mxf_read_index_entry_array(AVIOContext *pb, MXFIndexTableSegment > > *seg > > static int mxf_read_index_table_segment(void *arg, AVIOContext *pb, > > int tag, int size, UID uid, int64_t klv_offset) > > { > > MXFIndexTableSegment *segment = arg; > > + int tmp; > > switch(tag) { > > case 0x3F05: > > - segment->edit_unit_byte_count = avio_rb32(pb); > > Why not simply make segment->edit_unit_byte_count unsigned? This might run afoul with various calcultions. Speaking of, mxf_edit_unit_absolute_offset() does not check for multiplication overflows.. /Tomas
On Wed, 3 Apr 2024, Tomas Härdin wrote: > mån 2024-04-01 klockan 18:22 +0200 skrev Marton Balint: >> >> >> On Mon, 1 Apr 2024, Michael Niedermayer wrote: >> >> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >> > --- >> > libavformat/mxfdec.c | 6 +++++- >> > 1 file changed, 5 insertions(+), 1 deletion(-) >> > >> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c >> > index e484db052ef..37446963369 100644 >> > --- a/libavformat/mxfdec.c >> > +++ b/libavformat/mxfdec.c >> > @@ -1245,9 +1245,13 @@ static int >> > mxf_read_index_entry_array(AVIOContext *pb, MXFIndexTableSegment >> > *seg >> > static int mxf_read_index_table_segment(void *arg, AVIOContext *pb, >> > int tag, int size, UID uid, int64_t klv_offset) >> > { >> > MXFIndexTableSegment *segment = arg; >> > + int tmp; >> > switch(tag) { >> > case 0x3F05: >> > - segment->edit_unit_byte_count = avio_rb32(pb); >> >> Why not simply make segment->edit_unit_byte_count unsigned? > > This might run afoul with various calcultions. Speaking of, > mxf_edit_unit_absolute_offset() does not check for multiplication > overflows.. Michael's earlier patch fixed that, and the fix should work for both signed and unsigned. Regards, Marton
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c index e484db052ef..37446963369 100644 --- a/libavformat/mxfdec.c +++ b/libavformat/mxfdec.c @@ -1245,9 +1245,13 @@ static int mxf_read_index_entry_array(AVIOContext *pb, MXFIndexTableSegment *seg static int mxf_read_index_table_segment(void *arg, AVIOContext *pb, int tag, int size, UID uid, int64_t klv_offset) { MXFIndexTableSegment *segment = arg; + int tmp; switch(tag) { case 0x3F05: - segment->edit_unit_byte_count = avio_rb32(pb); + tmp = avio_rb32(pb); + if (tmp < 0) + return AVERROR_INVALIDDATA; + segment->edit_unit_byte_count = tmp; av_log(NULL, AV_LOG_TRACE, "EditUnitByteCount %d\n", segment->edit_unit_byte_count); break; case 0x3F06:
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/mxfdec.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)