thr3ads.net - flac dev - [flac-dev] Patch to improve malformed vorbiscomment handling [Sep 2014]

If this information is useful, please help other people find it:
Share via:

Janne Hyvärinen

2014-Sep-25 18:53 UTC

[flac-dev] Patch to improve malformed vorbiscomment handling

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;
 }

Janne Hyvärinen

2014-Sep-26 11:10 UTC

head link

[flac-dev] Patch to improve malformed vorbiscomment handling

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.

On 25.9.2014 21:53, Janne Hyv?rinen wrote:> Here's a patch to allow flac and metaflac handle files with malformed 
> vorbiscomment metadata block.
>
>
> _______________________________________________
> flac-dev mailing list
> flac-dev at xiph.org
> http://lists.xiph.org/mailman/listinfo/flac-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://lists.xiph.org/pipermail/flac-dev/attachments/20140926/54a4a32b/attachment-0001.htm
-------------- next part --------------
diff --git a/src/libFLAC/metadata_iterators.c b/src/libFLAC/metadata_iterators.c
index d50df39..cba864c 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_CHAIN_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_CHAIN_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_CHAIN_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_CHAIN_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 fac73f3..3c68fd8 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,23 @@ 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) goto skip; else length -= 8; /* 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 */
 	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;
@@ -1722,9 +1728,19 @@ 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) {
+				obj->num_comments = i;
+				goto skip;
+			} 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 */
 			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;
@@ -1741,6 +1757,13 @@ FLAC__bool
read_metadata_vorbiscomment_(FLAC__StreamDecoder *decoder, FLAC__Stre
 		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;
 }

Erik de Castro Lopo

2014-Sep-26 12:21 UTC

head link

[flac-dev] Patch to improve malformed vorbiscomment handling

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
-- 
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/

Possibly Parallel Threads

Search for more seemingly similar threads

flac dev - Sep 2014 - Patch to improve malformed vorbiscomment handling

[flac-dev] Patch to improve malformed vorbiscomment handling

[flac-dev] Patch to improve malformed vorbiscomment handling

[flac-dev] Patch to improve malformed vorbiscomment handling

Possibly Parallel Threads