| Message ID | 20181107133443.4978-1-revol@free.fr |
|---|---|
| State | New |
| Headers | show |
On Wed, Nov 07, 2018 at 02:34:43PM +0100, François Revol wrote: > When adding thumbnails to OGG files, the line can easily go up to 100kB. > > We thus try to allocate the file size or SIZE_MAX to avoid truncation. > --- > libavformat/ffmetadec.c | 21 +++++++++++++++++---- > 1 file changed, 17 insertions(+), 4 deletions(-) > > diff --git a/libavformat/ffmetadec.c b/libavformat/ffmetadec.c > index 3290b3b7bc..ccbff51c03 100644 > --- a/libavformat/ffmetadec.c > +++ b/libavformat/ffmetadec.c > @@ -128,16 +128,26 @@ static int read_tag(const uint8_t *line, AVDictionary **m) > static int read_header(AVFormatContext *s) > { > AVDictionary **m = &s->metadata; > - uint8_t line[1024]; > + int64_t line_size = avio_size(s->pb); > + uint8_t *line; > + > + if (line_size < 1 || line_size > SIZE_MAX) > + line_size = SIZE_MAX; > + > + line = av_malloc(line_size); > + if (!line) > + return AVERROR(ENOMEM); this would use alot of memory for large files, also avio_size() will not work with all inputs using av_fast_realloc() or similar should avoid both issues > > while(!avio_feof(s->pb)) { > - get_line(s->pb, line, sizeof(line)); > + get_line(s->pb, line, line_size); > > if (!memcmp(line, ID_STREAM, strlen(ID_STREAM))) { out of memory access can happen here thx [...]
Hi, Le 08/11/2018 à 20:50, Michael Niedermayer a écrit : >> + line = av_malloc(line_size); >> + if (!line) >> + return AVERROR(ENOMEM); > > this would use alot of memory for large files, also avio_size() will not > work with all inputs Yes I thought so as well, that was a quick fix for a friend in need. > using av_fast_realloc() or similar should avoid both issues You mean, having get_line() reallocate 1kB more each time it runs out without finding a \n? Btw, I noticed there was an avio_get_line() which was quite similar, maybe those could be merged? > >> >> while(!avio_feof(s->pb)) { >> - get_line(s->pb, line, sizeof(line)); >> + get_line(s->pb, line, line_size); >> > >> if (!memcmp(line, ID_STREAM, strlen(ID_STREAM))) { > > out of memory access can happen here Right. Preallocating at least 1kB would fix it though. François.
On Thu, 8 Nov 2018, François Revol wrote: > Hi, > > Le 08/11/2018 à 20:50, Michael Niedermayer a écrit : >>> + line = av_malloc(line_size); >>> + if (!line) >>> + return AVERROR(ENOMEM); >> >> this would use alot of memory for large files, also avio_size() will not >> work with all inputs > > Yes I thought so as well, that was a quick fix for a friend in need. > >> using av_fast_realloc() or similar should avoid both issues > > You mean, having get_line() reallocate 1kB more each time it runs out > without finding a \n? You should convert everything to use an AVBprint buffer. Regards, Marton
diff --git a/libavformat/ffmetadec.c b/libavformat/ffmetadec.c index 3290b3b7bc..ccbff51c03 100644 --- a/libavformat/ffmetadec.c +++ b/libavformat/ffmetadec.c @@ -128,16 +128,26 @@ static int read_tag(const uint8_t *line, AVDictionary **m) static int read_header(AVFormatContext *s) { AVDictionary **m = &s->metadata; - uint8_t line[1024]; + int64_t line_size = avio_size(s->pb); + uint8_t *line; + + if (line_size < 1 || line_size > SIZE_MAX) + line_size = SIZE_MAX; + + line = av_malloc(line_size); + if (!line) + return AVERROR(ENOMEM); while(!avio_feof(s->pb)) { - get_line(s->pb, line, sizeof(line)); + get_line(s->pb, line, line_size); if (!memcmp(line, ID_STREAM, strlen(ID_STREAM))) { AVStream *st = avformat_new_stream(s, NULL); - if (!st) + if (!st) { + av_free(line); return AVERROR(ENOMEM); + } st->codecpar->codec_type = AVMEDIA_TYPE_DATA; st->codecpar->codec_id = AV_CODEC_ID_FFMETADATA; @@ -146,8 +156,10 @@ static int read_header(AVFormatContext *s) } else if (!memcmp(line, ID_CHAPTER, strlen(ID_CHAPTER))) { AVChapter *ch = read_chapter(s); - if (!ch) + if (!ch) { + av_free(line); return AVERROR(ENOMEM); + } m = &ch->metadata; } else @@ -160,6 +172,7 @@ static int read_header(AVFormatContext *s) s->chapters[s->nb_chapters - 1]->time_base, AV_TIME_BASE_Q); + av_free(line); return 0; }