From f02be1fface3f19d741aa79a937ce766e933c34a Mon Sep 17 00:00:00 2001 From: Kelsi Date: Sun, 29 Mar 2026 20:27:16 -0700 Subject: [PATCH] fix: tolower/toupper UB on signed char at 10 remaining call sites Final sweep across mpq_manager, application, auth_screen, wmo_renderer, character_renderer, and terrain_manager. All now use the unsigned char cast pattern. No remaining bare ::tolower/::toupper or std::tolower(c) calls on signed char in the codebase. --- src/core/application.cpp | 7 ++++--- src/pipeline/mpq_manager.cpp | 2 +- src/rendering/character_renderer.cpp | 4 ++-- src/rendering/terrain_manager.cpp | 4 ++-- src/rendering/wmo_renderer.cpp | 2 +- src/ui/auth_screen.cpp | 5 +++-- 6 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/core/application.cpp b/src/core/application.cpp index 3557cdfa..3ceaf76d 100644 --- a/src/core/application.cpp +++ b/src/core/application.cpp @@ -5606,7 +5606,7 @@ void Application::buildCreatureDisplayLookups() { // Resolve gryphon/wyvern display IDs by exact model path so taxi mounts have textures. auto toLower = [](std::string s) { - for (char& c : s) c = static_cast(::tolower(c)); + for (char& c : s) c = static_cast(std::tolower(static_cast(c))); return s; }; auto normalizePath = [&](const std::string& p) { @@ -8251,7 +8251,7 @@ void Application::spawnOnlineGameObject(uint64_t guid, uint32_t entry, uint32_t if (basePath.size() > 4) { extension = basePath.substr(basePath.size() - 4); std::string extLower = extension; - for (char& c : extLower) c = static_cast(std::tolower(c)); + for (char& c : extLower) c = static_cast(std::tolower(static_cast(c))); if (extLower == ".wmo") { basePath = basePath.substr(0, basePath.size() - 4); } @@ -8423,7 +8423,8 @@ void Application::spawnOnlineGameObject(uint64_t guid, uint32_t entry, uint32_t // Freeze animation for static gameobjects, but let portals/effects/transports animate bool isTransportGO = gameHandler && gameHandler->isTransportGuid(guid); std::string lowerPath = modelPath; - std::transform(lowerPath.begin(), lowerPath.end(), lowerPath.begin(), ::tolower); + std::transform(lowerPath.begin(), lowerPath.end(), lowerPath.begin(), + [](unsigned char c) { return static_cast(std::tolower(c)); }); bool isAnimatedEffect = (lowerPath.find("instanceportal") != std::string::npos || lowerPath.find("instancenewportal") != std::string::npos || lowerPath.find("portalfx") != std::string::npos || diff --git a/src/pipeline/mpq_manager.cpp b/src/pipeline/mpq_manager.cpp index c6ab2264..c98e372b 100644 --- a/src/pipeline/mpq_manager.cpp +++ b/src/pipeline/mpq_manager.cpp @@ -310,7 +310,7 @@ std::vector MPQManager::readFile(const std::string& filename) const { std::string entryName = entry.path().filename().string(); // Case-insensitive comparison if (std::equal(comp.begin(), comp.end(), entryName.begin(), entryName.end(), - [](char a, char b) { return std::tolower(a) == std::tolower(b); })) { + [](unsigned char a, unsigned char b) { return std::tolower(a) == std::tolower(b); })) { searchPath = entry.path(); found = true; break; diff --git a/src/rendering/character_renderer.cpp b/src/rendering/character_renderer.cpp index 78c177df..b0b0602a 100644 --- a/src/rendering/character_renderer.cpp +++ b/src/rendering/character_renderer.cpp @@ -1002,7 +1002,7 @@ VkTexture* CharacterRenderer::compositeTextures(const std::vector& int dstX = 0, dstY = 0; int expectedW256 = 0, expectedH256 = 0; // Expected size at 256-base std::string pathLower = layerPaths[layer]; - for (auto& c : pathLower) c = std::tolower(c); + for (auto& c : pathLower) c = static_cast(std::tolower(static_cast(c))); if (pathLower.find("faceupper") != std::string::npos) { dstX = faceUpperRegion256.x; dstY = faceUpperRegion256.y; @@ -1220,7 +1220,7 @@ VkTexture* CharacterRenderer::compositeWithRegions(const std::string& basePath, // WoW 256-scale atlas coordinates (from CharComponentTextureSections) int dstX = 0, dstY = 0; std::string pathLower = ul; - for (auto& c : pathLower) c = std::tolower(c); + for (auto& c : pathLower) c = static_cast(std::tolower(static_cast(c))); // Scale factor from 256-base coordinates to actual canvas size int coordScale = width / 256; diff --git a/src/rendering/terrain_manager.cpp b/src/rendering/terrain_manager.cpp index 8ec00b92..312f9168 100644 --- a/src/rendering/terrain_manager.cpp +++ b/src/rendering/terrain_manager.cpp @@ -534,7 +534,7 @@ std::shared_ptr TerrainManager::prepareTile(int x, int y) { if (basePath.size() > 4) { extension = basePath.substr(basePath.size() - 4); std::string extLower = extension; - for (char& c : extLower) c = std::tolower(c); + for (char& c : extLower) c = static_cast(std::tolower(static_cast(c))); if (extLower == ".wmo") { basePath = basePath.substr(0, basePath.size() - 4); } @@ -612,7 +612,7 @@ std::shared_ptr TerrainManager::prepareTile(int x, int y) { if (m2Path.size() > 4) { std::string ext = m2Path.substr(m2Path.size() - 4); - for (char& c : ext) c = std::tolower(c); + for (char& c : ext) c = static_cast(std::tolower(static_cast(c))); if (ext == ".mdx" || ext == ".mdl") { m2Path = m2Path.substr(0, m2Path.size() - 4) + ".m2"; } diff --git a/src/rendering/wmo_renderer.cpp b/src/rendering/wmo_renderer.cpp index 4f827778..633e69bf 100644 --- a/src/rendering/wmo_renderer.cpp +++ b/src/rendering/wmo_renderer.cpp @@ -773,7 +773,7 @@ bool WMORenderer::loadModel(const pipeline::WMOModel& model, uint32_t id) { // Convert .mdx/.mdl to .m2 if (m2Path.size() > 4) { std::string ext = m2Path.substr(m2Path.size() - 4); - for (char& c : ext) c = std::tolower(c); + for (char& c : ext) c = static_cast(std::tolower(static_cast(c))); if (ext == ".mdx" || ext == ".mdl") { m2Path = m2Path.substr(0, m2Path.size() - 4) + ".m2"; } diff --git a/src/ui/auth_screen.cpp b/src/ui/auth_screen.cpp index 295739ed..4c4b36ba 100644 --- a/src/ui/auth_screen.cpp +++ b/src/ui/auth_screen.cpp @@ -454,8 +454,9 @@ void AuthScreen::render(auth::AuthHandler& authHandler) { if (!usingStoredHash) { std::string upperUser = username; std::string upperPass = password; - std::transform(upperUser.begin(), upperUser.end(), upperUser.begin(), ::toupper); - std::transform(upperPass.begin(), upperPass.end(), upperPass.begin(), ::toupper); + auto toUp = [](unsigned char c) { return static_cast(std::toupper(c)); }; + std::transform(upperUser.begin(), upperUser.end(), upperUser.begin(), toUp); + std::transform(upperPass.begin(), upperPass.end(), upperPass.begin(), toUp); std::string combined = upperUser + ":" + upperPass; auto hash = auth::Crypto::sha1(combined); savedPasswordHash = hexEncode(hash);