| Message ID | CAB0OVGr6Gy+uG7Q3b67ot=4t6Q8hsF7bcy_CBTkcbjLd82k7mA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Aug 22, 2019 at 12:06:03PM +0200, Carl Eugen Hoyos wrote: > Hi! > > Attached patch fixes usage of uninitialized data reading broken r3d files. > > Please review, Carl Eugen > From efce9940b890b9cf5a9e7400b05be10f6906fbb1 Mon Sep 17 00:00:00 2001 > From: Carl Eugen Hoyos <ceffmpeg@gmail.com> > Date: Thu, 22 Aug 2019 12:02:36 +0200 > Subject: [PATCH] lavf/r3d: Check avio_read() return value. > > Fixes use of uninitialized data reading broken files. > Reported-by: Anatoly Trosinenko <anatoly dot trosinenko at gmail> > --- > libavformat/r3d.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/r3d.c b/libavformat/r3d.c > index 224bcf780d..919c3c4960 100644 > --- a/libavformat/r3d.c > +++ b/libavformat/r3d.c > @@ -98,7 +98,8 @@ static int r3d_read_red1(AVFormatContext *s) > r3d->audio_channels = avio_r8(s->pb); // audio channels > av_log(s, AV_LOG_TRACE, "audio channels %d\n", tmp); > > - avio_read(s->pb, filename, 257); > + if (avio_read(s->pb, filename, 257) < 0) > + return AVERROR_INVALIDDATA; > filename[sizeof(filename)-1] = 0; As far as I can tell most code would either return the avio_read return value or AVERROR(EIO). Also I think short reads are also an issue and would cause the same issue? Maybe it would be sensible to also change the 257 to sizeof(filename)-1 while at it? Overall something along the lines (with possibly less stupid names) of namebytes = sizeof(filename)-1; ret = avio_read(s->pb, filename, namebytes); if (ret < namebytes) return ret < 0 ? ret : AVERROR(EIO); filename[namebytes] = 0; The hard failure return here seems appropriate as this logic reads the very start of the file, so if a read error happens here I don't think there's any way to recover anything.
From efce9940b890b9cf5a9e7400b05be10f6906fbb1 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <ceffmpeg@gmail.com> Date: Thu, 22 Aug 2019 12:02:36 +0200 Subject: [PATCH] lavf/r3d: Check avio_read() return value. Fixes use of uninitialized data reading broken files. Reported-by: Anatoly Trosinenko <anatoly dot trosinenko at gmail> --- libavformat/r3d.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavformat/r3d.c b/libavformat/r3d.c index 224bcf780d..919c3c4960 100644 --- a/libavformat/r3d.c +++ b/libavformat/r3d.c @@ -98,7 +98,8 @@ static int r3d_read_red1(AVFormatContext *s) r3d->audio_channels = avio_r8(s->pb); // audio channels av_log(s, AV_LOG_TRACE, "audio channels %d\n", tmp); - avio_read(s->pb, filename, 257); + if (avio_read(s->pb, filename, 257) < 0) + return AVERROR_INVALIDDATA; filename[sizeof(filename)-1] = 0; av_dict_set(&st->metadata, "filename", filename, 0); -- 2.22.0