diff --git a/Common/Data/Format/RIFF.cpp b/Common/Data/Format/RIFF.cpp index 77a24438f8..bfdce68368 100644 --- a/Common/Data/Format/RIFF.cpp +++ b/Common/Data/Format/RIFF.cpp @@ -47,6 +47,7 @@ bool RIFFReader::Descend(uint32_t intoId) { int startLocation = pos_; if (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; return false; @@ -89,7 +90,8 @@ void RIFFReader::Ascend() { eof_ = stack[depth_].parentEOF; } -void RIFFReader::ReadData(void *what, int count) { +bool RIFFReader::ReadData(void *what, int count) { + bool success = true; if (count > 0) { int available = pos_ < fileSize_ ? fileSize_ - pos_ : 0; int toRead = count < available ? count : available; @@ -98,9 +100,11 @@ void RIFFReader::ReadData(void *what, int count) { } if (toRead < count) { // Truncated/corrupt file - don't read past the buffer. Zero the rest so - // callers don't read uninitialized data. + // callers don't read uninitialized data, but also return false. + // However, reaching this is probably impossible due to the check in Descend. ERROR_LOG(Log::IO, "RIFFReader::ReadData: wanted %d bytes but only %d available", count, toRead); memset((uint8_t *)what + toRead, 0, count - toRead); + success = false; } } pos_ += count; @@ -109,6 +113,7 @@ void RIFFReader::ReadData(void *what, int count) { count = 4 - count; pos_ += count; } + return success; } int RIFFReader::GetCurrentChunkSize() { diff --git a/Common/Data/Format/RIFF.h b/Common/Data/Format/RIFF.h index 79d1c567a6..7d85d5141f 100644 --- a/Common/Data/Format/RIFF.h +++ b/Common/Data/Format/RIFF.h @@ -20,7 +20,7 @@ public: void Ascend(); int ReadInt(); - void ReadData(void *data, int count); + bool ReadData(void *data, int count); // Read count bytes into data, return false if hits EOF during read. int GetCurrentChunkSize(); diff --git a/Common/Net/HTTPHeaders.cpp b/Common/Net/HTTPHeaders.cpp index 5ebc2e5781..410ed63060 100644 --- a/Common/Net/HTTPHeaders.cpp +++ b/Common/Net/HTTPHeaders.cpp @@ -49,8 +49,6 @@ bool RequestHeader::GetOther(const char *name, std::string *value) const { return false; } -// Intended to be a mad fast parser. It's not THAT fast currently, there's still -// things to optimize, but meh. int RequestHeader::ParseHttpHeader(const char *buffer) { if (first_header_) { // Step 1: Method @@ -101,7 +99,7 @@ int RequestHeader::ParseHttpHeader(const char *buffer) { resource[resource_name_len] = '\0'; if (q_ptr) { int param_length = (int)(endptr - q_ptr - 1); - if (param_length < 0) + if (param_length < 0) // This is likely just paranoia. param_length = 0; params = new char[param_length + 1]; memcpy(params, q_ptr + 1, param_length); diff --git a/Core/HLE/sceKernelThread.cpp b/Core/HLE/sceKernelThread.cpp index 451201401c..f2610be215 100644 --- a/Core/HLE/sceKernelThread.cpp +++ b/Core/HLE/sceKernelThread.cpp @@ -2608,7 +2608,7 @@ int sceKernelReleaseWaitThread(SceUID threadID) { return hleLogError(Log::sceKernel, error, "bad thread ID"); } else { if (!t->isWaiting()) { - return hleLogInfo(Log::sceKernel, SCE_KERNEL_ERROR_NOT_WAIT); + return hleLogDebug(Log::sceKernel, SCE_KERNEL_ERROR_NOT_WAIT); } if (t->nt.waitType == WAITTYPE_HLEDELAY) { WARN_LOG_REPORT_ONCE(rwt_delay, Log::sceKernel, "sceKernelReleaseWaitThread(): Refusing to wake HLE-delayed thread, right thing to do?"); diff --git a/Core/Util/AtracTrack.cpp b/Core/Util/AtracTrack.cpp index 5ebf3f6170..e263c8d307 100644 --- a/Core/Util/AtracTrack.cpp +++ b/Core/Util/AtracTrack.cpp @@ -201,20 +201,11 @@ int AnalyzeAtracTrack(const u8 *buffer, u32 size, Track *track, std::string *err *error = StringFromFormat("smpl chunk too small for loop (%d, %d)", checkNumLoops, chunkSize); return SCE_ERROR_ATRAC_UNKNOWN_FORMAT; } - if (checkNumLoops < 0) { + u32 maxLoops = chunkSize >= 36 ? (chunkSize - 36) / 24 : 0; + if (checkNumLoops < 0 || checkNumLoops > maxLoops) { *error = StringFromFormat("bad checkNumLoops (%d)", checkNumLoops); return SCE_ERROR_ATRAC_UNKNOWN_FORMAT; } - // checkNumLoops is otherwise just an unvalidated field from the file - left - // unclamped, it could both drive an unbounded (up to ~2 billion entry) - // allocation here, and (since the loop below compares the loop counter `i` - // against chunkSize, rather than the byte offset actually being advanced by - // 24 per iteration) let reads run well past the end of this chunk. Clamp it - // to how many 24-byte loop entries could actually fit. - u32 maxLoops = chunkSize >= 36 ? (chunkSize - 36) / 24 : 0; - if ((u32)checkNumLoops > maxLoops) { - checkNumLoops = (int)maxLoops; - } track->loopinfo.resize(checkNumLoops); u32 loopinfoOffset = offset + 36; diff --git a/UI/BackgroundAudio.cpp b/UI/BackgroundAudio.cpp index 9046c29602..2cca5599ef 100644 --- a/UI/BackgroundAudio.cpp +++ b/UI/BackgroundAudio.cpp @@ -161,7 +161,12 @@ bool WavData::Read(RIFFReader &file_) { raw_data_size = numBytes; if (num_channels == 1 || num_channels == 2) { - file_.ReadData(raw_data, numBytes); + if (!file_.ReadData(raw_data, numBytes)) { + ERROR_LOG(Log::Audio, "Error - data chunk truncated"); + free(raw_data); + raw_data = nullptr; + return false; + } } else { ERROR_LOG(Log::Audio, "Error - bad blockalign or channels"); free(raw_data); @@ -187,15 +192,16 @@ bool WavData::Read(RIFFReader &file_) { // Turns out that AT3 files used for this are modified WAVE files so fairly easy to parse. class AT3PlusReader { public: - explicit AT3PlusReader(const std::string &data) - : file_((const uint8_t *)&data[0], (int32_t)data.size()) { + explicit AT3PlusReader(const std::string &data) : file_((const uint8_t *)&data[0], (int32_t)data.size()) { + if (!wave_.Read(file_)) { + ERROR_LOG(Log::Audio, "Error - could not read wave data"); + return; + } + // Normally 8k but let's be safe. buffer_ = new short[32 * 1024]; - skip_next_samples_ = 0; - wave_.Read(file_); - uint8_t *extraData = nullptr; size_t extraDataSize = 0; size_t blockSize = 0;