Skip to content

Commit

Permalink
Silence oss-fuzz integer overflow warnings in audio data path
Browse files Browse the repository at this point in the history
Because fuzzing feeds bogus predictors and residual samples to the
decoder, having overflows in certain functions is unavoidable. Also,
because the calculated values are audio path only, there is little
potential for security problems

Should 'fix' the following reports
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=44824
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=46964
  • Loading branch information
ktmf01 committed Apr 30, 2022
1 parent ef4ad99 commit 63ac1c3
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 38 deletions.
8 changes: 8 additions & 0 deletions src/libFLAC/fixed.c
Expand Up @@ -364,6 +364,14 @@ void FLAC__fixed_compute_residual(const FLAC__int32 data[], uint32_t data_len, u
}
}

#if defined(__clang__)
/* The attribute below is to silence the undefined sanitizer of oss-fuzz.
* Because fuzzing feeds bogus predictors and residual samples to the
* decoder, having overflows in this section is unavoidable. Also,
* because the calculated values are audio path only, there is no
* potential for security problems */
__attribute__((no_sanitize("signed-integer-overflow")))
#endif
void FLAC__fixed_restore_signal(const FLAC__int32 residual[], uint32_t data_len, uint32_t order, FLAC__int32 data[])
{
int i, idata_len = (int)data_len;
Expand Down
10 changes: 9 additions & 1 deletion src/libFLAC/lpc.c
Expand Up @@ -781,6 +781,14 @@ void FLAC__lpc_compute_residual_from_qlp_coefficients_wide(const FLAC__int32 * f

#endif /* !defined FLAC__INTEGER_ONLY_LIBRARY */

#if defined(__clang__)
/* The attribute below is to silence the undefined sanitizer of oss-fuzz.
* Because fuzzing feeds bogus predictors and residual samples to the
* decoder, having overflows in this section is unavoidable. Also,
* because the calculated values are audio path only, there is no
* potential for security problems */
__attribute__((no_sanitize("signed-integer-overflow")))
#endif
void FLAC__lpc_restore_signal(const FLAC__int32 * flac_restrict residual, uint32_t data_len, const FLAC__int32 * flac_restrict qlp_coeff, uint32_t order, int lp_quantization, FLAC__int32 * flac_restrict data)
#if defined(FLAC__OVERFLOW_DETECT) || !defined(FLAC__LPC_UNROLLED_FILTER_LOOPS)
{
Expand Down Expand Up @@ -1248,7 +1256,7 @@ void FLAC__lpc_restore_signal_wide(const FLAC__int32 * flac_restrict residual, u
}
else { /* order == 1 */
for(i = 0; i < (int)data_len; i++)
data[i] = residual[i] + (FLAC__int32)((qlp_coeff[0] * (FLAC__int64)data[i-1]) >> lp_quantization);
data[i] = (FLAC__int32)(residual[i] + ((qlp_coeff[0] * (FLAC__int64)data[i-1]) >> lp_quantization));
}
}
}
Expand Down
87 changes: 50 additions & 37 deletions src/libFLAC/stream_decoder.c
Expand Up @@ -95,6 +95,7 @@ static FLAC__bool read_subframe_lpc_(FLAC__StreamDecoder *decoder, uint32_t chan
static FLAC__bool read_subframe_verbatim_(FLAC__StreamDecoder *decoder, uint32_t channel, uint32_t bps, FLAC__bool do_full_decode);
static FLAC__bool read_residual_partitioned_rice_(FLAC__StreamDecoder *decoder, uint32_t predictor_order, uint32_t partition_order, FLAC__EntropyCodingMethod_PartitionedRiceContents *partitioned_rice_contents, FLAC__int32 *residual, FLAC__bool is_extended);
static FLAC__bool read_zero_padding_(FLAC__StreamDecoder *decoder);
static void undo_channel_coding(FLAC__StreamDecoder *decoder);
static FLAC__bool read_callback_(FLAC__byte buffer[], size_t *bytes, void *client_data);
#if FLAC__HAS_OGG
static FLAC__StreamDecoderReadStatus read_callback_ogg_aspect_(const FLAC__StreamDecoder *decoder, FLAC__byte buffer[], size_t *bytes);
Expand Down Expand Up @@ -2041,7 +2042,6 @@ FLAC__bool read_frame_(FLAC__StreamDecoder *decoder, FLAC__bool *got_a_frame, FL
{
uint32_t channel;
uint32_t i;
FLAC__int32 mid, side;
uint32_t frame_crc; /* the one we calculate from the input stream */
FLAC__uint32 x;

Expand Down Expand Up @@ -2118,42 +2118,7 @@ FLAC__bool read_frame_(FLAC__StreamDecoder *decoder, FLAC__bool *got_a_frame, FL
#endif
if(do_full_decode) {
/* Undo any special channel coding */
switch(decoder->private_->frame.header.channel_assignment) {
case FLAC__CHANNEL_ASSIGNMENT_INDEPENDENT:
/* do nothing */
break;
case FLAC__CHANNEL_ASSIGNMENT_LEFT_SIDE:
FLAC__ASSERT(decoder->private_->frame.header.channels == 2);
for(i = 0; i < decoder->private_->frame.header.blocksize; i++)
decoder->private_->output[1][i] = decoder->private_->output[0][i] - decoder->private_->output[1][i];
break;
case FLAC__CHANNEL_ASSIGNMENT_RIGHT_SIDE:
FLAC__ASSERT(decoder->private_->frame.header.channels == 2);
for(i = 0; i < decoder->private_->frame.header.blocksize; i++)
decoder->private_->output[0][i] += decoder->private_->output[1][i];
break;
case FLAC__CHANNEL_ASSIGNMENT_MID_SIDE:
FLAC__ASSERT(decoder->private_->frame.header.channels == 2);
for(i = 0; i < decoder->private_->frame.header.blocksize; i++) {
#if 1
mid = decoder->private_->output[0][i];
side = decoder->private_->output[1][i];
mid = ((uint32_t) mid) << 1;
mid |= (side & 1); /* i.e. if 'side' is odd... */
decoder->private_->output[0][i] = (mid + side) >> 1;
decoder->private_->output[1][i] = (mid - side) >> 1;
#else
/* OPT: without 'side' temp variable */
mid = (decoder->private_->output[0][i] << 1) | (decoder->private_->output[1][i] & 1); /* i.e. if 'side' is odd... */
decoder->private_->output[0][i] = (mid + decoder->private_->output[1][i]) >> 1;
decoder->private_->output[1][i] = (mid - decoder->private_->output[1][i]) >> 1;
#endif
}
break;
default:
FLAC__ASSERT(0);
break;
}
undo_channel_coding(decoder);
/* 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++) {
Expand Down Expand Up @@ -3050,6 +3015,54 @@ FLAC__bool read_callback_(FLAC__byte buffer[], size_t *bytes, void *client_data)
*/
}

#if defined(__clang__)
/* The attribute below is to silence the undefined sanitizer of oss-fuzz.
* Because fuzzing feeds bogus predictors and residual samples to the
* decoder, having overflows in this section is unavoidable. Also,
* because the calculated values are audio path only, there is no
* potential for security problems */
__attribute__((no_sanitize("signed-integer-overflow")))
#endif
void undo_channel_coding(FLAC__StreamDecoder *decoder) {
FLAC__int32 mid, side;
switch(decoder->private_->frame.header.channel_assignment) {
case FLAC__CHANNEL_ASSIGNMENT_INDEPENDENT:
/* do nothing */
break;
case FLAC__CHANNEL_ASSIGNMENT_LEFT_SIDE:
FLAC__ASSERT(decoder->private_->frame.header.channels == 2);
for(uint32_t i = 0; i < decoder->private_->frame.header.blocksize; i++)
decoder->private_->output[1][i] = decoder->private_->output[0][i] - decoder->private_->output[1][i];
break;
case FLAC__CHANNEL_ASSIGNMENT_RIGHT_SIDE:
FLAC__ASSERT(decoder->private_->frame.header.channels == 2);
for(uint32_t i = 0; i < decoder->private_->frame.header.blocksize; i++)
decoder->private_->output[0][i] += decoder->private_->output[1][i];
break;
case FLAC__CHANNEL_ASSIGNMENT_MID_SIDE:
FLAC__ASSERT(decoder->private_->frame.header.channels == 2);
for(uint32_t i = 0; i < decoder->private_->frame.header.blocksize; i++) {
#if 1
mid = decoder->private_->output[0][i];
side = decoder->private_->output[1][i];
mid = ((uint32_t) mid) << 1;
mid |= (side & 1); /* i.e. if 'side' is odd... */
decoder->private_->output[0][i] = (mid + side) >> 1;
decoder->private_->output[1][i] = (mid - side) >> 1;
#else
/* OPT: without 'side' temp variable */
mid = (decoder->private_->output[0][i] << 1) | (decoder->private_->output[1][i] & 1); /* i.e. if 'side' is odd... */
decoder->private_->output[0][i] = (mid + decoder->private_->output[1][i]) >> 1;
decoder->private_->output[1][i] = (mid - decoder->private_->output[1][i]) >> 1;
#endif
}
break;
default:
FLAC__ASSERT(0);
break;
}
}

#if FLAC__HAS_OGG
FLAC__StreamDecoderReadStatus read_callback_ogg_aspect_(const FLAC__StreamDecoder *decoder, FLAC__byte buffer[], size_t *bytes)
{
Expand Down

0 comments on commit 63ac1c3

Please sign in to comment.