Rework error handling (#283)

This commit reworks the code decoding a frame, to add silence when
frames are missing and output silence when something other than the
frame header seems corrupted. Tests are added to the test suite for
this functionality. Also, decoded values are checked to be within bps
This commit is contained in:
Martijn van Beurden 2022-04-27 12:16:15 +02:00 committed by GitHub
parent 7e785eb9a8
commit 1793632ee6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 252 additions and 43 deletions

View File

@ -113,6 +113,7 @@ struct FLAC__BitReader {
uint32_t crc16_align; /* the number of bits in the current consumed word that should not be CRC'd */
FLAC__bool read_limit_set; /* whether reads are limited */
uint32_t read_limit; /* the remaining size of what can be read */
uint32_t last_seen_framesync; /* the location of the last seen framesync, if it is in the buffer, in bits from front of buffer */
FLAC__BitReaderReadCallback read_callback;
void *client_data;
};
@ -158,6 +159,9 @@ static FLAC__bool bitreader_read_from_client_(FLAC__BitReader *br)
size_t bytes;
FLAC__byte *target;
/* invalidate last seen framesync */
br->last_seen_framesync = -1;
/* first shift the unconsumed buffer data toward the front as much as possible */
if(br->consumed_words > 0) {
crc16_update_block_(br); /* CRC consumed words */
@ -279,6 +283,7 @@ FLAC__bool FLAC__bitreader_init(FLAC__BitReader *br, FLAC__BitReaderReadCallback
br->client_data = cd;
br->read_limit_set = false;
br->read_limit = -1;
br->last_seen_framesync = -1;
return true;
}
@ -297,6 +302,7 @@ void FLAC__bitreader_free(FLAC__BitReader *br)
br->client_data = 0;
br->read_limit_set = false;
br->read_limit = -1;
br->last_seen_framesync = -1;
}
FLAC__bool FLAC__bitreader_clear(FLAC__BitReader *br)
@ -305,9 +311,28 @@ FLAC__bool FLAC__bitreader_clear(FLAC__BitReader *br)
br->consumed_words = br->consumed_bits = 0;
br->read_limit_set = false;
br->read_limit = -1;
br->last_seen_framesync = -1;
return true;
}
void FLAC__bitreader_set_framesync_location(FLAC__BitReader *br)
{
br->last_seen_framesync = br->consumed_words * FLAC__BYTES_PER_WORD + br->consumed_bits / 8;
}
FLAC__bool FLAC__bitreader_rewind_to_after_last_seen_framesync(FLAC__BitReader *br)
{
if(br->last_seen_framesync == (uint32_t)-1) {
br->consumed_words = br->consumed_bits = 0;
return false;
}
else {
br->consumed_words = (br->last_seen_framesync + 1) / FLAC__BYTES_PER_WORD;
br->consumed_bits = ((br->last_seen_framesync + 1) % FLAC__BYTES_PER_WORD) * 8;
return true;
}
}
void FLAC__bitreader_dump(const FLAC__BitReader *br, FILE *out)
{
uint32_t i, j;

View File

@ -53,6 +53,8 @@ void FLAC__bitreader_delete(FLAC__BitReader *br);
FLAC__bool FLAC__bitreader_init(FLAC__BitReader *br, FLAC__BitReaderReadCallback rcb, void *cd);
void FLAC__bitreader_free(FLAC__BitReader *br); /* does not 'free(br)' */
FLAC__bool FLAC__bitreader_clear(FLAC__BitReader *br);
void FLAC__bitreader_set_framesync_location(FLAC__BitReader *br);
FLAC__bool FLAC__bitreader_rewind_to_after_last_seen_framesync(FLAC__BitReader *br);
void FLAC__bitreader_dump(const FLAC__BitReader *br, FILE *out);
/*

View File

@ -162,8 +162,10 @@ typedef struct FLAC__StreamDecoderPrivate {
FLAC__MD5Context md5context;
FLAC__byte computed_md5sum[16]; /* this is the sum we computed from the decoded data */
/* (the rest of these are only used for seeking) */
FLAC__Frame last_frame; /* holds the info of the last frame we seeked to */
FLAC__Frame last_frame; /* holds the info of the last frame we decoded or seeked to */
FLAC__bool last_frame_is_set;
FLAC__uint64 first_frame_offset; /* hint to the seek routine of where in the stream the first audio frame starts */
FLAC__uint64 last_seen_framesync; /* if tell callback works, the location of the last seen frame sync code, to rewind to if needed */
FLAC__uint64 target_sample;
uint32_t unparseable_frame_count; /* used to tell whether we're decoding a future version of FLAC or just got a bad sync */
FLAC__bool got_a_frame; /* hack needed in Ogg FLAC seek routine to check when process_single() actually writes a frame */
@ -998,6 +1000,8 @@ FLAC_API FLAC__bool FLAC__stream_decoder_reset(FLAC__StreamDecoder *decoder)
decoder->private_->first_frame_offset = 0;
decoder->private_->unparseable_frame_count = 0;
decoder->private_->last_seen_framesync = 0;
decoder->private_->last_frame_is_set = false;
return true;
}
@ -2015,6 +2019,12 @@ FLAC__bool frame_sync_(FLAC__StreamDecoder *decoder)
else if(x >> 1 == 0x7c) { /* MAGIC NUMBER for the last 6 sync bits and reserved 7th bit */
decoder->private_->header_warmup[1] = (FLAC__byte)x;
decoder->protected_->state = FLAC__STREAM_DECODER_READ_FRAME;
/* Save location so we can rewind in case the frame turns
* out to be invalid after the header */
FLAC__bitreader_set_framesync_location(decoder->private_->input);
if(!FLAC__stream_decoder_get_decode_position(decoder, &decoder->private_->last_seen_framesync))
decoder->private_->last_seen_framesync = 0;
return true;
}
}
@ -2079,26 +2089,32 @@ FLAC__bool read_frame_(FLAC__StreamDecoder *decoder, FLAC__bool *got_a_frame, FL
/*
* now read it
*/
if(!read_subframe_(decoder, channel, bps, do_full_decode))
return false;
if(decoder->protected_->state == FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC) /* means bad sync or got corruption */
return true;
if(!read_subframe_(decoder, channel, bps, do_full_decode)){
/* read_callback_ sets the state for us */
if(decoder->protected_->state == FLAC__STREAM_DECODER_END_OF_STREAM)
break;
else
return false;
}
}
if(!read_zero_padding_(decoder))
return false;
if(decoder->protected_->state == FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC) /* means bad sync or got corruption (i.e. "zero bits" were not all zeroes) */
return true;
if(decoder->protected_->state != FLAC__STREAM_DECODER_END_OF_STREAM)
if(!read_zero_padding_(decoder))
return false;
/*
* Read the frame CRC-16 from the footer and check
*/
frame_crc = FLAC__bitreader_get_read_crc16(decoder->private_->input);
if(!FLAC__bitreader_read_raw_uint32(decoder->private_->input, &x, FLAC__FRAME_FOOTER_CRC_LEN))
return false; /* read_callback_ sets the state for us */
#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
if(1){
#else
if(frame_crc == x) {
if(decoder->protected_->state == FLAC__STREAM_DECODER_READ_FRAME) {
frame_crc = FLAC__bitreader_get_read_crc16(decoder->private_->input);
if(!FLAC__bitreader_read_raw_uint32(decoder->private_->input, &x, FLAC__FRAME_FOOTER_CRC_LEN)) {
/* read_callback_ sets the state for us */
if(decoder->protected_->state != FLAC__STREAM_DECODER_END_OF_STREAM)
return false;
}
#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
}
if(decoder->protected_->state == FLAC__STREAM_DECODER_READ_FRAME && frame_crc == x) {
#endif
if(do_full_decode) {
/* Undo any special channel coding */
@ -2138,39 +2154,133 @@ FLAC__bool read_frame_(FLAC__StreamDecoder *decoder, FLAC__bool *got_a_frame, FL
FLAC__ASSERT(0);
break;
}
/* Check whether decoded data actually fits bps */
for(channel = 0; channel < decoder->private_->frame.header.channels; channel++) {
for(i = 0; i < decoder->private_->frame.header.blocksize; i++) {
int shift_bits = 32 - decoder->private_->frame.header.bits_per_sample;
/* Check whether shift_bits MSBs are 'empty' by shifting up and down */
if((decoder->private_->output[channel][i] < (INT32_MIN >> shift_bits)) ||
(decoder->private_->output[channel][i] > (INT32_MAX >> shift_bits))) {
/* Bad frame, emit error */
send_error_to_client_(decoder, FLAC__STREAM_DECODER_ERROR_STATUS_FRAME_CRC_MISMATCH);
decoder->protected_->state = FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC;
break;
}
}
}
}
}
else {
/* Bad frame, emit error and zero the output signal */
#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
else if (decoder->protected_->state == FLAC__STREAM_DECODER_READ_FRAME) {
/* Bad frame, emit error */
send_error_to_client_(decoder, FLAC__STREAM_DECODER_ERROR_STATUS_FRAME_CRC_MISMATCH);
if(do_full_decode) {
for(channel = 0; channel < decoder->private_->frame.header.channels; channel++) {
memset(decoder->private_->output[channel], 0, sizeof(FLAC__int32) * decoder->private_->frame.header.blocksize);
decoder->protected_->state = FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC;
}
#endif
/* Check whether frames are missing, if so, add silence to compensate */
if(decoder->private_->last_frame_is_set && decoder->protected_->state == FLAC__STREAM_DECODER_READ_FRAME && !decoder->private_->is_seeking && do_full_decode) {
FLAC__ASSERT(decoder->private_->frame.header.number_type == FLAC__FRAME_NUMBER_TYPE_SAMPLE_NUMBER);
FLAC__ASSERT(decoder->private_->last_frame.header.number_type == FLAC__FRAME_NUMBER_TYPE_SAMPLE_NUMBER);
if(decoder->private_->last_frame.header.number.sample_number + decoder->private_->last_frame.header.blocksize < decoder->private_->frame.header.number.sample_number) {
uint32_t padding_samples_needed = decoder->private_->frame.header.number.sample_number - (decoder->private_->last_frame.header.number.sample_number + decoder->private_->last_frame.header.blocksize);
/* Do some extra validation to assure last frame an current frame
* header are both valid before adding silence inbetween
* Technically both frames could be valid with differing sample_rates,
* channels and bits_per_sample, but it is quite rare */
if(decoder->private_->last_frame.header.sample_rate == decoder->private_->frame.header.sample_rate &&
decoder->private_->last_frame.header.channels == decoder->private_->frame.header.channels &&
decoder->private_->last_frame.header.bits_per_sample == decoder->private_->frame.header.bits_per_sample) {
FLAC__Frame empty_frame;
empty_frame.header = decoder->private_->last_frame.header;
empty_frame.footer.crc = 0;
/* No repairs larger than 5 seconds are made, to not unexpectedly create
* enormous files when one of the headers was corrupt after all */
if(padding_samples_needed > (5*empty_frame.header.sample_rate))
padding_samples_needed = 5*empty_frame.header.sample_rate;
while(padding_samples_needed){
empty_frame.header.number.sample_number += empty_frame.header.blocksize;
if(padding_samples_needed < empty_frame.header.blocksize)
empty_frame.header.blocksize = padding_samples_needed;
padding_samples_needed -= empty_frame.header.blocksize;
decoder->protected_->blocksize = empty_frame.header.blocksize;
FLAC__ASSERT(empty_frame.header.number_type == FLAC__FRAME_NUMBER_TYPE_SAMPLE_NUMBER);
decoder->private_->samples_decoded = empty_frame.header.number.sample_number + empty_frame.header.blocksize;
if(!allocate_output_(decoder, empty_frame.header.blocksize, empty_frame.header.channels))
return false;
for(channel = 0; channel < empty_frame.header.channels; channel++) {
empty_frame.subframes[channel].type = FLAC__SUBFRAME_TYPE_CONSTANT;
empty_frame.subframes[channel].data.constant.value = 0;
empty_frame.subframes[channel].wasted_bits = 0;
memset(decoder->private_->output[channel], 0, sizeof(FLAC__int32) * empty_frame.header.blocksize);
}
if(write_audio_frame_to_client_(decoder, &empty_frame, (const FLAC__int32 * const *)decoder->private_->output) != FLAC__STREAM_DECODER_WRITE_STATUS_CONTINUE) {
decoder->protected_->state = FLAC__STREAM_DECODER_ABORTED;
return false;
}
}
}
}
}
*got_a_frame = true;
if(decoder->protected_->state == FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC || decoder->protected_->state == FLAC__STREAM_DECODER_END_OF_STREAM) {
/* Got corruption, rewind if possible. Return value of seek
* isn't checked, if the seek fails the decoder will continue anyway */
if(!FLAC__bitreader_rewind_to_after_last_seen_framesync(decoder->private_->input)){
#ifndef NDEBUG
fprintf(stderr, "Rewinding, seeking necessary\n");
#endif
if(decoder->private_->seek_callback){
/* Last framesync isn't in bitreader anymore, rewind with seek if possible */
#ifndef NDEBUG
FLAC__uint64 current_decode_position;
if(FLAC__stream_decoder_get_decode_position(decoder, &current_decode_position))
fprintf(stderr, "Bitreader was %lu bytes short\n", current_decode_position-decoder->private_->last_seen_framesync);
#endif
if(decoder->private_->seek_callback(decoder, decoder->private_->last_seen_framesync, decoder->private_->client_data) == FLAC__STREAM_DECODER_SEEK_STATUS_ERROR) {
decoder->protected_->state = FLAC__STREAM_DECODER_SEEK_ERROR;
return false;
}
if(!FLAC__bitreader_clear(decoder->private_->input)) {
decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR;
return false;
}
}
}
#ifndef NDEBUG
else{
fprintf(stderr, "Rewinding, seeking not necessary\n");
}
#endif
}
else {
*got_a_frame = true;
/* we wait to update fixed_block_size until here, when we're sure we've got a proper frame and hence a correct blocksize */
if(decoder->private_->next_fixed_block_size)
decoder->private_->fixed_block_size = decoder->private_->next_fixed_block_size;
/* we wait to update fixed_block_size until here, when we're sure we've got a proper frame and hence a correct blocksize */
if(decoder->private_->next_fixed_block_size)
decoder->private_->fixed_block_size = decoder->private_->next_fixed_block_size;
/* put the latest values into the public section of the decoder instance */
decoder->protected_->channels = decoder->private_->frame.header.channels;
decoder->protected_->channel_assignment = decoder->private_->frame.header.channel_assignment;
decoder->protected_->bits_per_sample = decoder->private_->frame.header.bits_per_sample;
decoder->protected_->sample_rate = decoder->private_->frame.header.sample_rate;
decoder->protected_->blocksize = decoder->private_->frame.header.blocksize;
/* put the latest values into the public section of the decoder instance */
decoder->protected_->channels = decoder->private_->frame.header.channels;
decoder->protected_->channel_assignment = decoder->private_->frame.header.channel_assignment;
decoder->protected_->bits_per_sample = decoder->private_->frame.header.bits_per_sample;
decoder->protected_->sample_rate = decoder->private_->frame.header.sample_rate;
decoder->protected_->blocksize = decoder->private_->frame.header.blocksize;
FLAC__ASSERT(decoder->private_->frame.header.number_type == FLAC__FRAME_NUMBER_TYPE_SAMPLE_NUMBER);
decoder->private_->samples_decoded = decoder->private_->frame.header.number.sample_number + decoder->private_->frame.header.blocksize;
FLAC__ASSERT(decoder->private_->frame.header.number_type == FLAC__FRAME_NUMBER_TYPE_SAMPLE_NUMBER);
decoder->private_->samples_decoded = decoder->private_->frame.header.number.sample_number + decoder->private_->frame.header.blocksize;
/* write it */
if(do_full_decode) {
if(write_audio_frame_to_client_(decoder, &decoder->private_->frame, (const FLAC__int32 * const *)decoder->private_->output) != FLAC__STREAM_DECODER_WRITE_STATUS_CONTINUE) {
decoder->protected_->state = FLAC__STREAM_DECODER_ABORTED;
return false;
/* write it */
if(do_full_decode) {
if(write_audio_frame_to_client_(decoder, &decoder->private_->frame, (const FLAC__int32 * const *)decoder->private_->output) != FLAC__STREAM_DECODER_WRITE_STATUS_CONTINUE) {
decoder->protected_->state = FLAC__STREAM_DECODER_ABORTED;
return false;
}
}
}
@ -2827,10 +2937,16 @@ FLAC__bool read_residual_partitioned_rice_(FLAC__StreamDecoder *decoder, uint32_
if(!FLAC__bitreader_read_raw_uint32(decoder->private_->input, &rice_parameter, FLAC__ENTROPY_CODING_METHOD_PARTITIONED_RICE_RAW_LEN))
return false; /* read_callback_ sets the state for us */
partitioned_rice_contents->raw_bits[partition] = rice_parameter;
for(u = (partition == 0)? predictor_order : 0; u < partition_samples; u++, sample++) {
if(!FLAC__bitreader_read_raw_int32(decoder->private_->input, &i, rice_parameter))
return false; /* read_callback_ sets the state for us */
residual[sample] = i;
if(rice_parameter == 0) {
for(u = (partition == 0)? predictor_order : 0; u < partition_samples; u++, sample++)
residual[sample] = 0;
}
else{
for(u = (partition == 0)? predictor_order : 0; u < partition_samples; u++, sample++) {
if(!FLAC__bitreader_read_raw_int32(decoder->private_->input, &i, rice_parameter))
return false; /* read_callback_ sets the state for us */
residual[sample] = i;
}
}
}
}
@ -2982,6 +3098,8 @@ FLAC__OggDecoderAspectReadStatus read_callback_proxy_(const void *void_decoder,
FLAC__StreamDecoderWriteStatus write_audio_frame_to_client_(FLAC__StreamDecoder *decoder, const FLAC__Frame *frame, const FLAC__int32 * const buffer[])
{
decoder->private_->last_frame = *frame; /* save the frame */
decoder->private_->last_frame_is_set = true;
if(decoder->private_->is_seeking) {
FLAC__uint64 this_frame_sample = frame->header.number.sample_number;
FLAC__uint64 next_frame_sample = this_frame_sample + (FLAC__uint64)frame->header.blocksize;
@ -2992,7 +3110,6 @@ FLAC__StreamDecoderWriteStatus write_audio_frame_to_client_(FLAC__StreamDecoder
#if FLAC__HAS_OGG
decoder->private_->got_a_frame = true;
#endif
decoder->private_->last_frame = *frame; /* save the frame */
if(this_frame_sample <= target_sample && target_sample < next_frame_sample) { /* we hit our target frame */
uint32_t delta = (uint32_t)(target_sample - this_frame_sample);
/* kick out of seek mode */

View File

@ -125,6 +125,50 @@ test_file_piped ()
echo OK
}
test_corrupted_file ()
{
name=$1
channels=$2
bps=$3
encode_options="$4"
echo $ECHO_N "$name (--channels=$channels --bps=$bps $encode_options): encode..." $ECHO_C
cmd="run_flac --verify --silent --no-padding --force --force-raw-format --endian=little --sign=signed --sample-rate=44100 --bps=$bps --channels=$channels $encode_options --no-padding $name.raw"
echo "### ENCODE $name #######################################################" >> ./streams.log
echo "### cmd=$cmd" >> ./streams.log
$cmd 2>>./streams.log || die "ERROR during encode of $name"
filesize=$(wc -c < $name.flac)
bs=$((filesize/13))
# Overwrite with 'garbagegarbagegarbage....'
yes garbage 2>/dev/null | dd of=$name.flac conv=notrunc bs=$bs seek=1 count=2 2>> ./streams.log
# Overwrite with 0x00
dd if=/dev/zero of=$name.flac conv=notrunc bs=$bs seek=4 count=2 2>> ./streams.log
# Overwrite with 0xFF
tr '\0' '\377' < /dev/zero | dd of=$name.flac conv=notrunc bs=$bs seek=7 count=2 2>> ./streams.log
# Remove section
cp $name.flac $name.tmp.flac
dd if=$name.tmp.flac of=$name.flac bs=$bs skip=12 seek=10 2>> ./streams.log
echo $ECHO_N "decode..." $ECHO_C
cmd="run_flac --silent --decode-through-errors --force --endian=little --sign=signed --decode --force-raw-format --output-name=$name.cmp $name.flac"
echo "### DECODE $name.corrupt #######################################################" >> ./streams.log
echo "### cmd=$cmd" >> ./streams.log
$cmd 2>>./streams.log || die "ERROR during decode of $name"
ls -1l $name.raw >> ./streams.log
ls -1l $name.flac >> ./streams.log
ls -1l $name.cmp >> ./streams.log
echo $ECHO_N "compare..." $ECHO_C
if [ $(wc -c < $name.raw) -ne $(wc -c < $name.cmp) ]; then
die "ERROR, length of decoded file not equal to length of original"
fi
echo OK
}
if [ "$FLAC__TEST_LEVEL" -gt 1 ] ; then
max_lpc_order=32
else
@ -230,6 +274,27 @@ for f in 10 11 12 13 14 15 16 17 18 19 ; do
done
done
echo "Testing corruption handling..."
for bps in 8 16 24 ; do
for f in 00 01 02 03 04 10 11 12 13 14 15 16 17 18 19; do
for disable in '' '--disable-verbatim-subframes --disable-constant-subframes' '--disable-verbatim-subframes --disable-constant-subframes --disable-fixed-subframes' ; do
if [ -z "$disable" ] || [ "$FLAC__TEST_LEVEL" -gt 0 ] ; then
for opt in 0 1 2 4 5 6 8 ; do
for extras in '' '-p' '-e' ; do
if ([ -z "$extras" ] || [ "$FLAC__TEST_LEVEL" -gt 0 ]) && (([ "$bps" -eq 16 ] && [ "$f" -lt 15 ]) || [ "$FLAC__TEST_LEVEL" -gt 1 ]) ; then
if [ "$f" -lt 10 ] ; then
test_corrupted_file sine$bps-$f 1 $bps "-$opt $extras $disable"
else
test_corrupted_file sine$bps-$f 2 $bps "-$opt $extras $disable"
fi
fi
done
done
fi
done
done
done
echo "Testing noise..."
for disable in '' '--disable-verbatim-subframes --disable-constant-subframes' '--disable-verbatim-subframes --disable-constant-subframes --disable-fixed-subframes' ; do
if [ -z "$disable" ] || [ "$FLAC__TEST_LEVEL" -gt 0 ] ; then