NULL is returned in FLAC__StreamMetadata_VorbisComment_Entry->entry

[louiz]

See for more context: MusicPlayerDaemon/MPD#108 (comment)

When decoding some FLAC (I guess, corrupted) file, libflac sets the entry pointer as NULL, while the documentation (https://xiph.org/flac/api/structFLAC____StreamMetadata__VorbisComment__Entry.html) doesn’t say it can.

This causes client applications (here MPD) to segfault, because it doesn’t expect it to be NULL.

Let me know if you want me to send you a file by e-mail that reproduces this issue.

[erikd]

Would you be able to upload an example file here? Or somewhere else and provide a link?

Also, can the flac command line program decode this file correctly?

[louiz]

I just sent you an e-mail with an attached file.

Also, can the flac command line program decode this file correctly?

Apparently not, I get

$ flac -d Disc\ 2\ -\ 10\ -\ March\ of\ the\ Machines.flac
Disc 2 - 10 - March of the Machines.flac: *** Got error code 0:FLAC__STREAM_DECODER_ERROR_STATUS_LOST_SYNC
                                                                                                          
                                                                                                          
Disc 2 - 10 - March of the Machines.flac: ERROR while decoding data                                       
                                          state = FLAC__STREAM_DECODER_ABORTED                            

Also with mpv:

[ffmpeg/audio] flac: invalid sync code    
[ffmpeg/audio] flac: invalid frame header 
[ffmpeg/audio] flac: decode_frame() failed
Error decoding audio.                     

(but the audio still plays fine)

So, there’s apparently a bug in the encoder I used to rip (and then FLAC-encode) that song. But the flac API should still (I think) correctly report the error to the user.

[erikd]

Got the file. Thanks. Will have a look some time soon.

[ktmf01]

@louiz Do you still have that file? Erik is no longer actively involved with this project, and I would like to try and fix this.

[louiz]

I’m sorry, it’s been 4 years and I can’t find this file (I have it in mp3, but not in FLAC, for some reason).
Can’t find the e-mail either.

[ktmf01]

Ok, no problem, thanks for checking. I can make sure this behavior is eliminated through the metadata fuzzer (as soon as it works correctly), that just takes a little longer.

[ktmf01]

It seems the problem might be here:

flac/src/libFLAC/metadata_iterators.c

Lines 2322 to 2333 in 5186025

	if(entry->length == 0) {
	entry->entry = 0;
	}
	else {
	if(0 == (entry->entry = safe_malloc_add_2op_(entry->length, /*+*/1)))
	return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_MEMORY_ALLOCATION_ERROR;
	if(read_cb(entry->entry, 1, entry->length, handle) != entry->length)
	return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_READ_ERROR;
	entry->entry[entry->length] = '\0';
	}

This might be fixed by simply returning an entry with an empty string instead of nothing at all. It seems this was an oversight in commit def597e