[FFmpeg-devel] [PATCH] avformat/mov: ensure required number of bytes is read

[FFmpeg-devel] [PATCH] avformat/mov: ensure required number of bytes is read

[Kacper Michajłow]

Fixes: use-of-uninitialized-value

Found by OSS-Fuzz.
---
 libavformat/mov.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 1052691936..f2d8aee766 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -7096,7 +7096,7 @@ static int mov_read_free(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     if (atom.size < 8)
         return 0;
 
-    ret = avio_read(pb, content, FFMIN(sizeof(content), atom.size));
+    ret = ffio_read_size(pb, content, FFMIN(sizeof(content), atom.size));
     if (ret < 0)
         return ret;
 
-- 
2.43.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] avformat/mov: ensure required number of bytes is read

[Michael Niedermayer]

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

On Wed, Aug 07, 2024 at 04:09:20PM +0200, Kacper Michajłow wrote:
> Fixes: use-of-uninitialized-value
> 
> Found by OSS-Fuzz.
> ---
>  libavformat/mov.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

will apply

thx

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

Any man who breaks a law that conscience tells him is unjust and willingly 
accepts the penalty by staying in jail in order to arouse the conscience of 
the community on the injustice of the law is at that moment expressing the 
very highest respect for law. - Martin Luther King Jr

[-- 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/mov: ensure required number of bytes is read

[James Almer]

On 8/7/2024 11:09 AM, Kacper Michajłow wrote:
> Fixes: use-of-uninitialized-value
> 
> Found by OSS-Fuzz.
> ---
>   libavformat/mov.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 1052691936..f2d8aee766 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -7096,7 +7096,7 @@ static int mov_read_free(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>       if (atom.size < 8)
>           return 0;
>   
> -    ret = avio_read(pb, content, FFMIN(sizeof(content), atom.size));
> +    ret = ffio_read_size(pb, content, FFMIN(sizeof(content), atom.size));
>       if (ret < 0)
>           return ret;

Unrelated (somewhat) to this patch, but why does ffio_read_size() 
replace EOF with INVALIDDATA? Is it a good idea to mask the former?

_______________________________________________
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/mov: ensure required number of bytes is read

[Michael Niedermayer]

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

On Thu, Aug 08, 2024 at 01:09:01PM -0300, James Almer wrote:
> On 8/7/2024 11:09 AM, Kacper Michajłow wrote:
> > Fixes: use-of-uninitialized-value
> > 
> > Found by OSS-Fuzz.
> > ---
> >   libavformat/mov.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 1052691936..f2d8aee766 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -7096,7 +7096,7 @@ static int mov_read_free(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >       if (atom.size < 8)
> >           return 0;
> > -    ret = avio_read(pb, content, FFMIN(sizeof(content), atom.size));
> > +    ret = ffio_read_size(pb, content, FFMIN(sizeof(content), atom.size));
> >       if (ret < 0)
> >           return ret;
> 
> Unrelated (somewhat) to this patch, but why does ffio_read_size() replace
> EOF with INVALIDDATA? Is it a good idea to mask the former?

EOF might be interpreted as normal / no error end of file i guess

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.

[-- 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/mov: ensure required number of bytes is read

[Andreas Rheinhardt]

James Almer:
> On 8/7/2024 11:09 AM, Kacper Michajłow wrote:
>> Fixes: use-of-uninitialized-value
>>
>> Found by OSS-Fuzz.
>> ---
>>   libavformat/mov.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 1052691936..f2d8aee766 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -7096,7 +7096,7 @@ static int mov_read_free(MOVContext *c,
>> AVIOContext *pb, MOVAtom atom)
>>       if (atom.size < 8)
>>           return 0;
>>   -    ret = avio_read(pb, content, FFMIN(sizeof(content), atom.size));
>> +    ret = ffio_read_size(pb, content, FFMIN(sizeof(content),
>> atom.size));
>>       if (ret < 0)
>>           return ret;
> 
> Unrelated (somewhat) to this patch, but why does ffio_read_size()
> replace EOF with INVALIDDATA? Is it a good idea to mask the former?
> 

ffio_read_size() is supposed to be used in scenarios where a certain
number of bytes is supposed to be available (e.g. because some size
field says that there have to be that many bytes of payload there). If
we are at EOF when there is supposed to be data, the file is invalid.

- Andreas

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