Skip to content

Commit

Permalink
Fix out of bounds when trying to decode LPCM frames with more samples…
Browse files Browse the repository at this point in the history
… than expected.

  - Add direct coverage of this type of bug.
  - b/386824953: Fix issue detected by fuzzer.

PiperOrigin-RevId: 712906131
  • Loading branch information
jwcullen committed Jan 7, 2025
1 parent 3d423d0 commit 6a9cdec
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 14 deletions.
12 changes: 11 additions & 1 deletion iamf/cli/codec/lpcm_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,17 @@ absl::Status LpcmDecoder::DecodeAudioFrame(
bytes_per_sample, ") * number of channels (", num_channels_, ")."));
}
// Each time tick has one sample for each channel.
num_valid_ticks_ = encoded_frame.size() / bytes_per_sample / num_channels_;
const size_t num_ticks =
encoded_frame.size() / bytes_per_sample / num_channels_;
if (num_ticks > num_samples_per_channel_) {
return absl::InvalidArgumentError(
absl::StrCat("Detected num_ticks= ", num_ticks,
", but the decoder is only configured for up to "
"num_samples_per_channel_= ",
num_samples_per_channel_, "."));
}
num_valid_ticks_ = num_ticks;

const bool little_endian = decoder_config_.IsLittleEndian();

