[FFmpeg-devel] [PATCH] avformat/mxfdec: Check that edit_unit_byte_count is not negative

[FFmpeg-devel] [PATCH] avformat/mxfdec: Check that edit_unit_byte_count is not negative

[Michael Niedermayer]

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);
+        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".

Re: [FFmpeg-devel] [PATCH] avformat/mxfdec: Check that edit_unit_byte_count is not negative

[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?

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".
>
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat/mxfdec: Check that edit_unit_byte_count is not negative

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1172 bytes --]

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

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat/mxfdec: Check that edit_unit_byte_count is not negative

[Tomas Härdin]

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
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat/mxfdec: Check that edit_unit_byte_count is not negative

[Marton Balint]

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
_______________________________________________
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".