On 26.9.2014 15:21, Erik de Castro Lopo wrote:
Janne Hyvärinen wrote:
Patch v2, now handles more malformed cases. Original patch was for a
file for which I had a sample from a user but this allows handling some
manually broken test cases.
Err, I'm getting warning messages on that patch:
CC metadata_iterators.lo
metadata_iterators.c: In function ‘read_metadata_block_data_vorbis_comment_cb_’:
metadata_iterators.c:2238:12: error: comparison between
‘FLAC__Metadata_SimpleIteratorStatus’ and ‘enum <anonymous>’
[-Werror=enum-compare]
if(status == FLAC__METADATA_CHAIN_STATUS_BAD_METADATA) goto skip;
^
metadata_iterators.c:2256:13: error: comparison between
‘FLAC__Metadata_SimpleIteratorStatus’ and ‘enum <anonymous>’
[-Werror=enum-compare]
if(status == FLAC__METADATA_CHAIN_STATUS_BAD_METADATA) {
^
Erik
Darn, MSVC didn't warn anything and I didn't realize there are so many
BAD_METADATA statuses. I picked the first one autocomplete offered.
One thing I don't like is that these don't return any warning messages
to tools. But I didn't see a way for the library to do that. I see only
fatal errors.
diff --git a/src/libFLAC/metadata_iterators.c b/src/libFLAC/metadata_iterators.c
index d50df39..52ce7de 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,21 @@ 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; else 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 +2224,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 +2233,13 @@ 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))))
- return status;
+ 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 +2251,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;
_______________________________________________
flac-dev mailing list
[email protected]
http://lists.xiph.org/mailman/listinfo/flac-dev
