BackgroundAudio: fix crashes and OOB reads parsing WAV/AT3 files

raw_bytes_per_frame (the 'fmt ' chunk's blockAlign field) is
unvalidated file data, and was used unchecked in three places:
- Divided into the 'data' chunk size to compute numFrames - a value
  of 0 divides by zero (crash).
- malloc()'d for raw_data was never null-checked before ReadData()
  wrote into it.
- Passed directly as the read length to the audio decoder on every
  frame, regardless of how much data is actually left in raw_data at
  the current offset - a bogus blockAlign larger than the real 'data'
  chunk size reads past the (padded) allocation into the decoder.
  Clamp it to what's actually available.

IsSimpleWAV() only checked raw_bytes_per_frame's upper bound, not that
it exactly matched one of the two cases Sample::Load() actually
handles (16-bit or 8-bit raw PCM) - a value in between passed the
check but matched neither of Load()'s conversion branches, leaving its
output buffer uninitialized and played back as heap garbage.

Reachable via a WAV/AT3 file parsed by BackgroundAudio.cpp - either
the menu background music preview (any EBOOT.PBP's SND0.AT3 track,
just from browsing the game list) or a user-configurable achievement
sound file.
This commit is contained in:
Henrik Rydgård committed 2026-08-12 09:29:17 +02:00
1 parent e35764d8e0
commit 4239c29928
2 files changed
+35 -4

No files matched your search

+8 -1
View File
@@ -46,7 +46,14 @@ bool RIFFReader::Descend(uint32_t intoId) {
int length = ReadInt();
int startLocation = pos_;
if (pos_ + length > fileSize_) {
// length is a raw 4-byte value from the file. Validate in 64-bit to avoid
// pos_ + length overflowing a 32-bit int (which could wrap negative and
// bypass this check for a length near INT_MAX), and reject negative lengths
// outright here - the mismatch branch below already checks length > 0 before
// advancing pos_, but a *matched* chunk with a negative length used to reach
// stack[depth_].length unchecked, and from there GetCurrentChunkSize() and
// its callers (e.g. a std::vector::resize() sized from it).
if (length < 0 || (int64_t)pos_ + length > fileSize_) {
// This should already catch the case where the file is truncated, but we also check for it in ReadData just in case.
ERROR_LOG(Log::IO, "Block extends outside of RIFF file - failing descend");
pos_ = stack[depth_].parentStartLocation;
+27 -3
View File
@@ -48,8 +48,12 @@ struct WavData {
[[nodiscard]]
bool IsSimpleWAV() const {
bool isBad = raw_bytes_per_frame > sizeof(int16_t) * num_channels;
return !isBad && num_channels > 0 && sample_rate >= 8000 && codec == 0;
// Sample::Load() only actually handles these two exact cases (16-bit or 8-bit
// raw PCM); anything else used to pass this check while leaving Load()'s
// output buffer uninitialized (played back as heap garbage) since neither of
// its two conversion branches would match.
bool validFrameSize = raw_bytes_per_frame == (int)sizeof(int16_t) * num_channels || raw_bytes_per_frame == num_channels;
return validFrameSize && num_channels > 0 && sample_rate >= 8000 && codec == 0;
}
};
@@ -152,12 +156,25 @@ bool WavData::Read(RIFFReader &file_) {
// enter the data chunk
if (file_.Descend('data')) {
// raw_bytes_per_frame (the 'fmt ' chunk's blockAlign field, read above) is
// unvalidated file data - a value of 0 would otherwise divide by zero here.
if (raw_bytes_per_frame <= 0) {
ERROR_LOG(Log::Audio, "Error - bad blockalign");
file_.Ascend();
return false;
}
int numBytes = file_.GetCurrentChunkSize();
numFrames = numBytes / raw_bytes_per_frame; // numFrames
// It seems the atrac3 codec likes to read a little bit outside.
const int padding = 32; // 32 is the value FFMPEG uses.
raw_data = (uint8_t *)malloc(numBytes + padding);
if (!raw_data) {
ERROR_LOG(Log::Audio, "Error - failed to allocate %d bytes for wave data", numBytes + padding);
file_.Ascend();
return false;
}
raw_data_size = numBytes;
if (num_channels == 1 || num_channels == 2) {
@@ -232,7 +249,14 @@ public:
while (bgQueue.size() < (size_t)(len * 2)) {
int outSamples = 0;
int inbytesConsumed = 0;
bool result = decoder_->Decode(wave_.raw_data + raw_offset_, wave_.raw_bytes_per_frame, &inbytesConsumed, 2, (int16_t *)buffer_, &outSamples);
// raw_bytes_per_frame is unvalidated file data (the 'fmt ' chunk's blockAlign
// field) - clamp the length passed to the decoder to what's actually left in
// raw_data at raw_offset_, so a bogus blockAlign can't make it read past the
// (padded) allocation.
const int kPadding = 32; // Matches WavData::Read's allocation padding.
int available = std::max(0, wave_.raw_data_size + kPadding - raw_offset_);
int inBytes = std::min(wave_.raw_bytes_per_frame, available);
bool result = decoder_->Decode(wave_.raw_data + raw_offset_, inBytes, &inbytesConsumed, 2, (int16_t *)buffer_, &outSamples);
if (!result || !outSamples)
return false;
int outBytes = outSamples * 2 * sizeof(int16_t);