From cd8bd69c6e65c185e757133bcf67ff82ce7cf748 Mon Sep 17 00:00:00 2001 From: Paul Schwabauer Date: Sat, 10 Jan 2026 22:31:21 +0100 Subject: [PATCH] Fix undefined behavior (#6089) Fix TimeSplit crash on empty name Initialize OptionValue::mVal to fix undefined behavior Fix undefined behavior in GraveHoleJumps surface type copy. The memcpy was reading 33 SurfaceTypes regardless of the actual count, causing a buffer overread since NTSC 1.0 only has 31 surface types and later versions have 32. Now uses the actual surfaceTypesCount from the collision header. Fix undefined behavior in framebuffer OTR signature check. Use calloc instead of malloc for framebuffer allocation to zero-initialize the memory. This fixes Valgrind warnings about reading uninitialized values when ResourceMgr_OTRSigCheck reads from framebuffer pointers to check for the "__OTR__" signature. Fix undefined behavior in fontLoadStatus initialization. Use calloc instead of malloc when allocating fontLoadStatus array to ensure zero-initialization. This fixes Valgrind warnings about conditional jumps depending on uninitialized values in AudioLoad_SetFontLoadStatus. --- soh/soh/Enhancements/Restorations/GraveHoleJumps.cpp | 11 ++++++----- soh/soh/Enhancements/randomizer/option.h | 2 +- soh/soh/SohGui/UIWidgets.hpp | 2 +- soh/src/code/audio_load.c | 2 +- soh/src/code/sys_cfb.c | 4 ++-- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/soh/soh/Enhancements/Restorations/GraveHoleJumps.cpp b/soh/soh/Enhancements/Restorations/GraveHoleJumps.cpp index 525d30dc2..78b1a45ba 100644 --- a/soh/soh/Enhancements/Restorations/GraveHoleJumps.cpp +++ b/soh/soh/Enhancements/Restorations/GraveHoleJumps.cpp @@ -31,15 +31,16 @@ CollisionHeader* getGraveyardCollisionHeader() { */ SOH::Scene* scene = (SOH::Scene*)Ship::Context::GetInstance()->GetResourceManager()->LoadResource(GRAVEYARD_SCENE_FILEPATH).get(); - SOH::ISceneCommand* sceneCmd = nullptr; - for (int i = 0; i < scene->commands.size(); i++) { + SOH::SetCollisionHeader* sceneCmd = nullptr; + for (size_t i = 0; i < scene->commands.size(); i++) { auto cmd = scene->commands[i]; if (cmd->cmdId == SOH::SceneCommandID::SetCollisionHeader) { - sceneCmd = cmd.get(); + sceneCmd = static_cast(cmd.get()); break; } } - CollisionHeader* graveyardColHeader = (CollisionHeader*)((SOH::SetCollisionHeader*)sceneCmd)->GetRawPointer(); + CollisionHeader* graveyardColHeader = (CollisionHeader*)sceneCmd->GetRawPointer(); + uint32_t surfaceTypesCount = sceneCmd->collisionHeader->surfaceTypesCount; /* * Copy the surface type list and give ourselves some extra space to create another surface type for Link to fall @@ -47,7 +48,7 @@ CollisionHeader* getGraveyardCollisionHeader() { * are shifted somewhat between versions, so to be safe we just create an extra slot that is not in any version. */ static SurfaceType newSurfaceTypes[33]; - memcpy(newSurfaceTypes, graveyardColHeader->surfaceTypeList, sizeof(SurfaceType) * 33); + memcpy(newSurfaceTypes, graveyardColHeader->surfaceTypeList, sizeof(SurfaceType) * surfaceTypesCount); newSurfaceTypes[CUSTOM_SURFACE_TYPE].data[0] = 0x24000004; newSurfaceTypes[CUSTOM_SURFACE_TYPE].data[1] = 0xFC8; graveyardColHeader->surfaceTypeList = newSurfaceTypes; diff --git a/soh/soh/Enhancements/randomizer/option.h b/soh/soh/Enhancements/randomizer/option.h index 9d847f471..bbab8d5c3 100644 --- a/soh/soh/Enhancements/randomizer/option.h +++ b/soh/soh/Enhancements/randomizer/option.h @@ -85,7 +85,7 @@ class OptionValue { explicit operator bool() const; private: - uint8_t mVal; + uint8_t mVal = 0; }; /** diff --git a/soh/soh/SohGui/UIWidgets.hpp b/soh/soh/SohGui/UIWidgets.hpp index dc116b95b..1504b38f3 100644 --- a/soh/soh/SohGui/UIWidgets.hpp +++ b/soh/soh/SohGui/UIWidgets.hpp @@ -826,7 +826,7 @@ bool Combobox(const char* label, T* value, const std::vector& combo ImGui::BeginDisabled(options.disabled); PushStyleCombobox(options.color); - const char* longest; + const char* longest = ""; size_t length = 0; for (auto& string : comboVector) { size_t len = string.length(); diff --git a/soh/src/code/audio_load.c b/soh/src/code/audio_load.c index 71197c133..6f6b4f4a3 100644 --- a/soh/src/code/audio_load.c +++ b/soh/src/code/audio_load.c @@ -1364,7 +1364,7 @@ void AudioLoad_Init(void* heap, size_t heapSize) { char** fntList = ResourceMgr_ListFiles("audio/fonts*", &fntListSize); char** customFntList = ResourceMgr_ListFiles("custom/fonts/*", &customFntListSize); - gAudioContext.fontLoadStatus = malloc(customFntListSize + fntListSize); + gAudioContext.fontLoadStatus = calloc(customFntListSize + fntListSize, sizeof(u8)); fontMap = calloc(customFntListSize + fntListSize, sizeof(char*)); fontMapSize = customFntListSize + fntListSize; for (int i = 0; i < fntListSize; i++) { diff --git a/soh/src/code/sys_cfb.c b/soh/src/code/sys_cfb.c index 53e9306bc..8bfb3f669 100644 --- a/soh/src/code/sys_cfb.c +++ b/soh/src/code/sys_cfb.c @@ -36,8 +36,8 @@ void SysCfb_Init(s32 n64dd) { osSyncPrintf("システムが使用する最終アドレスは %08x です\n", sSysCfbEnd); // sSysCfbFbPtr[0] = sSysCfbEnd - (screenSize * 4); // sSysCfbFbPtr[1] = sSysCfbEnd - (screenSize * 2); - sSysCfbFbPtr[0] = malloc(screenSize * 4); - sSysCfbFbPtr[1] = malloc(screenSize * 4); + sSysCfbFbPtr[0] = (uintptr_t)calloc(screenSize, 4); + sSysCfbFbPtr[1] = (uintptr_t)calloc(screenSize, 4); // "Frame buffer addresses are %08x and %08x" // osSyncPrintf("フレームバッファのアドレスは %08x と %08x です\n", sSysCfbFbPtr[0], sSysCfbFbPtr[1]);