diff options
author | Erik de Castro Lopo <erikd@mega-nerd.com> | 2014-09-27 08:52:27 +1000 |
---|---|---|
committer | Erik de Castro Lopo <erikd@mega-nerd.com> | 2014-09-27 09:28:45 +1000 |
commit | 25db9b2cce84db62a4689e7e69d5ed14b517da7a (patch) | |
tree | 33f7904507f66c6ac9421670e87b0e53022d0996 | |
parent | cf780068dec390ec0a80a100cac50bf47a41a85b (diff) | |
download | flac-25db9b2cce84db62a4689e7e69d5ed14b517da7a.tar.gz |
Improve malformed vorbiscomment handling.
A few minor tweaks on a patch from Janne Hyvärinen <cse@sci.fi>.
fixup
-rw-r--r-- | src/libFLAC/metadata_iterators.c | 44 | ||||
-rw-r--r-- | src/libFLAC/stream_decoder.c | 110 |
2 files changed, 106 insertions, 48 deletions
diff --git a/src/libFLAC/metadata_iterators.c b/src/libFLAC/metadata_iterators.c index d50df39e..b0f63788 100644 --- a/src/libFLAC/metadata_iterators.c +++ b/src/libFLAC/metadata_iterators.c @@ -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; diff --git a/src/libFLAC/stream_decoder.c b/src/libFLAC/stream_decoder.c index fac73f36..cb66fe25 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,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; |