[FFmpeg-devel] [PATCH 1/4] avcodec/vp9_superframe_split_bsf: Check in size

[FFmpeg-devel] [PATCH 1/4] avcodec/vp9_superframe_split_bsf: Check in size

[Michael Niedermayer]

Fixes: Out of array read
Fixes: 45137/clusterfuzz-testcase-minimized-ffmpeg_BSF_VP9_SUPERFRAME_SPLIT_fuzzer-4984270639202304

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/vp9_superframe_split_bsf.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/libavcodec/vp9_superframe_split_bsf.c b/libavcodec/vp9_superframe_split_bsf.c
index ed0444561a..6af555c078 100644
--- a/libavcodec/vp9_superframe_split_bsf.c
+++ b/libavcodec/vp9_superframe_split_bsf.c
@@ -51,6 +51,11 @@ static int vp9_superframe_split_filter(AVBSFContext *ctx, AVPacket *out)
             return ret;
         in = s->buffer_pkt;
 
+        if (in->size == 0) {
+            ret = AVERROR_INVALIDDATA;
+            goto fail;
+        }
+
         marker = in->data[in->size - 1];
         if ((marker & 0xe0) == 0xc0) {
             int length_size = 1 + ((marker >> 3) & 0x3);
-- 
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] [PATCH 2/4] avformat/mxfdec: Check count in mxf_read_strong_ref_array()

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/mxfdec.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index b85c10bf19..d7cdd22c8a 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -932,7 +932,13 @@ static int mxf_read_cryptographic_context(void *arg, AVIOContext *pb, int tag, i
 
 static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
 {
-    *count = avio_rb32(pb);
+    unsigned c = avio_rb32(pb);
+
+    //avio_read() used int
+    if (c > INT_MAX / sizeof(UID))
+        return AVERROR_PATCHWELCOME;
+    *count = c;
+
     av_free(*refs);
     *refs = av_calloc(*count, sizeof(UID));
     if (!*refs) {
-- 
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] [PATCH 3/4] avformat/mxfdec: Check for avio_read() failure in mxf_read_strong_ref_array()

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/mxfdec.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index d7cdd22c8a..828fc0f9f1 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -932,6 +932,7 @@ static int mxf_read_cryptographic_context(void *arg, AVIOContext *pb, int tag, i
 
 static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
 {
+    int64_t ret;
     unsigned c = avio_rb32(pb);
 
     //avio_read() used int
@@ -946,7 +947,12 @@ static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
         return AVERROR(ENOMEM);
     }
     avio_skip(pb, 4); /* useless size of objects, always 16 according to specs */
-    avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
+    ret = avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
+    if (ret != *count * sizeof(UID)) {
+        *count = ret < 0 ? 0   : ret / sizeof(UID);
+        return   ret < 0 ? ret : AVERROR_INVALIDDATA;
+    }
+
     return 0;
 }
 
-- 
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] [PATCH 4/4] avformat/mxfdec: Do not clear array in mxf_read_strong_ref_array() before writing

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/mxfdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 828fc0f9f1..f088712494 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -941,7 +941,7 @@ static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
     *count = c;
 
     av_free(*refs);
-    *refs = av_calloc(*count, sizeof(UID));
+    *refs = av_malloc(*count * sizeof(UID));
     if (!*refs) {
         *count = 0;
         return AVERROR(ENOMEM);
-- 
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 3/4] avformat/mxfdec: Check for avio_read() failure in mxf_read_strong_ref_array()

[Marton Balint]

On Sun, 13 Mar 2022, Michael Niedermayer wrote:

> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
> libavformat/mxfdec.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index d7cdd22c8a..828fc0f9f1 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -932,6 +932,7 @@ static int mxf_read_cryptographic_context(void *arg, AVIOContext *pb, int tag, i
>
> static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
> {
> +    int64_t ret;
>     unsigned c = avio_rb32(pb);
>
>     //avio_read() used int
> @@ -946,7 +947,12 @@ static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
>         return AVERROR(ENOMEM);
>     }
>     avio_skip(pb, 4); /* useless size of objects, always 16 according to specs */
> -    avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
> +    ret = avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
> +    if (ret != *count * sizeof(UID)) {
> +        *count = ret < 0 ? 0   : ret / sizeof(UID);

I suggest you hard fail if the read count is not the expected, do not 
silently ignore corrupt file.

Regards,
Marton

> +        return   ret < 0 ? ret : AVERROR_INVALIDDATA;
> +    }
> +
>     return 0;
> }
>
> -- 
> 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 4/4] avformat/mxfdec: Do not clear array in mxf_read_strong_ref_array() before writing

