Commit af97c986 authored by Mattias Wadman's avatar Mattias Wadman Committed by Anton Khirnov

libavformat/flacdec: Workaround for truncated metadata picture size

Some flac muxers write truncated metadata picture size if the picture
data do not fit in 24 bits. Detect this by truncting the size found inside
the picture block and if it matches the block size use it and read rest
of picture data.

This workaround is only for flac files and not ogg files with flac
METADATA_BLOCK_PICTURE comments and it can be disabled with strict level
above normal. Currently there is a 500MB limit on truncate size to protect
from large memory allocations.

The truncation bug in lavf flacenc was fixed in e447a4d1
but based on existing broken files other unknown flac muxers seems to truncate also.
Before the fix a broken flac file for reproduction could be generated with:
ffmpeg -f lavfi -i sine -f lavfi -i color=red:size=2400x2400 -map 0:0 -map 1:0 -c:v:0 bmp -disposition:1 attached_pic -t 1 test.flac

Fixes ticket 6333
Signed-off-by: 's avatarAnton Khirnov <anton@khirnov.net>
parent ea980d41
...@@ -27,7 +27,9 @@ ...@@ -27,7 +27,9 @@
#include "id3v2.h" #include "id3v2.h"
#include "internal.h" #include "internal.h"
int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size) #define MAX_TRUNC_PICTURE_SIZE (500 * 1024 * 1024)
int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size, int truncate_workaround)
{ {
const CodecMime *mime = ff_id3v2_mime_tags; const CodecMime *mime = ff_id3v2_mime_tags;
enum AVCodecID id = AV_CODEC_ID_NONE; enum AVCodecID id = AV_CODEC_ID_NONE;
...@@ -36,7 +38,8 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size) ...@@ -36,7 +38,8 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size)
GetByteContext g; GetByteContext g;
AVStream *st; AVStream *st;
int width, height, ret = 0; int width, height, ret = 0;
unsigned int len, type; unsigned int type;
uint32_t len, left, trunclen = 0;
if (buf_size < 34) { if (buf_size < 34) {
av_log(s, AV_LOG_ERROR, "Attached picture metadata block too short\n"); av_log(s, AV_LOG_ERROR, "Attached picture metadata block too short\n");
...@@ -114,16 +117,44 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size) ...@@ -114,16 +117,44 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size)
/* picture data */ /* picture data */
len = bytestream2_get_be32u(&g); len = bytestream2_get_be32u(&g);
if (len <= 0 || len > bytestream2_get_bytes_left(&g)) {
av_log(s, AV_LOG_ERROR, "Attached picture metadata block too short\n"); left = bytestream2_get_bytes_left(&g);
if (s->error_recognition & AV_EF_EXPLODE) if (len <= 0 || len > left) {
ret = AVERROR_INVALIDDATA; if (len > MAX_TRUNC_PICTURE_SIZE || len >= INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE) {
goto fail; av_log(s, AV_LOG_ERROR, "Attached picture metadata block too big %u\n", len);
if (s->error_recognition & AV_EF_EXPLODE)
ret = AVERROR_INVALIDDATA;
goto fail;
}
// Workaround bug for flac muxers that writs truncated metadata picture block size if
// the picture size do not fit in 24 bits. lavf flacenc used to have the issue and based
// on existing broken files other unknown flac muxers seems to truncate also.
if (truncate_workaround &&
s->strict_std_compliance <= FF_COMPLIANCE_NORMAL &&
len > left && (len & 0xffffff) == left) {
av_log(s, AV_LOG_INFO, "Correcting truncated metadata picture size from %u to %u\n", left, len);
trunclen = len - left;
} else {
av_log(s, AV_LOG_ERROR, "Attached picture metadata block too short\n");
if (s->error_recognition & AV_EF_EXPLODE)
ret = AVERROR_INVALIDDATA;
goto fail;
}
} }
if (!(data = av_buffer_alloc(len + AV_INPUT_BUFFER_PADDING_SIZE))) { if (!(data = av_buffer_alloc(len + AV_INPUT_BUFFER_PADDING_SIZE))) {
RETURN_ERROR(AVERROR(ENOMEM)); RETURN_ERROR(AVERROR(ENOMEM));
} }
bytestream2_get_bufferu(&g, data->data, len);
if (trunclen == 0) {
bytestream2_get_bufferu(&g, data->data, len);
} else {
// If truncation was detected copy all data from block and read missing bytes
// not included in the block size
bytestream2_get_bufferu(&g, data->data, left);
if (avio_read(s->pb, data->data + len - trunclen, trunclen) < trunclen)
RETURN_ERROR(AVERROR_INVALIDDATA);
}
memset(data->data + len, 0, AV_INPUT_BUFFER_PADDING_SIZE); memset(data->data + len, 0, AV_INPUT_BUFFER_PADDING_SIZE);
if (AV_RB64(data->data) == PNGSIG) if (AV_RB64(data->data) == PNGSIG)
......
...@@ -26,6 +26,6 @@ ...@@ -26,6 +26,6 @@
#define RETURN_ERROR(code) do { ret = (code); goto fail; } while (0) #define RETURN_ERROR(code) do { ret = (code); goto fail; } while (0)
int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size); int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size, int truncate_workaround);
#endif /* AVFORMAT_FLAC_PICTURE_H */ #endif /* AVFORMAT_FLAC_PICTURE_H */
...@@ -146,7 +146,7 @@ static int flac_read_header(AVFormatContext *s) ...@@ -146,7 +146,7 @@ static int flac_read_header(AVFormatContext *s)
} }
av_freep(&buffer); av_freep(&buffer);
} else if (metadata_type == FLAC_METADATA_TYPE_PICTURE) { } else if (metadata_type == FLAC_METADATA_TYPE_PICTURE) {
ret = ff_flac_parse_picture(s, buffer, metadata_size); ret = ff_flac_parse_picture(s, buffer, metadata_size, 1);
av_freep(&buffer); av_freep(&buffer);
if (ret < 0) { if (ret < 0) {
av_log(s, AV_LOG_ERROR, "Error parsing attached picture.\n"); av_log(s, AV_LOG_ERROR, "Error parsing attached picture.\n");
......
...@@ -165,7 +165,7 @@ int ff_vorbis_comment(AVFormatContext *as, AVDictionary **m, ...@@ -165,7 +165,7 @@ int ff_vorbis_comment(AVFormatContext *as, AVDictionary **m,
av_freep(&tt); av_freep(&tt);
av_freep(&ct); av_freep(&ct);
if (ret > 0) if (ret > 0)
ret = ff_flac_parse_picture(as, pict, ret); ret = ff_flac_parse_picture(as, pict, ret, 0);
av_freep(&pict); av_freep(&pict);
if (ret < 0) { if (ret < 0) {
av_log(as, AV_LOG_WARNING, "Failed to parse cover art block.\n"); av_log(as, AV_LOG_WARNING, "Failed to parse cover art block.\n");
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment