MrBesen
/
ffmpeg
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

avformat/oggparsevorbis: Avoid tmp bufs when parsing VorbisComment 

A single VorbisComment consists of a length field and a
non-NUL-terminated string of the form "key=value". Up until now,
when parsing such a VorbisComment, zero-terminated duplicates of
key and value would be created. This is wasteful if these duplicates
are freed shortly afterwards, as happens in particular in case of
attached pictures: In this case value is base64 encoded and only
needed to decode the actual data.

Therefore this commit changes this: The buffer is temporarily modified
so that both key and value are zero-terminated. Then the data is used
in-place and restored to its original state afterwards.

This requires that the buffer has at least one byte of padding. All
buffers currently have AV_INPUT_BUFFER_PADDING_SIZE bytes padding,
so this is ok.

Finally, this also fixes weird behaviour from ogm_chapter():
It sometimes freed given to it, leaving the caller with dangling
pointers.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
This commit is contained in:
Andreas Rheinhardt 2021-08-23 11:24:20 +02:00
parent f1d89d6dd0
commit b10a8a30db
2 changed files with 39 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
libavformat/oggdec.h
View File

@ -130,9 +130,25 @@ extern const struct ogg_codec ff_theora_codec;
extern const struct ogg_codec ff_vorbis_codec;
extern const struct ogg_codec ff_vp8_codec;
/**
* Parse Vorbis comments
*
* @note The buffer will be temporarily modifed by this function,
* so it needs to be writable. Furthermore it must be padded
* by a single byte (not counted in size).
* All changes will have been reverted upon return.
*/
int ff_vorbis_comment(AVFormatContext *ms, AVDictionary **m,
const uint8_t *buf, int size, int parse_picture);
/**
* Parse Vorbis comments and add metadata to an AVStream
*
* @note The buffer will be temporarily modifed by this function,
* so it needs to be writable. Furthermore it must be padded
* by a single byte (not counted in size).
* All changes will have been reverted upon return.
*/
int ff_vorbis_stream_comment(AVFormatContext *as, AVStream *st,
const uint8_t *buf, int size);

56
libavformat/oggparsevorbis.c
View File

@ -39,7 +39,7 @@
#include "vorbiscomment.h"
#include "replaygain.h"
static int ogm_chapter(AVFormatContext *as, uint8_t *key, uint8_t *val)
static int ogm_chapter(AVFormatContext *as, const uint8_t *key, const uint8_t *val)
{
int i, cnum, h, m, s, ms, keylen = strlen(key);
AVChapter *chapter = NULL;
@ -54,7 +54,6 @@ static int ogm_chapter(AVFormatContext *as, uint8_t *key, uint8_t *val)
avpriv_new_chapter(as, cnum, (AVRational) { 1, 1000 },
ms + 1000 * (s + 60 * (m + 60 * h)),
AV_NOPTS_VALUE, NULL);
av_free(val);
} else if (!av_strcasecmp(key + keylen - 4, "NAME")) {
for (i = 0; i < as->nb_chapters; i++)
if (as->chapters[i]->id == cnum) {
@ -64,11 +63,10 @@ static int ogm_chapter(AVFormatContext *as, uint8_t *key, uint8_t *val)
if (!chapter)
return 0;
av_dict_set(&chapter->metadata, "title", val, AV_DICT_DONT_STRDUP_VAL);
av_dict_set(&chapter->metadata, "title", val, 0);
} else
return 0;
av_free(key);
return 1;
}
@ -84,13 +82,18 @@ int ff_vorbis_stream_comment(AVFormatContext *as, AVStream *st,
return updates;
}
/**
* This function temporarily modifies the (const qualified) input buffer
* and reverts its changes before return. The input buffer needs to have
* at least one byte of padding.
*/
static int vorbis_parse_single_comment(AVFormatContext *as, AVDictionary **m,
const uint8_t *buf, uint32_t size,
int *updates, int parse_picture)
{
const char *t = buf, *v = memchr(t, '=', size);
char *tt, *ct;
char *t = (char*)buf, *v = memchr(t, '=', size);
int tl, vl;
char backup;
if (!v)
return 0;
@ -102,19 +105,10 @@ static int vorbis_parse_single_comment(AVFormatContext *as, AVDictionary **m,
if (!tl || !vl)
return 0;
tt = av_malloc(tl + 1);
ct = av_malloc(vl + 1);
if (!tt || !ct) {
av_freep(&tt);
av_freep(&ct);
return AVERROR(ENOMEM);
}
t[tl] = 0;
memcpy(tt, t, tl);
tt[tl] = 0;
memcpy(ct, v, vl);
ct[vl] = 0;
backup = v[vl];
v[vl] = 0;
/* The format in which the pictures are stored is the FLAC format.
* Xiph says: "The binary FLAC picture structure is base64 encoded
@ -122,35 +116,31 @@ static int vorbis_parse_single_comment(AVFormatContext *as, AVDictionary **m,
* 'METADATA_BLOCK_PICTURE'. This is the preferred and
* recommended way of embedding cover art within VorbisComments."
*/
if (!av_strcasecmp(tt, "METADATA_BLOCK_PICTURE") && parse_picture) {
if (!av_strcasecmp(t, "METADATA_BLOCK_PICTURE") && parse_picture) {
int ret, len = AV_BASE64_DECODE_SIZE(vl);
uint8_t *pict = av_malloc(len);
if (!pict) {
av_log(as, AV_LOG_WARNING, "out-of-memory error. Skipping cover art block.\n");
av_freep(&tt);
av_freep(&ct);
return 0;
goto end;
}
ret = av_base64_decode(pict, ct, len);
av_freep(&tt);
av_freep(&ct);
ret = av_base64_decode(pict, v, len);
if (ret > 0)
ret = ff_flac_parse_picture(as, pict, ret, 0);
av_freep(&pict);
if (ret < 0) {
av_log(as, AV_LOG_WARNING, "Failed to parse cover art block.\n");
return 0;
goto end;
}
} else if (!ogm_chapter(as, tt, ct)) {
} else if (!ogm_chapter(as, t, v)) {
(*updates)++;
if (av_dict_get(*m, tt, NULL, 0)) {
av_dict_set(m, tt, ";", AV_DICT_APPEND);
}
av_dict_set(m, tt, ct,
AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL |
AV_DICT_APPEND);
if (av_dict_get(*m, t, NULL, 0))
av_dict_set(m, t, ";", AV_DICT_APPEND);
av_dict_set(m, t, v, AV_DICT_APPEND);
}
end:
t[tl] = '=';
v[vl] = backup;
return 0;
}