[Marton Balint]

On Sun, 13 Mar 2022, Michael Niedermayer wrote:

> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
> libavformat/mxfdec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index 828fc0f9f1..f088712494 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -941,7 +941,7 @@ static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
>     *count = c;
>
>     av_free(*refs);
> -    *refs = av_calloc(*count, sizeof(UID));
> +    *refs = av_malloc(*count * sizeof(UID));

I suggest av_malloc_array(), even if it can't overflow because of earlier 
checks.

Thanks,
Marton

>     if (!*refs) {
>         *count = 0;
>         return AVERROR(ENOMEM);
> -- 
> 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 1/4] avcodec/vp9_superframe_split_bsf: Check in size

[James Almer]

On 3/12/2022 8:52 PM, Michael Niedermayer wrote:
> Fixes: Out of array read
> Fixes: 45137/clusterfuzz-testcase-minimized-ffmpeg_BSF_VP9_SUPERFRAME_SPLIT_fuzzer-4984270639202304
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavcodec/vp9_superframe_split_bsf.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/libavcodec/vp9_superframe_split_bsf.c b/libavcodec/vp9_superframe_split_bsf.c
> index ed0444561a..6af555c078 100644
> --- a/libavcodec/vp9_superframe_split_bsf.c
> +++ b/libavcodec/vp9_superframe_split_bsf.c
> @@ -51,6 +51,11 @@ static int vp9_superframe_split_filter(AVBSFContext *ctx, AVPacket *out)
>               return ret;
>           in = s->buffer_pkt;
>   
> +        if (in->size == 0) {

!in->size

> +            ret = AVERROR_INVALIDDATA;
> +            goto fail;
> +        }
> +
>           marker = in->data[in->size - 1];

Do we want to abort on in->data && !in->size, or just pass the packet 
through?
I'm partial to the latter, so it would mean initializing marker to 0 and 
check instead for in->size before setting marker, so the check below 
fails and the packet is just passed through.

Not sure what others think about it.

>           if ((marker & 0xe0) == 0xc0) {
>               int length_size = 1 + ((marker >> 3) & 0x3);
_______________________________________________
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 3/4] avformat/mxfdec: Check for avio_read() failure in mxf_read_strong_ref_array()

[Michael Niedermayer]

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

