diff --git a/Common/Data/Collections/CharQueue.h b/Common/Data/Collections/CharQueue.h index 4927f81931..6f9f453c71 100644 --- a/Common/Data/Collections/CharQueue.h +++ b/Common/Data/Collections/CharQueue.h @@ -188,6 +188,10 @@ public: // If return value is negative, one wasn't found. int next_crlf_offset() { + // A trailing '\r' with no '\n' after it yet (e.g. right at a TCP fragmentation + // boundary) is a normal "not found yet", not an error - don't peek() past the + // data we actually have. + const size_t totalSize = size(); int offset = 0; Block *b = head_; do { @@ -195,7 +199,7 @@ public: for (int i = 0; i < remain; i++) { if (b->data[b->head + i] == '\r') { // Use peek to avoid handling edge cases. - if (peek(offset + i + 1) == '\n') { + if ((size_t)(offset + i + 1) < totalSize && peek(offset + i + 1) == '\n') { return offset + i; } } diff --git a/Common/Data/Collections/FastVec.h b/Common/Data/Collections/FastVec.h index 6993de36d7..7e84cb128a 100644 --- a/Common/Data/Collections/FastVec.h +++ b/Common/Data/Collections/FastVec.h @@ -89,9 +89,13 @@ public: // Limited functionality for inserts and similar, add as needed. T &insert(T *iter) { int pos = iter - data_; + // Capture the old size before ExtendByOne() bumps size_ - the move should only + // cover the elements that actually existed before this insert, not size_ + 1 + // worth (which would read/write one element past the valid old range). + int oldSize = (int)size_; ExtendByOne(); - if (pos + 1 < (int)size_) { - memmove(data_ + pos + 1, data_ + pos, (size_ - pos) * sizeof(T)); + if (pos < oldSize) { + memmove(data_ + pos + 1, data_ + pos, (oldSize - pos) * sizeof(T)); } return data_[pos]; } diff --git a/Common/Data/Encoding/Shiftjis.h b/Common/Data/Encoding/Shiftjis.h index bcda427d1c..35ffb8e752 100644 --- a/Common/Data/Encoding/Shiftjis.h +++ b/Common/Data/Encoding/Shiftjis.h @@ -43,6 +43,11 @@ struct ShiftJIS { } // Okay, if we didn't return, it's time for the second byte (the cell.) + if (c_[index_] == 0) { + // Truncated sequence right at the end of the string - don't consume the + // terminator, or index_ would end up one past it (OOB on the next call). + return INVALID; + } j = (uint8_t)c_[index_++]; // Not a valid second byte. if (j < 0x40 || j == 0x7F || j >= 0xFD) { diff --git a/Common/Data/Format/JSONReader.cpp b/Common/Data/Format/JSONReader.cpp index 3a4e947bb0..b5f11e57d9 100644 --- a/Common/Data/Format/JSONReader.cpp +++ b/Common/Data/Format/JSONReader.cpp @@ -106,7 +106,12 @@ bool JsonGet::getStringVector(std::vector *vec) const { } double JsonGet::getFloat(const char *child_name) const { - return get(child_name, JSON_NUMBER)->value.toNumber(); + const JsonNode *val = get(child_name, JSON_NUMBER); + if (!val) { + ERROR_LOG(Log::IO, "Number '%s' missing from node", child_name); + return 0.0; + } + return val->value.toNumber(); } double JsonGet::getFloat(const char *child_name, double default_value) const { @@ -117,7 +122,12 @@ double JsonGet::getFloat(const char *child_name, double default_value) const { } int JsonGet::getInt(const char *child_name) const { - return (int)get(child_name, JSON_NUMBER)->value.toNumber(); + const JsonNode *val = get(child_name, JSON_NUMBER); + if (!val) { + ERROR_LOG(Log::IO, "Number '%s' missing from node", child_name); + return 0; + } + return (int)val->value.toNumber(); } int JsonGet::getInt(const char *child_name, int default_value) const { @@ -128,7 +138,12 @@ int JsonGet::getInt(const char *child_name, int default_value) const { } bool JsonGet::getBool(const char *child_name) const { - return get(child_name)->value.getTag() == JSON_TRUE; + const JsonNode *val = get(child_name); + if (!val) { + ERROR_LOG(Log::IO, "Value '%s' missing from node", child_name); + return false; + } + return val->value.getTag() == JSON_TRUE; } bool JsonGet::getBoolOr(const char *child_name, bool default_value) const { diff --git a/Common/Data/Format/RIFF.cpp b/Common/Data/Format/RIFF.cpp index a748e41dfd..77a24438f8 100644 --- a/Common/Data/Format/RIFF.cpp +++ b/Common/Data/Format/RIFF.cpp @@ -90,7 +90,19 @@ void RIFFReader::Ascend() { } void RIFFReader::ReadData(void *what, int count) { - memcpy(what, data_ + pos_, count); + if (count > 0) { + int available = pos_ < fileSize_ ? fileSize_ - pos_ : 0; + int toRead = count < available ? count : available; + if (toRead > 0) { + memcpy(what, data_ + pos_, toRead); + } + if (toRead < count) { + // Truncated/corrupt file - don't read past the buffer. Zero the rest so + // callers don't read uninitialized data. + ERROR_LOG(Log::IO, "RIFFReader::ReadData: wanted %d bytes but only %d available", count, toRead); + memset((uint8_t *)what + toRead, 0, count - toRead); + } + } pos_ += count; count &= 3; if (count) { diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index 2ad9cbeeb0..8e1282c51d 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -43,7 +43,7 @@ RenderPassType MergeRPTypes(RenderPassType a, RenderPassType b) { } void VulkanQueueRunner::CreateDeviceObjects() { - INFO_LOG(Log::G3D, "VulkanQueueRunner::CreateDeviceObjects"); + DEBUG_LOG(Log::G3D, "VulkanQueueRunner::CreateDeviceObjects"); RPKey key{ VKRRenderPassLoadAction::CLEAR, VKRRenderPassLoadAction::CLEAR, VKRRenderPassLoadAction::CLEAR, @@ -67,7 +67,7 @@ void VulkanQueueRunner::CreateDeviceObjects() { } void VulkanQueueRunner::DestroyDeviceObjects() { - INFO_LOG(Log::G3D, "VulkanQueueRunner::DestroyDeviceObjects"); + DEBUG_LOG(Log::G3D, "VulkanQueueRunner::DestroyDeviceObjects"); syncReadback_.Destroy(vulkan_); diff --git a/Common/Net/HTTPClient.cpp b/Common/Net/HTTPClient.cpp index bbc8e09c75..a2743f0213 100644 --- a/Common/Net/HTTPClient.cpp +++ b/Common/Net/HTTPClient.cpp @@ -141,7 +141,7 @@ bool Connection::Connect(int maxTries, double timeout, bool *cancelConnect) { if (!unreachable) { ERROR_LOG(Log::HTTP, "connect(%d) call to %s failed (%d: %s)", sock, addrStr, errorCode, errorString.c_str()); } else { - INFO_LOG(Log::HTTP, "connect(%d): Ignoring unreachable resolved address %s", sock, addrStr); + VERBOSE_LOG(Log::HTTP, "connect(%d): Ignoring unreachable resolved address %s", sock, addrStr); } closesocket(sock); continue; diff --git a/Common/Render/Text/draw_text_sdl.cpp b/Common/Render/Text/draw_text_sdl.cpp index 3b314311aa..cde252b9a4 100644 --- a/Common/Render/Text/draw_text_sdl.cpp +++ b/Common/Render/Text/draw_text_sdl.cpp @@ -292,13 +292,13 @@ void TextDrawerSDL::SetOrCreateFont(const FontStyle &style) { uint8_t *fileData = nullptr; std::string useFont = GetFilenameForFontStyle(style) + ".ttf"; const int ptSize = static_cast(style.sizePts / dpiScale_ * 1.25f); - INFO_LOG(Log::G3D, "Loading SDL font '%s' from VFS at size %d pts", useFont.c_str(), ptSize); + DEBUG_LOG(Log::G3D, "Loading SDL font '%s' from VFS at size %d pts", useFont.c_str(), ptSize); size_t fileSz; fileData = g_VFS.ReadFile(useFont.c_str(), &fileSz); if (fileData) { SDL_IOStream *rw = SDL_IOFromConstMem(fileData, fileSz); - INFO_LOG(Log::G3D, "Opened font from RW: '%p' '%d'", fileData, (int)fileSz); + DEBUG_LOG(Log::G3D, "Opened font from RW: '%p' '%d'", fileData, (int)fileSz); font = TTF_OpenFontIO(rw, true, static_cast(ptSize)); if (!font) { ERROR_LOG(Log::G3D, "Failed to load font from asset file: '%s'", useFont.c_str()); diff --git a/Core/Config.cpp b/Core/Config.cpp index 07ea027eff..04eb036dd1 100644 --- a/Core/Config.cpp +++ b/Core/Config.cpp @@ -1464,7 +1464,7 @@ bool Config::Save(const char *saveReason) { } if (!ShouldSaveSetting(meta.settings[j].GetVoidPtr(configBlock))) { // Skip settings marked as "don't save". - INFO_LOG(Log::Config, "Not saving setting '%.*s' as marked as don't save.", STR_VIEW(meta.settings[j].IniKey())); + DEBUG_LOG(Log::Config, "Not saving setting '%.*s' as marked as don't save.", STR_VIEW(meta.settings[j].IniKey())); continue; } meta.settings[j].WriteToIniSection(configBlock, section); diff --git a/Core/ELF/ElfReader.cpp b/Core/ELF/ElfReader.cpp index 658d668b18..6bdb048bd5 100644 --- a/Core/ELF/ElfReader.cpp +++ b/Core/ELF/ElfReader.cpp @@ -560,7 +560,7 @@ int ElfReader::LoadInto(u32 loadAddress, bool fromTop) { } } } - memblock.ListBlocks(); + memblock.ListBlocks(LogLevel::LDEBUG); DEBUG_LOG(Log::Loader, "%d sections:", header->e_shnum); diff --git a/Core/HLE/sceKernelMemory.cpp b/Core/HLE/sceKernelMemory.cpp index 939fbe0fe9..77f039119a 100644 --- a/Core/HLE/sceKernelMemory.cpp +++ b/Core/HLE/sceKernelMemory.cpp @@ -371,18 +371,18 @@ void __KernelMemoryDoState(PointerWrap &p) void __KernelMemoryShutdown() { #ifdef _DEBUG - INFO_LOG(Log::sceKernel, "Shutting down volatile memory pool: "); - volatileMemory.ListBlocks(); + DEBUG_LOG(Log::sceKernel, "Shutting down volatile memory pool"); + volatileMemory.ListBlocks(LogLevel::LDEBUG); #endif volatileMemory.Shutdown(); #ifdef _DEBUG - INFO_LOG(Log::sceKernel,"Shutting down user memory pool: "); - userMemory.ListBlocks(); + DEBUG_LOG(Log::sceKernel,"Shutting down user memory pool"); + userMemory.ListBlocks(LogLevel::LDEBUG); #endif userMemory.Shutdown(); #ifdef _DEBUG - INFO_LOG(Log::sceKernel,"Shutting down \"kernel\" memory pool: "); - kernelMemory.ListBlocks(); + DEBUG_LOG(Log::sceKernel,"Shutting down \"kernel\" memory pool"); + kernelMemory.ListBlocks(LogLevel::LDEBUG); #endif kernelMemory.Shutdown(); tlsplThreadEndChecks.clear(); @@ -823,7 +823,7 @@ public: else address = alloc->Alloc(size, type == PSP_SMEM_High, name); #ifdef _DEBUG - alloc->ListBlocks(); + alloc->ListBlocks(LogLevel::LDEBUG); #endif } } @@ -1914,7 +1914,7 @@ SceUID sceKernelCreateTlspl(const char *name, u32 partition, u32 attr, u32 block u32 totalSize = alignedSize * count; u32 blockPtr = allocator->Alloc(totalSize, (attr & PSP_TLSPL_ATTR_HIGHMEM) != 0, StringFromFormat("TLS/%s", name).c_str()); #ifdef _DEBUG - allocator->ListBlocks(); + allocator->ListBlocks(LogLevel::LDEBUG); #endif if (blockPtr == (u32)-1) diff --git a/Core/Util/BlockAllocator.cpp b/Core/Util/BlockAllocator.cpp index 85d5265d98..b680fefaa1 100644 --- a/Core/Util/BlockAllocator.cpp +++ b/Core/Util/BlockAllocator.cpp @@ -134,7 +134,7 @@ u32 BlockAllocator::AllocAligned(u32 &size, u32 sizeGrain, u32 grain, bool fromT } //Out of memory :( - ListBlocks(); + ListBlocks(LogLevel::LINFO); ERROR_LOG(Log::sceKernel, "Block Allocator (%08x-%08x) failed to allocate %i (%08x) bytes of contiguous memory", rangeStart_, rangeStart_ + rangeSize_, size, size); return -1; } @@ -215,7 +215,7 @@ u32 BlockAllocator::AllocAt(u32 position, u32 size, const char *tag) //Out of memory :( - ListBlocks(); + ListBlocks(LogLevel::LINFO); ERROR_LOG(Log::sceKernel, "Block Allocator (%08x-%08x) failed to allocate %i (%08x) bytes of contiguous memory", rangeStart_, rangeStart_ + rangeSize_, alignedSize, alignedSize); return -1; } @@ -387,15 +387,14 @@ u32 BlockAllocator::GetBlockSizeFromAddress(u32 addr) const return -1; } -void BlockAllocator::ListBlocks() const -{ - DEBUG_LOG(Log::sceKernel,"-----------"); +void BlockAllocator::ListBlocks(LogLevel level) const { + GENERIC_LOG(Log::sceKernel, level, "-----------"); for (const Block *bp = bottom_; bp != NULL; bp = bp->next) { const Block &b = *bp; - DEBUG_LOG(Log::sceKernel, "Block: %08x - %08x size %08x taken=%i tag=%s", b.start, b.start+b.size, b.size, b.taken ? 1:0, b.tag); + GENERIC_LOG(Log::sceKernel, level, "Block: %08x - %08x size %08x taken=%i tag=%s", b.start, b.start+b.size, b.size, b.taken ? 1:0, b.tag); } - DEBUG_LOG(Log::sceKernel,"-----------"); + GENERIC_LOG(Log::sceKernel, level, "-----------"); } u32 BlockAllocator::GetLargestFreeBlockSize() const diff --git a/Core/Util/BlockAllocator.h b/Core/Util/BlockAllocator.h index 10949bd60e..e98ae8aad2 100644 --- a/Core/Util/BlockAllocator.h +++ b/Core/Util/BlockAllocator.h @@ -21,6 +21,8 @@ class PointerWrap; #include "Common/CommonTypes.h" +#include "Common/Log.h" + class BlockAllocator { public: @@ -30,7 +32,7 @@ public: void Init(u32 _rangeStart, u32 _rangeSize, bool suballoc); void Shutdown(); - void ListBlocks() const; + void ListBlocks(LogLevel level) const; // WARNING: size can be modified upwards! u32 Alloc(u32 &size, bool fromTop = false, const char *tag = 0); diff --git a/GPU/Common/ReplacedTexture.cpp b/GPU/Common/ReplacedTexture.cpp index 3590d79bd2..6da8465662 100644 --- a/GPU/Common/ReplacedTexture.cpp +++ b/GPU/Common/ReplacedTexture.cpp @@ -649,27 +649,33 @@ ReplacedTexture::LoadLevelResult ReplacedTexture::LoadLevelData(VFSFileReference } vfs_->CloseFile(openFile); - int w, h, f; - uint8_t *image; + // LoadZIMPtr writes to these as arrays (one entry per mip level, up to + // ZIM_MAX_MIP_LEVELS) whenever the file has ZIM_HAS_MIPS set - passing plain + // scalars here was an OOB stack write waiting for a mipped (or malicious) ZIM. + int w[ZIM_MAX_MIP_LEVELS], h[ZIM_MAX_MIP_LEVELS], f; + uint8_t *image[ZIM_MAX_MIP_LEVELS]; std::vector &out = data_[mipLevel]; // TODO: Zim files can actually hold mipmaps (although no tool has ever been made to create them :P) - if (LoadZIMPtr(&zim[0], fileSize, &w, &h, &f, &image)) { - if (w > level.w || h > level.h) { + // We only use the first level for now. + int numLevels = LoadZIMPtr(&zim[0], fileSize, w, h, &f, image); + if (numLevels > 0) { + if (w[0] > level.w || h[0] > level.h) { ERROR_LOG(Log::TexReplacement, "Texture replacement changed since header read: %s", filename.c_str()); + free(image[0]); return LoadLevelResult::LOAD_ERROR; } out.resize(level.w * level.h * 4); - if (w == level.w) { - memcpy(&out[0], image, level.w * 4 * level.h); + if (w[0] == level.w) { + memcpy(&out[0], image[0], level.w * 4 * level.h); } else { - for (int y = 0; y < h; ++y) { - memcpy(&out[level.w * 4 * y], image + w * 4 * y, w * 4); + for (int y = 0; y < h[0]; ++y) { + memcpy(&out[level.w * 4 * y], image[0] + w[0] * 4 * y, w[0] * 4); } } - free(image); + free(image[0]); - const TextureAlpha res = CheckAlpha32Rect((u32 *)&out[0], level.w, w, h, 0xFF000000); + const TextureAlpha res = CheckAlpha32Rect((u32 *)&out[0], level.w, w[0], h[0], 0xFF000000); if (res == TextureAlpha::Any || mipLevel == 0) { alphaStatus_ = res; } diff --git a/SDL/SDLJoystick.cpp b/SDL/SDLJoystick.cpp index d681c0cad0..6c8dad665a 100644 --- a/SDL/SDLJoystick.cpp +++ b/SDL/SDLJoystick.cpp @@ -25,11 +25,10 @@ SDLJoystick::SDLJoystick(bool init_SDL ) : registeredAsEventHandler(false) { } const char *dbPath = "gamecontrollerdb.txt"; - INFO_LOG(Log::System, "loading control pad mappings from %s:", dbPath); - size_t size; u8 *mappingData = g_VFS.ReadFile(dbPath, &size); if (mappingData) { + DEBUG_LOG(Log::System, "loading control pad mappings from '%s'", dbPath); SDL_IOStream *io = SDL_IOFromConstMem(mappingData, size); if (SDL_AddGamepadMappingsFromIO(io, true) == -1) { ERROR_LOG(Log::System, "Failed to read mapping data - corrupt?"); diff --git a/SDL/SDLMain.cpp b/SDL/SDLMain.cpp index db5f7e28be..65b1a59eb2 100644 --- a/SDL/SDLMain.cpp +++ b/SDL/SDLMain.cpp @@ -1521,7 +1521,6 @@ static void ProcessSDLEvent(SDL_Window *window, const SDL_Event &event, InputSta switch (event.button.button) { case SDL_BUTTON_LEFT: { - INFO_LOG(Log::UI, "SDL_EVENT_MOUSE_BUTTON_DOWN: %f x %f", event.button.x, event.button.y); // We have to juggle around 3 kinds of "DPI spaces" if a logical DPI is // provided (through --dpi, it is equal to system DPI if unspecified): // - SDL gives us motion events in "system DPI" points diff --git a/UI/BackgroundAudio.cpp b/UI/BackgroundAudio.cpp index 63f1ed8acd..9046c29602 100644 --- a/UI/BackgroundAudio.cpp +++ b/UI/BackgroundAudio.cpp @@ -107,37 +107,42 @@ bool WavData::Read(RIFFReader &file_) { if (file_.Descend('smpl')) { std::vector smplData; smplData.resize(file_.GetCurrentChunkSize()); - file_.ReadData(&smplData[0], (int)smplData.size()); + if (!smplData.empty()) { + file_.ReadData(smplData.data(), (int)smplData.size()); + } - int numLoops = *(int *)&smplData[28]; - struct AtracLoopInfo { - int cuePointID; - int type; - int startSample; - int endSample; - int fraction; - int playCount; - }; + // A short/corrupt 'smpl' chunk shouldn't make us read past the buffer. + if (smplData.size() >= 32) { + int numLoops = *(int *)&smplData[28]; + struct AtracLoopInfo { + int cuePointID; + int type; + int startSample; + int endSample; + int fraction; + int playCount; + }; - if (numLoops > 0 && smplData.size() >= 36 + sizeof(AtracLoopInfo) * numLoops) { - AtracLoopInfo *loops = (AtracLoopInfo *)&smplData[36]; - int samplesPerFrame = codec == PSP_CODEC_AT3PLUS ? 2048 : 1024; + if (numLoops > 0 && smplData.size() >= 36 + sizeof(AtracLoopInfo) * numLoops) { + AtracLoopInfo *loops = (AtracLoopInfo *)&smplData[36]; + int samplesPerFrame = codec == PSP_CODEC_AT3PLUS ? 2048 : 1024; - for (int i = 0; i < numLoops; ++i) { - // Only seen forward loops, so let's ignore others. - if (loops[i].type != 0) - continue; + for (int i = 0; i < numLoops; ++i) { + // Only seen forward loops, so let's ignore others. + if (loops[i].type != 0) + continue; - // We ignore loop interpolation (fraction) and play count for now. - raw_offset_loop_start = (loops[i].startSample / samplesPerFrame) * raw_bytes_per_frame; - loop_start_offset = loops[i].startSample % samplesPerFrame; - raw_offset_loop_end = (loops[i].endSample / samplesPerFrame) * raw_bytes_per_frame; - loop_end_offset = loops[i].endSample % samplesPerFrame; + // We ignore loop interpolation (fraction) and play count for now. + raw_offset_loop_start = (loops[i].startSample / samplesPerFrame) * raw_bytes_per_frame; + loop_start_offset = loops[i].startSample % samplesPerFrame; + raw_offset_loop_end = (loops[i].endSample / samplesPerFrame) * raw_bytes_per_frame; + loop_end_offset = loops[i].endSample % samplesPerFrame; - if (loops[i].playCount == 0) { - // This was an infinite loop, so ignore the rest. - // In practice, there's usually only one and it's usually infinite. - break; + if (loops[i].playCount == 0) { + // This was an infinite loop, so ignore the rest. + // In practice, there's usually only one and it's usually infinite. + break; + } } } } diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp index 71ad7df558..e715adbb2d 100644 --- a/UI/NativeApp.cpp +++ b/UI/NativeApp.cpp @@ -223,6 +223,8 @@ class GlobalListener : public ControlListener { g_Config.bShowImDebugger = !g_Config.bShowImDebugger; } break; + default: + break; } } }; @@ -801,7 +803,6 @@ void NativeInit(int argc, const char *argv[], const CommandLineOptions &cmdLineO ApplyAchievementsHostOverride(); - DEBUG_LOG(Log::System, "ScreenManager!"); g_screenManager = new ScreenManager(); if (g_Config.memStickDirectory.empty()) { INFO_LOG(Log::System, "No memstick directory! Asking for one to be configured."); @@ -1203,6 +1204,8 @@ void NativeFrame(GraphicsContext *graphicsContext) { case QueuedEventType::TOUCH: ImGui_ImplPlatform_TouchEvent(event.touch); break; + default: + break; } } }