Commit 25db9b2c authored by Erik de Castro Lopo's avatar Erik de Castro Lopo

Improve malformed vorbiscomment handling.

A few minor tweaks on a patch from Janne Hyvärinen <cse@sci.fi>.

fixup
parent cf780068
......@@ -77,8 +77,8 @@ static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_streaminfo_c
static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_padding_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Seek seek_cb, FLAC__StreamMetadata_Padding *block, unsigned block_length);
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_entry_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_VorbisComment_Entry *entry, unsigned max_length);
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:
......@@ -2189,16 +2189,24 @@ FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_seektable_cb_(FLAC_
return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_OK;
}
FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_vorbis_comment_entry_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_VorbisComment_Entry *entry)
FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_vorbis_comment_entry_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_VorbisComment_Entry *entry, unsigned max_length)
{
const unsigned entry_length_len = FLAC__STREAM_METADATA_VORBIS_COMMENT_ENTRY_LENGTH_LEN / 8;
FLAC__byte buffer[4]; /* magic number is asserted below */
FLAC__ASSERT(FLAC__STREAM_METADATA_VORBIS_COMMENT_ENTRY_LENGTH_LEN / 8 == sizeof(buffer));
if(max_length < entry_length_len)
return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_BAD_METADATA;
max_length -= entry_length_len;
if(read_cb(buffer, 1, entry_length_len, handle) != entry_length_len)
return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_READ_ERROR;
entry->length = unpack_uint32_little_endian_(buffer, entry_length_len);
if(max_length < entry->length) {
entry->length = 0;
return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_BAD_METADATA;
} else max_length -= entry->length;
if(0 != entry->entry)
free(entry->entry);
......@@ -2219,7 +2227,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;
......@@ -2228,9 +2236,16 @@ FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_vorbis_comment_cb_(
FLAC__ASSERT(FLAC__STREAM_METADATA_VORBIS_COMMENT_NUM_COMMENTS_LEN / 8 == sizeof(buffer));
if(FLAC__METADATA_SIMPLE_ITERATOR_STATUS_OK != (status = read_metadata_block_data_vorbis_comment_entry_cb_(handle, read_cb, &(block->vendor_string))))
status = read_metadata_block_data_vorbis_comment_entry_cb_(handle, read_cb, &(block->vendor_string), block_length);
if(block_length >= 4)
block_length -= 4;
if(status == FLAC__METADATA_SIMPLE_ITERATOR_STATUS_BAD_METADATA)
goto skip;
else if(status != FLAC__METADATA_SIMPLE_ITERATOR_STATUS_OK)
return status;
block_length -= block->vendor_string.length;
if(block_length < num_comments_len) goto skip; else block_length -= num_comments_len;
if(read_cb(buffer, 1, num_comments_len, handle) != num_comments_len)
return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_READ_ERROR;
block->num_comments = unpack_uint32_little_endian_(buffer, num_comments_len);
......@@ -2242,8 +2257,21 @@ FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_vorbis_comment_cb_(
return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_MEMORY_ALLOCATION_ERROR;
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;
status = read_metadata_block_data_vorbis_comment_entry_cb_(handle, read_cb, block->comments + i, block_length);
if(block_length >= 4) block_length -= 4;
if(status == FLAC__METADATA_SIMPLE_ITERATOR_STATUS_BAD_METADATA) {
block->num_comments = i;
goto skip;
}
else if(status != FLAC__METADATA_SIMPLE_ITERATOR_STATUS_OK) return status;
block_length -= block->comments[i].length;
}
skip:
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;
......
......@@ -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,58 +1687,88 @@ 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));
/* 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 */
if(obj->vendor_string.length > 0) {
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;
}
if(!FLAC__bitreader_read_byte_block_aligned_no_crc(decoder->private_->input, obj->vendor_string.entry, obj->vendor_string.length))
if (length >= 8) {
length -= 8; /* vendor string length + num comments entries alone take 8 bytes */
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 */
obj->vendor_string.entry[obj->vendor_string.length] = '\0';
}
else
obj->vendor_string.entry = 0;
if (obj->vendor_string.length > 0) {
if (length < obj->vendor_string.length) {
obj->vendor_string.length = 0;
obj->vendor_string.entry = 0;
goto skip;
}
else
length -= obj->vendor_string.length;
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;
}
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';
}
else
obj->vendor_string.entry = 0;
/* read num comments */
FLAC__ASSERT(FLAC__STREAM_METADATA_VORBIS_COMMENT_NUM_COMMENTS_LEN == 32);
if(!FLAC__bitreader_read_uint32_little_endian(decoder->private_->input, &obj->num_comments))
return false; /* read_callback_ sets the state for us */
/* read num comments */
FLAC__ASSERT(FLAC__STREAM_METADATA_VORBIS_COMMENT_NUM_COMMENTS_LEN == 32);
if (!FLAC__bitreader_read_uint32_little_endian(decoder->private_->input, &obj->num_comments))
return false; /* read_callback_ sets the state for us */
/* read comments */
if(obj->num_comments > 0) {
if(0 == (obj->comments = safe_malloc_mul_2op_p(obj->num_comments, /*times*/sizeof(FLAC__StreamMetadata_VorbisComment_Entry)))) {
decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR;
return false;
}
for(i = 0; i < obj->num_comments; i++) {
FLAC__ASSERT(FLAC__STREAM_METADATA_VORBIS_COMMENT_ENTRY_LENGTH_LEN == 32);
if(!FLAC__bitreader_read_uint32_little_endian(decoder->private_->input, &obj->comments[i].length))
return false; /* read_callback_ sets the state for us */
if(obj->comments[i].length > 0) {
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;
/* read comments */
if (obj->num_comments > 0) {
if (0 == (obj->comments = safe_malloc_mul_2op_p(obj->num_comments, /*times*/sizeof(FLAC__StreamMetadata_VorbisComment_Entry)))) {
decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR;
return false;
}
for (i = 0; i < obj->num_comments; i++) {
FLAC__ASSERT(FLAC__STREAM_METADATA_VORBIS_COMMENT_ENTRY_LENGTH_LEN == 32);
if (length < 4) {
obj->num_comments = i;
goto skip;
}
if(!FLAC__bitreader_read_byte_block_aligned_no_crc(decoder->private_->input, obj->comments[i].entry, obj->comments[i].length))
else
length -= 4;
if (!FLAC__bitreader_read_uint32_little_endian(decoder->private_->input, &obj->comments[i].length))
return false; /* read_callback_ sets the state for us */
obj->comments[i].entry[obj->comments[i].length] = '\0';
if (obj->comments[i].length > 0) {
if (length < obj->comments[i].length) {
obj->comments[i].length = 0;
obj->comments[i].entry = 0;
obj->num_comments = i;
goto skip;
}
else
length -= obj->comments[i].length;
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;
}
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';
}
else
obj->comments[i].entry = 0;
}
else
obj->comments[i].entry = 0;
}
else
obj->comments = 0;
}
else {
obj->comments = 0;
skip:
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;
......
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