int32_t sample_result;
Expand Down
58 changes: 45 additions & 13 deletions iamf/cli/codec/tests/lpcm_decoder_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@ namespace {
using ::absl_testing::IsOk;

constexpr bool kOverrideAudioRollDistance = true;
constexpr uint32_t kNumSamplesPerFrame = 1024;
constexpr uint8_t kSampleSize16 = 16;
constexpr bool kLittleEndian = true;

CodecConfigObu CreateCodecConfigObu(LpcmDecoderConfig lpcm_decoder_config,
uint32_t num_samples_per_frame = 1024) {
uint32_t num_samples_per_frame) {
const CodecConfig codec_config = {
.codec_id = CodecConfig::kCodecIdLpcm,
.num_samples_per_frame = num_samples_per_frame,
Expand All @@ -35,7 +38,8 @@ TEST(LpcmDecoderTest, Construct) {
lpcm_decoder_config.sample_size_ = 16;
lpcm_decoder_config.sample_format_flags_bitmask_ =
LpcmDecoderConfig::LpcmFormatFlagsBitmask::kLpcmLittleEndian;
CodecConfigObu codec_config_obu = CreateCodecConfigObu(lpcm_decoder_config);
CodecConfigObu codec_config_obu =
CreateCodecConfigObu(lpcm_decoder_config, kNumSamplesPerFrame);
ASSERT_THAT(codec_config_obu.Initialize(kOverrideAudioRollDistance), IsOk());
int number_of_channels = 11; // Arbitrary.

Expand All @@ -51,7 +55,8 @@ TEST(LpcmDecoderTest, Initialize_InvalidConfigFails) {
lpcm_decoder_config.sample_size_ = 16;
lpcm_decoder_config.sample_format_flags_bitmask_ =
LpcmDecoderConfig::LpcmFormatFlagsBitmask::kLpcmBeginReserved;
CodecConfigObu codec_config_obu = CreateCodecConfigObu(lpcm_decoder_config);
CodecConfigObu codec_config_obu =
CreateCodecConfigObu(lpcm_decoder_config, kNumSamplesPerFrame);
ASSERT_THAT(codec_config_obu.Initialize(kOverrideAudioRollDistance), IsOk());
int number_of_channels = 11; // Arbitrary.

Expand All @@ -62,7 +67,8 @@ TEST(LpcmDecoderTest, Initialize_InvalidConfigFails) {
}

LpcmDecoder CreateDecoderForDecodingTest(uint8_t sample_size,
bool little_endian) {
bool little_endian,
uint32_t num_samples_per_frame) {
LpcmDecoderConfig lpcm_decoder_config;
// The sample rate and bit depth are validated with CodecConfigObu::Initialize
// so if we want to test the validation in LpcmDecoderConfig::Initialize we
Expand All @@ -72,7 +78,8 @@ LpcmDecoder CreateDecoderForDecodingTest(uint8_t sample_size,
using enum LpcmDecoderConfig::LpcmFormatFlagsBitmask;
lpcm_decoder_config.sample_format_flags_bitmask_ =
little_endian ? kLpcmLittleEndian : kLpcmBigEndian;
CodecConfigObu codec_config_obu = CreateCodecConfigObu(lpcm_decoder_config);
CodecConfigObu codec_config_obu =
CreateCodecConfigObu(lpcm_decoder_config, num_samples_per_frame);
if (!codec_config_obu.Initialize(kOverrideAudioRollDistance).ok()) {
LOG(ERROR) << "Failed to initialize codec config OBU";
}
Expand All @@ -83,11 +90,36 @@ LpcmDecoder CreateDecoderForDecodingTest(uint8_t sample_size,
return lpcm_decoder;
}

TEST(LpcmDecoderTest, DecodeAudioFrame_FailsWhenFrameIsLargerThanExpected) {
constexpr uint32_t kShortNumberOfSamplesPerFrame = 1;
LpcmDecoder lpcm_decoder = CreateDecoderForDecodingTest(
kSampleSize16, kLittleEndian, kShortNumberOfSamplesPerFrame);
const std::vector<uint8_t>& kEncodedFrameWithOneSamplesPerFrame = {
0x00, 0x00, // 0
0x01, 0x00, // 1
};
// The decoder is configured correctly. One sample per frame decodes fine.
ASSERT_THAT(
lpcm_decoder.DecodeAudioFrame(kEncodedFrameWithOneSamplesPerFrame),
IsOk());

// But decoding two samples per frame fails, since the decoder was configured
// for at most one sample per frame.
const std::vector<uint8_t>& kEncodedFrameWithTwoSamplesPerFrame = {
0x00, 0x00, // 0
0x01, 0x00, // 1
0x00, 0x01, // 256
0x80, 0xff, // -128
};
EXPECT_FALSE(
lpcm_decoder.DecodeAudioFrame(kEncodedFrameWithTwoSamplesPerFrame).ok());
}

TEST(LpcmDecoderTest, DecodeAudioFrame_LittleEndian16BitSamples) {
uint8_t sample_size = 16;
bool little_endian = true;
LpcmDecoder lpcm_decoder =
CreateDecoderForDecodingTest(sample_size, little_endian);
LpcmDecoder lpcm_decoder = CreateDecoderForDecodingTest(
sample_size, little_endian, kNumSamplesPerFrame);
const std::vector<uint8_t>& encoded_frame = {
0x00, 0x00, // 0
0x01, 0x00, // 1
Expand All @@ -113,8 +145,8 @@ TEST(LpcmDecoderTest, DecodeAudioFrame_LittleEndian16BitSamples) {
TEST(LpcmDecoderTest, DecodeAudioFrame_BigEndian24BitSamples) {
uint8_t sample_size = 24;
bool little_endian = false;
LpcmDecoder lpcm_decoder =
CreateDecoderForDecodingTest(sample_size, little_endian);
LpcmDecoder lpcm_decoder = CreateDecoderForDecodingTest(
sample_size, little_endian, kNumSamplesPerFrame);
const std::vector<uint8_t>& encoded_frame = {
0x00, 0x00, 0x00, // 0
0x00, 0x00, 0x01, // 1
Expand Down Expand Up @@ -145,8 +177,8 @@ TEST(LpcmDecoderTest, DecodeAudioFrame_BigEndian24BitSamples) {
TEST(LpcmDecoderTest, DecodeAudioFrame_WillNotDecodeWrongSize) {
uint8_t sample_size = 16;
bool little_endian = true;
LpcmDecoder lpcm_decoder =
CreateDecoderForDecodingTest(sample_size, little_endian);
LpcmDecoder lpcm_decoder = CreateDecoderForDecodingTest(
sample_size, little_endian, kNumSamplesPerFrame);
// If we have 6 bytes, 16-bit samples, and two channels, we only have 3
// samples which doesn't divide evenly into the number of channels.
const std::vector<uint8_t>& encoded_frame = {0x00, 0x00, 0x00,
Expand All @@ -160,8 +192,8 @@ TEST(LpcmDecoderTest, DecodeAudioFrame_WillNotDecodeWrongSize) {
TEST(LpcmDecoderTest, DecodeAudioFrame_OverwritesExistingSamples) {
uint8_t sample_size = 16;
bool little_endian = true;
LpcmDecoder lpcm_decoder =
CreateDecoderForDecodingTest(sample_size, little_endian);
LpcmDecoder lpcm_decoder = CreateDecoderForDecodingTest(
sample_size, little_endian, kNumSamplesPerFrame);
const std::vector<uint8_t>& encoded_frame = {0x00, 0x00, 0x01, 0x00};

auto status = lpcm_decoder.DecodeAudioFrame(encoded_frame);
Expand Down

0 comments on commit 6a9cdec

Please sign in to comment.