| Message ID | 20240807140920.1583-1-kasper93@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/mov: ensure required number of bytes is read | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
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 [...]
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?
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 [...]
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
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;