On Sun, Mar 13, 2022 at 04:52:25PM +0100, Marton Balint wrote:
> 
> 
> On Sun, 13 Mar 2022, Michael Niedermayer wrote:
> 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> > libavformat/mxfdec.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > index d7cdd22c8a..828fc0f9f1 100644
> > --- a/libavformat/mxfdec.c
> > +++ b/libavformat/mxfdec.c
> > @@ -932,6 +932,7 @@ static int mxf_read_cryptographic_context(void *arg, AVIOContext *pb, int tag, i
> > 
> > static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
> > {
> > +    int64_t ret;
> >     unsigned c = avio_rb32(pb);
> > 
> >     //avio_read() used int
> > @@ -946,7 +947,12 @@ static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
> >         return AVERROR(ENOMEM);
> >     }
> >     avio_skip(pb, 4); /* useless size of objects, always 16 according to specs */
> > -    avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
> > +    ret = avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
> > +    if (ret != *count * sizeof(UID)) {
> > +        *count = ret < 0 ? 0   : ret / sizeof(UID);
> 

> I suggest you hard fail if the read count is not the expected, do not
> silently ignore corrupt file.
> 
> Regards,
> Marton
> 
> > +        return   ret < 0 ? ret : AVERROR_INVALIDDATA;

Does it not hard fail here ?

thx

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

Take away the freedom of one citizen and you will be jailed, take away
the freedom of all citizens and you will be congratulated by your peers
in Parliament.

[-- 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 4/4] avformat/mxfdec: Do not clear array in mxf_read_strong_ref_array() before writing

[Michael Niedermayer]

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

On Sun, Mar 13, 2022 at 04:53:29PM +0100, Marton Balint wrote:
> 
> 
> On Sun, 13 Mar 2022, Michael Niedermayer wrote:
> 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> > libavformat/mxfdec.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > index 828fc0f9f1..f088712494 100644
> > --- a/libavformat/mxfdec.c
> > +++ b/libavformat/mxfdec.c
> > @@ -941,7 +941,7 @@ static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
> >     *count = c;
> > 
> >     av_free(*refs);
> > -    *refs = av_calloc(*count, sizeof(UID));
> > +    *refs = av_malloc(*count * sizeof(UID));
> 
> I suggest av_malloc_array(), even if it can't overflow because of earlier
> checks.

agree, will change that

thx

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

"You are 36 times more likely to die in a bathtub than at the hands of a
terrorist. Also, you are 2.5 times more likely to become a president and
2 times more likely to become an astronaut, than to die in a terrorist
attack." -- Thoughty2


[-- 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 1/4] avcodec/vp9_superframe_split_bsf: Check in size

[Michael Niedermayer]

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

On Sun, Mar 13, 2022 at 04:03:42PM -0300, James Almer wrote:
> 
> 
> On 3/12/2022 8:52 PM, Michael Niedermayer wrote:
> > Fixes: Out of array read
> > Fixes: 45137/clusterfuzz-testcase-minimized-ffmpeg_BSF_VP9_SUPERFRAME_SPLIT_fuzzer-4984270639202304
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavcodec/vp9_superframe_split_bsf.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/libavcodec/vp9_superframe_split_bsf.c b/libavcodec/vp9_superframe_split_bsf.c
> > index ed0444561a..6af555c078 100644
> > --- a/libavcodec/vp9_superframe_split_bsf.c
> > +++ b/libavcodec/vp9_superframe_split_bsf.c
> > @@ -51,6 +51,11 @@ static int vp9_superframe_split_filter(AVBSFContext *ctx, AVPacket *out)
> >               return ret;
> >           in = s->buffer_pkt;
> > +        if (in->size == 0) {
> 
> !in->size

I favor checking "== 0" when its about the value 0 and !X when its about
something being not set / not allocated.
but i can change it if you prefer

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

The smallest minority on earth is the individual. Those who deny 
individual rights cannot claim to be defenders of minorities. - Ayn Rand

[-- 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 1/4] avcodec/vp9_superframe_split_bsf: Check in size

[James Almer]

On 3/14/2022 11:04 AM, Michael Niedermayer wrote:
> On Sun, Mar 13, 2022 at 04:03:42PM -0300, James Almer wrote:
>>
>>
>> On 3/12/2022 8:52 PM, Michael Niedermayer wrote:
>>> Fixes: Out of array read
>>> Fixes: 45137/clusterfuzz-testcase-minimized-ffmpeg_BSF_VP9_SUPERFRAME_SPLIT_fuzzer-4984270639202304
>>>
>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>    libavcodec/vp9_superframe_split_bsf.c | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/libavcodec/vp9_superframe_split_bsf.c b/libavcodec/vp9_superframe_split_bsf.c
>>> index ed0444561a..6af555c078 100644
>>> --- a/libavcodec/vp9_superframe_split_bsf.c
>>> +++ b/libavcodec/vp9_superframe_split_bsf.c
>>> @@ -51,6 +51,11 @@ static int vp9_superframe_split_filter(AVBSFContext *ctx, AVPacket *out)
>>>                return ret;
>>>            in = s->buffer_pkt;
>>> +        if (in->size == 0) {
>>
>> !in->size
> 
> I favor checking "== 0" when its about the value 0 and !X when its about
> something being not set / not allocated.
> but i can change it if you prefer
> 
> thx

I suggested it for the sake of consistence. Most AVPacket.size checks 
use !X to check for 0.
_______________________________________________
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 1/4] avcodec/vp9_superframe_split_bsf: Check in size

[Michael Niedermayer]

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

On Mon, Mar 14, 2022 at 11:07:38AM -0300, James Almer wrote:
> 
> 
> On 3/14/2022 11:04 AM, Michael Niedermayer wrote:
> > On Sun, Mar 13, 2022 at 04:03:42PM -0300, James Almer wrote:
> > > 
> > > 
> > > On 3/12/2022 8:52 PM, Michael Niedermayer wrote:
> > > > Fixes: Out of array read
> > > > Fixes: 45137/clusterfuzz-testcase-minimized-ffmpeg_BSF_VP9_SUPERFRAME_SPLIT_fuzzer-4984270639202304
> > > > 
> > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > >    libavcodec/vp9_superframe_split_bsf.c | 5 +++++
> > > >    1 file changed, 5 insertions(+)
> > > > 
> > > > diff --git a/libavcodec/vp9_superframe_split_bsf.c b/libavcodec/vp9_superframe_split_bsf.c
> > > > index ed0444561a..6af555c078 100644
> > > > --- a/libavcodec/vp9_superframe_split_bsf.c
> > > > +++ b/libavcodec/vp9_superframe_split_bsf.c
> > > > @@ -51,6 +51,11 @@ static int vp9_superframe_split_filter(AVBSFContext *ctx, AVPacket *out)
> > > >                return ret;
> > > >            in = s->buffer_pkt;
> > > > +        if (in->size == 0) {
> > > 
> > > !in->size
> > 
> > I favor checking "== 0" when its about the value 0 and !X when its about
> > something being not set / not allocated.
> > but i can change it if you prefer
> > 
> > thx
> 
> I suggested it for the sake of consistence. Most AVPacket.size checks use !X
> to check for 0.

changed
thx

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

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes

[-- 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 3/4] avformat/mxfdec: Check for avio_read() failure in mxf_read_strong_ref_array()

[Marton Balint]

On Mon, 14 Mar 2022, Michael Niedermayer wrote:

> On Sun, Mar 13, 2022 at 04:52:25PM +0100, Marton Balint wrote:
>>
>>
>> On Sun, 13 Mar 2022, Michael Niedermayer wrote:
>>
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>> libavformat/mxfdec.c | 8 +++++++-
>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
>>> index d7cdd22c8a..828fc0f9f1 100644
>>> --- a/libavformat/mxfdec.c
>>> +++ b/libavformat/mxfdec.c
>>> @@ -932,6 +932,7 @@ static int mxf_read_cryptographic_context(void *arg, AVIOContext *pb, int tag, i
>>>
>>> static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
>>> {
>>> +    int64_t ret;
>>>     unsigned c = avio_rb32(pb);
>>>
>>>     //avio_read() used int
>>> @@ -946,7 +947,12 @@ static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
>>>         return AVERROR(ENOMEM);
>>>     }
>>>     avio_skip(pb, 4); /* useless size of objects, always 16 according to specs */
>>> -    avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
>>> +    ret = avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
>>> +    if (ret != *count * sizeof(UID)) {
>>> +        *count = ret < 0 ? 0   : ret / sizeof(UID);
>>
>
>> I suggest you hard fail if the read count is not the expected, do not
>> silently ignore corrupt file.
>>
>> Regards,
>> Marton
>>
>>> +        return   ret < 0 ? ret : AVERROR_INVALIDDATA;
>
> Does it not hard fail here ?

Yeah, it does, sorry. This extra count calculation confused me... I'd just 
probably set it to 0 in case of a partial read, same as in case of an 
error, but fine either way I guess.

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

Re: [FFmpeg-devel] [PATCH 2/4] avformat/mxfdec: Check count in mxf_read_strong_ref_array()

[Tomas Härdin]

sön 2022-03-13 klockan 00:52 +0100 skrev Michael Niedermayer:
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/mxfdec.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index b85c10bf19..d7cdd22c8a 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -932,7 +932,13 @@ static int mxf_read_cryptographic_context(void
> *arg, AVIOContext *pb, int tag, i
>  
>  static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs,
> int *count)
>  {
> -    *count = avio_rb32(pb);
> +    unsigned c = avio_rb32(pb);

not uint32_t?

> +
> +    //avio_read() used int
> +    if (c > INT_MAX / sizeof(UID))
> +        return AVERROR_PATCHWELCOME;
> +    *count = c;
> +

This should already be caught by av_calloc(), no?

/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 3/4] avformat/mxfdec: Check for avio_read() failure in mxf_read_strong_ref_array()

[Tomas Härdin]

sön 2022-03-13 klockan 00:52 +0100 skrev Michael Niedermayer:
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/mxfdec.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index d7cdd22c8a..828fc0f9f1 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -932,6 +932,7 @@ static int mxf_read_cryptographic_context(void
> *arg, AVIOContext *pb, int tag, i
>  
>  static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs,
> int *count)
>  {
> +    int64_t ret;
>      unsigned c = avio_rb32(pb);
>  
>      //avio_read() used int
> @@ -946,7 +947,12 @@ static int mxf_read_strong_ref_array(AVIOContext
> *pb, UID **refs, int *count)
>          return AVERROR(ENOMEM);
>      }
>      avio_skip(pb, 4); /* useless size of objects, always 16
> according to specs */
> -    avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
> +    ret = avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
> +    if (ret != *count * sizeof(UID)) {
> +        *count = ret < 0 ? 0   : ret / sizeof(UID);
> +        return   ret < 0 ? ret : AVERROR_INVALIDDATA;

Looks ok

/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 3/4] avformat/mxfdec: Check for avio_read() failure in mxf_read_strong_ref_array()

[Michael Niedermayer]

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

On Mon, Mar 14, 2022 at 06:08:32PM +0100, Marton Balint wrote:
> 
> 
> On Mon, 14 Mar 2022, Michael Niedermayer wrote:
> 
> > On Sun, Mar 13, 2022 at 04:52:25PM +0100, Marton Balint wrote:
> > > 
> > > 
> > > On Sun, 13 Mar 2022, Michael Niedermayer wrote:
> > > 
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > > libavformat/mxfdec.c | 8 +++++++-
> > > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > > index d7cdd22c8a..828fc0f9f1 100644
> > > > --- a/libavformat/mxfdec.c
> > > > +++ b/libavformat/mxfdec.c
> > > > @@ -932,6 +932,7 @@ static int mxf_read_cryptographic_context(void *arg, AVIOContext *pb, int tag, i
> > > > 
> > > > static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
> > > > {
> > > > +    int64_t ret;
> > > >     unsigned c = avio_rb32(pb);
> > > > 
> > > >     //avio_read() used int
> > > > @@ -946,7 +947,12 @@ static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs, int *count)
> > > >         return AVERROR(ENOMEM);
> > > >     }
> > > >     avio_skip(pb, 4); /* useless size of objects, always 16 according to specs */
> > > > -    avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
> > > > +    ret = avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
> > > > +    if (ret != *count * sizeof(UID)) {
> > > > +        *count = ret < 0 ? 0   : ret / sizeof(UID);
> > > 
> > 
> > > I suggest you hard fail if the read count is not the expected, do not
> > > silently ignore corrupt file.
> > > 
> > > Regards,
> > > Marton
> > > 
> > > > +        return   ret < 0 ? ret : AVERROR_INVALIDDATA;
> > 
> > Does it not hard fail here ?
> 
> Yeah, it does, sorry. This extra count calculation confused me... I'd just
> probably set it to 0 in case of a partial read, same as in case of an error,
> but fine either way I guess.

i wanted to set the count to be most correct value. So if someone was
debuging this and tried to just not fail it would already have a useable
value in count.

thx
[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle

[-- 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 2/4] avformat/mxfdec: Check count in mxf_read_strong_ref_array()

[Michael Niedermayer]

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

On Mon, Mar 14, 2022 at 08:19:51PM +0100, Tomas Härdin wrote:
> sön 2022-03-13 klockan 00:52 +0100 skrev Michael Niedermayer:
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavformat/mxfdec.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > index b85c10bf19..d7cdd22c8a 100644
> > --- a/libavformat/mxfdec.c
> > +++ b/libavformat/mxfdec.c
> > @@ -932,7 +932,13 @@ static int mxf_read_cryptographic_context(void
> > *arg, AVIOContext *pb, int tag, i
> >  
> >  static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs,
> > int *count)
> >  {
> > -    *count = avio_rb32(pb);
> > +    unsigned c = avio_rb32(pb);
> 
> not uint32_t?

we dont need an exact length variable here


> 
> > +
> > +    //avio_read() used int
> > +    if (c > INT_MAX / sizeof(UID))
> > +        return AVERROR_PATCHWELCOME;
> > +    *count = c;
> > +
> 
> This should already be caught by av_calloc(), no?

the API as in the documentation of av_calloc() does not gurantee
this. Its bad practice if we write code that depends on some implementation
of some code in a diferent module/lib

thx

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

Does the universe only have a finite lifespan? No, its going to go on
forever, its just that you wont like living in it. -- Hiranya Peiri

[-- 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 3/4] avformat/mxfdec: Check for avio_read() failure in mxf_read_strong_ref_array()

[Michael Niedermayer]

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

On Mon, Mar 14, 2022 at 08:21:04PM +0100, Tomas Härdin wrote:
> sön 2022-03-13 klockan 00:52 +0100 skrev Michael Niedermayer:
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavformat/mxfdec.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > index d7cdd22c8a..828fc0f9f1 100644
> > --- a/libavformat/mxfdec.c
> > +++ b/libavformat/mxfdec.c
> > @@ -932,6 +932,7 @@ static int mxf_read_cryptographic_context(void
> > *arg, AVIOContext *pb, int tag, i
> >  
> >  static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs,
> > int *count)
> >  {
> > +    int64_t ret;
> >      unsigned c = avio_rb32(pb);
> >  
> >      //avio_read() used int
> > @@ -946,7 +947,12 @@ static int mxf_read_strong_ref_array(AVIOContext
> > *pb, UID **refs, int *count)
> >          return AVERROR(ENOMEM);
> >      }
> >      avio_skip(pb, 4); /* useless size of objects, always 16
> > according to specs */
> > -    avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
> > +    ret = avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
> > +    if (ret != *count * sizeof(UID)) {
> > +        *count = ret < 0 ? 0   : ret / sizeof(UID);
> > +        return   ret < 0 ? ret : AVERROR_INVALIDDATA;
> 
> Looks ok

i will apply once we agree on the previous patch as that is on top of it

thx

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

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data

[-- 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 2/4] avformat/mxfdec: Check count in mxf_read_strong_ref_array()

[Tomas Härdin]

lör 2022-03-19 klockan 23:50 +0100 skrev Michael Niedermayer:
> On Mon, Mar 14, 2022 at 08:19:51PM +0100, Tomas Härdin wrote:
> > sön 2022-03-13 klockan 00:52 +0100 skrev Michael Niedermayer:
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > ---
> > >  libavformat/mxfdec.c | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > index b85c10bf19..d7cdd22c8a 100644
> > > --- a/libavformat/mxfdec.c
> > > +++ b/libavformat/mxfdec.c
> > > @@ -932,7 +932,13 @@ static int
> > > mxf_read_cryptographic_context(void
> > > *arg, AVIOContext *pb, int tag, i
> > >  
> > >  static int mxf_read_strong_ref_array(AVIOContext *pb, UID
> > > **refs,
> > > int *count)
> > >  {
> > > -    *count = avio_rb32(pb);
> > > +    unsigned c = avio_rb32(pb);
> > 
> > not uint32_t?
> 
> we dont need an exact length variable here

Right, we don't support 16-bit machines

> 
> 
> > 
> > > +
> > > +    //avio_read() used int
> > > +    if (c > INT_MAX / sizeof(UID))
> > > +        return AVERROR_PATCHWELCOME;
> > > +    *count = c;
> > > +
> > 
> > This should already be caught by av_calloc(), no?
> 
> the API as in the documentation of av_calloc() does not gurantee
> this. 

Yes it does:

  The allocated memory will have size `size * nmemb` bytes.
  [...]
  `NULL` if the block cannot be allocated

> Its bad practice if we write code that depends on some implementation
> of some code in a diferent module/lib

If av_calloc() does not guarantee this then it is useless. It is used
precisely for this all over the place. Are you going to change every
use of av_calloc() in mxfdec in the same way?

/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 2/4] avformat/mxfdec: Check count in mxf_read_strong_ref_array()

[Michael Niedermayer]

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

On Sun, Mar 20, 2022 at 02:05:41PM +0100, Tomas Härdin wrote:
> lör 2022-03-19 klockan 23:50 +0100 skrev Michael Niedermayer:
[...]
> > 
> > 
> > > 
> > > > +
> > > > +    //avio_read() used int
> > > > +    if (c > INT_MAX / sizeof(UID))
> > > > +        return AVERROR_PATCHWELCOME;
> > > > +    *count = c;
> > > > +
> > > 
> > > This should already be caught by av_calloc(), no?
> > 
> > the API as in the documentation of av_calloc() does not gurantee
> > this. 
> 
> Yes it does:
> 
>   The allocated memory will have size `size * nmemb` bytes.
>   [...]
>   `NULL` if the block cannot be allocated

void *av_calloc(size_t nmemb, size_t size)
size_t can be larger than int, so size * nmemb may be larger than INT_MAX


> 
> > Its bad practice if we write code that depends on some implementation
> > of some code in a diferent module/lib
> 
> If av_calloc() does not guarantee this then it is useless. It is used
> precisely for this all over the place. Are you going to change every
> use of av_calloc() in mxfdec in the same way?

well, when max_alloc_size is set above INT_MAX 
then int checks will become needed when these values ever get stored in
ints. For example here avio_read() has a int argument that is set to the
product of the 2. Or all such ints need to be changed to something bigger

thx

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

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato

[-- 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 2/4] avformat/mxfdec: Check count in mxf_read_strong_ref_array()

[Tomas Härdin]

sön 2022-03-20 klockan 15:06 +0100 skrev Michael Niedermayer:
> On Sun, Mar 20, 2022 at 02:05:41PM +0100, Tomas Härdin wrote:
> > lör 2022-03-19 klockan 23:50 +0100 skrev Michael Niedermayer:
> [...]
> > > 
> > > 
> > > > 
> > > > > +
> > > > > +    //avio_read() used int
> > > > > +    if (c > INT_MAX / sizeof(UID))
> > > > > +        return AVERROR_PATCHWELCOME;
> > > > > +    *count = c;
> > > > > +
> > > > 
> > > > This should already be caught by av_calloc(), no?
> > > 
> > > the API as in the documentation of av_calloc() does not gurantee
> > > this. 
> > 
> > Yes it does:
> > 
> >   The allocated memory will have size `size * nmemb` bytes.
> >   [...]
> >   `NULL` if the block cannot be allocated
> 
> void *av_calloc(size_t nmemb, size_t size)
> size_t can be larger than int, so size * nmemb may be larger than
> INT_MAX

Crap, you're right. This also brings to mind the question why
packages_count etc are int rather than unsigned or uint32_t..

Patch is OK then

/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 1/4] avcodec/vp9_superframe_split_bsf: Check in size

[Michael Niedermayer]

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

On Sun, Mar 13, 2022 at 04:03:42PM -0300, James Almer wrote:
> 
> 
> On 3/12/2022 8:52 PM, Michael Niedermayer wrote:
> > Fixes: Out of array read
> > Fixes: 45137/clusterfuzz-testcase-minimized-ffmpeg_BSF_VP9_SUPERFRAME_SPLIT_fuzzer-4984270639202304
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavcodec/vp9_superframe_split_bsf.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/libavcodec/vp9_superframe_split_bsf.c b/libavcodec/vp9_superframe_split_bsf.c
> > index ed0444561a..6af555c078 100644
> > --- a/libavcodec/vp9_superframe_split_bsf.c
> > +++ b/libavcodec/vp9_superframe_split_bsf.c
> > @@ -51,6 +51,11 @@ static int vp9_superframe_split_filter(AVBSFContext *ctx, AVPacket *out)
> >               return ret;
> >           in = s->buffer_pkt;
> > +        if (in->size == 0) {
> 
> !in->size
> 
> > +            ret = AVERROR_INVALIDDATA;
> > +            goto fail;
> > +        }
> > +
> >           marker = in->data[in->size - 1];
> 
> Do we want to abort on in->data && !in->size, or just pass the packet
> through?
> I'm partial to the latter, so it would mean initializing marker to 0 and
> check instead for in->size before setting marker, so the check below fails
> and the packet is just passed through.
> 
> Not sure what others think about it.

I have no opinion on this. I just want the bug fixed. As both andreas
and ossfuzz noticed this affects other filters too.
not sure the passthough makes sense for all them, i will just dumbly
post a passthrough based patch for the other one ossfuzz found too
but maybe erroring out as in andreas patch makes more sense

thx

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

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."

[-- 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 2/4] avformat/mxfdec: Check count in mxf_read_strong_ref_array()

[Michael Niedermayer]

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

On Mon, Mar 21, 2022 at 11:06:14AM +0100, Tomas Härdin wrote:
> sön 2022-03-20 klockan 15:06 +0100 skrev Michael Niedermayer:
> > On Sun, Mar 20, 2022 at 02:05:41PM +0100, Tomas Härdin wrote:
> > > lör 2022-03-19 klockan 23:50 +0100 skrev Michael Niedermayer:
> > [...]
> > > > 
> > > > 
> > > > > 
> > > > > > +
> > > > > > +    //avio_read() used int
> > > > > > +    if (c > INT_MAX / sizeof(UID))
> > > > > > +        return AVERROR_PATCHWELCOME;
> > > > > > +    *count = c;
> > > > > > +
> > > > > 
> > > > > This should already be caught by av_calloc(), no?
> > > > 
> > > > the API as in the documentation of av_calloc() does not gurantee
> > > > this. 
> > > 
> > > Yes it does:
> > > 
> > >   The allocated memory will have size `size * nmemb` bytes.
> > >   [...]
> > >   `NULL` if the block cannot be allocated
> > 
> > void *av_calloc(size_t nmemb, size_t size)
> > size_t can be larger than int, so size * nmemb may be larger than
> > INT_MAX
> 
> Crap, you're right. This also brings to mind the question why
> packages_count etc are int rather than unsigned or uint32_t..
> 
> Patch is OK then

will apply

thx

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

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus

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