| Message ID | 20181213192826.92374-1-chcunningham@chromium.org |
|---|---|
| State | Accepted |
| Headers | show |
tor 2018-12-13 klockan 11:28 -0800 skrev chcunningham: > avio_read may return EOF, leaving the mimetype array unitialized. fail > early when this occurs to avoid using the array in an unitialized state. > --- > libavformat/id3v2.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c > index f7de26a1d8..f06b9a787e 100644 > --- a/libavformat/id3v2.c > +++ b/libavformat/id3v2.c > @@ -609,10 +609,13 @@ static void read_apic(AVFormatContext *s, AVIOContext *pb, int taglen, > taglen--; > > /* mimetype */ > + memset(mimetype, 0, sizeof(mimetype)); -char mimetype[64]; +char mimetype[64] = {0}; would be enough /Tomas
> > -char mimetype[64]; > +char mimetype[64] = {0}; > > would be enough > Agree! Next patch. I'd love to do similar for the isv34 branch, but I'm not sure how to detect > the condition given the EOF behavior for avio_get_str: > * @return number of bytes read (is always <= maxlen). > * If reading ends on EOF or error, the return value will be one more than > * bytes actually read. > How do callers differentiate between cases where you read 5 bytes vs > reading just 4 bytes and hitting an error - IIIUC both cases return 5. Anyone familiar with this? Seems like a bad way to signal EOF.
diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c index f7de26a1d8..f06b9a787e 100644 --- a/libavformat/id3v2.c +++ b/libavformat/id3v2.c @@ -609,10 +609,13 @@ static void read_apic(AVFormatContext *s, AVIOContext *pb, int taglen, taglen--; /* mimetype */ + memset(mimetype, 0, sizeof(mimetype)); if (isv34) { taglen -= avio_get_str(pb, taglen, mimetype, sizeof(mimetype)); } else { - avio_read(pb, mimetype, 3); + if (avio_read(pb, mimetype, 3) < 0) + goto fail; + mimetype[3] = 0; taglen -= 3; }