[flac-dev] Patch to improve malformed vorbiscomment handling
Janne Hyvärinen
cse at sci.fi
Thu Sep 25 11:53:34 PDT 2014
Here's a patch to allow flac and metaflac handle files with malformed
vorbiscomment metadata block.
-------------- next part --------------
diff --git a/src/libFLAC/metadata_iterators.c b/src/libFLAC/metadata_iterators.c
index d50df39..39cb276 100644
--- a/src/libFLAC/metadata_iterators.c
+++ b/src/libFLAC/metadata_iterators.c
@@ -78,7 +78,7 @@ static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_padding_cb_(
static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_application_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_Application *block, unsigned block_length);
static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_seektable_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_SeekTable *block, unsigned block_length);
static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_vorbis_comment_entry_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_VorbisComment_Entry *entry);
-static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_vorbis_comment_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_VorbisComment *block);
+static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_vorbis_comment_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__IOCallback_Seek seek_cb, FLAC__StreamMetadata_VorbisComment *block, unsigned block_length);
static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_cuesheet_track_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_CueSheet_Track *track);
static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_cuesheet_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_CueSheet *block);
static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_picture_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_Picture *block);
@@ -2091,7 +2091,7 @@ FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_cb_(FLAC__IOHandle
case FLAC__METADATA_TYPE_SEEKTABLE:
return read_metadata_block_data_seektable_cb_(handle, read_cb, &block->data.seek_table, block->length);
case FLAC__METADATA_TYPE_VORBIS_COMMENT:
- return read_metadata_block_data_vorbis_comment_cb_(handle, read_cb, &block->data.vorbis_comment);
+ return read_metadata_block_data_vorbis_comment_cb_(handle, read_cb, seek_cb, &block->data.vorbis_comment, block->length);
case FLAC__METADATA_TYPE_CUESHEET:
return read_metadata_block_data_cuesheet_cb_(handle, read_cb, &block->data.cue_sheet);
case FLAC__METADATA_TYPE_PICTURE:
@@ -2219,7 +2219,7 @@ FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_vorbis_comment_entr
return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_OK;
}
-FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_vorbis_comment_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_VorbisComment *block)
+FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_vorbis_comment_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__IOCallback_Seek seek_cb, FLAC__StreamMetadata_VorbisComment *block, unsigned block_length)
{
unsigned i;
FLAC__Metadata_SimpleIteratorStatus status;
@@ -2241,9 +2241,17 @@ FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_vorbis_comment_cb_(
else if(0 == (block->comments = calloc(block->num_comments, sizeof(FLAC__StreamMetadata_VorbisComment_Entry))))
return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_MEMORY_ALLOCATION_ERROR;
+ block_length -= 8+block->vendor_string.length;
for(i = 0; i < block->num_comments; i++) {
if(FLAC__METADATA_SIMPLE_ITERATOR_STATUS_OK != (status = read_metadata_block_data_vorbis_comment_entry_cb_(handle, read_cb, block->comments + i)))
return status;
+ block_length -= 4+block->comments[i].length;
+ }
+
+ if(block_length > 0) {
+ /* bad metadata */
+ if(0 != seek_cb(handle, block_length, SEEK_CUR))
+ return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_SEEK_ERROR;
}
return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_OK;
diff --git a/src/libFLAC/stream_decoder.c b/src/libFLAC/stream_decoder.c
index fac73f3..604f6ea 100644
--- a/src/libFLAC/stream_decoder.c
+++ b/src/libFLAC/stream_decoder.c
@@ -87,7 +87,7 @@ static FLAC__bool find_metadata_(FLAC__StreamDecoder *decoder);
static FLAC__bool read_metadata_(FLAC__StreamDecoder *decoder);
static FLAC__bool read_metadata_streaminfo_(FLAC__StreamDecoder *decoder, FLAC__bool is_last, unsigned length);
static FLAC__bool read_metadata_seektable_(FLAC__StreamDecoder *decoder, FLAC__bool is_last, unsigned length);
-static FLAC__bool read_metadata_vorbiscomment_(FLAC__StreamDecoder *decoder, FLAC__StreamMetadata_VorbisComment *obj);
+static FLAC__bool read_metadata_vorbiscomment_(FLAC__StreamDecoder *decoder, FLAC__StreamMetadata_VorbisComment *obj, unsigned length);
static FLAC__bool read_metadata_cuesheet_(FLAC__StreamDecoder *decoder, FLAC__StreamMetadata_CueSheet *obj);
static FLAC__bool read_metadata_picture_(FLAC__StreamDecoder *decoder, FLAC__StreamMetadata_Picture *obj);
static FLAC__bool skip_id3v2_tag_(FLAC__StreamDecoder *decoder);
@@ -1485,7 +1485,7 @@ FLAC__bool read_metadata_(FLAC__StreamDecoder *decoder)
block.data.application.data = 0;
break;
case FLAC__METADATA_TYPE_VORBIS_COMMENT:
- if(!read_metadata_vorbiscomment_(decoder, &block.data.vorbis_comment))
+ if(!read_metadata_vorbiscomment_(decoder, &block.data.vorbis_comment, real_length))
ok = false;
break;
case FLAC__METADATA_TYPE_CUESHEET:
@@ -1687,17 +1687,20 @@ FLAC__bool read_metadata_seektable_(FLAC__StreamDecoder *decoder, FLAC__bool is_
return true;
}
-FLAC__bool read_metadata_vorbiscomment_(FLAC__StreamDecoder *decoder, FLAC__StreamMetadata_VorbisComment *obj)
+FLAC__bool read_metadata_vorbiscomment_(FLAC__StreamDecoder *decoder, FLAC__StreamMetadata_VorbisComment *obj, unsigned length)
{
FLAC__uint32 i;
FLAC__ASSERT(FLAC__bitreader_is_consumed_byte_aligned(decoder->private_->input));
+ if (length < 8) return false; /* vendor string length + num comments entries alone take 8 bytes */
/* read vendor string */
FLAC__ASSERT(FLAC__STREAM_METADATA_VORBIS_COMMENT_ENTRY_LENGTH_LEN == 32);
if(!FLAC__bitreader_read_uint32_little_endian(decoder->private_->input, &obj->vendor_string.length))
return false; /* read_callback_ sets the state for us */
+ length -= 8;
if(obj->vendor_string.length > 0) {
+ if (length < obj->vendor_string.length) return false;
if(0 == (obj->vendor_string.entry = safe_malloc_add_2op_(obj->vendor_string.length, /*+*/1))) {
decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR;
return false;
@@ -1705,6 +1708,7 @@ FLAC__bool read_metadata_vorbiscomment_(FLAC__StreamDecoder *decoder, FLAC__Stre
if(!FLAC__bitreader_read_byte_block_aligned_no_crc(decoder->private_->input, obj->vendor_string.entry, obj->vendor_string.length))
return false; /* read_callback_ sets the state for us */
obj->vendor_string.entry[obj->vendor_string.length] = '\0';
+ length -= obj->vendor_string.length;
}
else
obj->vendor_string.entry = 0;
@@ -1722,9 +1726,12 @@ FLAC__bool read_metadata_vorbiscomment_(FLAC__StreamDecoder *decoder, FLAC__Stre
}
for(i = 0; i < obj->num_comments; i++) {
FLAC__ASSERT(FLAC__STREAM_METADATA_VORBIS_COMMENT_ENTRY_LENGTH_LEN == 32);
+ if (length < 4) return false;
if(!FLAC__bitreader_read_uint32_little_endian(decoder->private_->input, &obj->comments[i].length))
return false; /* read_callback_ sets the state for us */
+ length -= 4;
if(obj->comments[i].length > 0) {
+ if (length < obj->comments[i].length) return false;
if(0 == (obj->comments[i].entry = safe_malloc_add_2op_(obj->comments[i].length, /*+*/1))) {
decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR;
return false;
@@ -1732,6 +1739,7 @@ FLAC__bool read_metadata_vorbiscomment_(FLAC__StreamDecoder *decoder, FLAC__Stre
if(!FLAC__bitreader_read_byte_block_aligned_no_crc(decoder->private_->input, obj->comments[i].entry, obj->comments[i].length))
return false; /* read_callback_ sets the state for us */
obj->comments[i].entry[obj->comments[i].length] = '\0';
+ length -= obj->comments[i].length;
}
else
obj->comments[i].entry = 0;
@@ -1741,6 +1749,12 @@ FLAC__bool read_metadata_vorbiscomment_(FLAC__StreamDecoder *decoder, FLAC__Stre
obj->comments = 0;
}
+ if(length > 0) {
+ /* This will only happen on files with invalid data in comments */
+ if(!FLAC__bitreader_skip_byte_block_aligned_no_crc(decoder->private_->input, length))
+ return false; /* read_callback_ sets the state for us */
+ }
+
return true;
}
More information about the flac-dev
mailing list