From 7b372db5593ddc92dd442e57170f13072b43ade8 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Mon, 23 Feb 2026 20:59:46 +0100 Subject: [PATCH] DiscIO: Only allow alphanumeric ASCII in game IDs We often use game IDs in paths, so we should try to make sure path traversal is impossible in game IDs. Admittedly, doing any kind of real attack using the six bytes available in game IDs is unrealistic, but no game ID should contain non-alphanumeric or non-ASCII characters anyway. Might also fix https://bugs.dolphin-emu.org/issues/13982 by skipping converting between encodings for game IDs. --- Source/Core/Core/IOS/ES/Formats.cpp | 4 ++-- Source/Core/Core/IOS/ES/Formats.h | 4 ++-- Source/Core/DiscIO/Volume.cpp | 13 +++++++++++++ Source/Core/DiscIO/Volume.h | 1 + Source/Core/DiscIO/VolumeDisc.cpp | 10 +++++----- Source/Core/DiscIO/VolumeWad.cpp | 2 +- 6 files changed, 24 insertions(+), 10 deletions(-) diff --git a/Source/Core/Core/IOS/ES/Formats.cpp b/Source/Core/Core/IOS/ES/Formats.cpp index 93d4e6606e..a43ff50de5 100644 --- a/Source/Core/Core/IOS/ES/Formats.cpp +++ b/Source/Core/Core/IOS/ES/Formats.cpp @@ -298,7 +298,7 @@ std::string TMDReader::GetGameID() const std::memcpy(game_id, m_bytes.data() + offsetof(TMDHeader, title_id) + 4, 4); std::memcpy(game_id + 4, m_bytes.data() + offsetof(TMDHeader, group_id), 2); - if (std::ranges::all_of(game_id, Common::IsPrintableCharacter)) + if (std::ranges::all_of(game_id, Common::IsAlnum)) return std::string(game_id, sizeof(game_id)); return fmt::format("{:016x}", GetTitleId()); @@ -309,7 +309,7 @@ std::string TMDReader::GetGameTDBID() const const u8* begin = m_bytes.data() + offsetof(TMDHeader, title_id) + 4; const u8* end = begin + 4; - if (std::all_of(begin, end, Common::IsPrintableCharacter)) + if (std::all_of(begin, end, Common::IsAlnum)) return std::string(begin, end); return fmt::format("{:016x}", GetTitleId()); diff --git a/Source/Core/Core/IOS/ES/Formats.h b/Source/Core/Core/IOS/ES/Formats.h index f1fb1ff782..f431333481 100644 --- a/Source/Core/Core/IOS/ES/Formats.h +++ b/Source/Core/Core/IOS/ES/Formats.h @@ -217,12 +217,12 @@ public: bool IsvWii() const; // Constructs a 6-character game ID in the format typically used by Dolphin. - // If the 6-character game ID would contain unprintable characters, + // If the 6-character game ID would contain non-alphanumeric characters, // the title ID converted to 16 hexadecimal digits is returned instead. std::string GetGameID() const; // Constructs a 4-character game ID in the format typically used by GameTDB. - // If the 4-character game ID would contain unprintable characters, + // If the 4-character game ID would contain non-alphanumeric characters, // the title ID converted to 16 hexadecimal digits is returned instead // (a format which GameTDB does not actually use). std::string GetGameTDBID() const; diff --git a/Source/Core/DiscIO/Volume.cpp b/Source/Core/DiscIO/Volume.cpp index 7ef63f6950..c1942b2aeb 100644 --- a/Source/Core/DiscIO/Volume.cpp +++ b/Source/Core/DiscIO/Volume.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -43,6 +44,18 @@ std::string Volume::DecodeString(std::span data) const return GetRegion() == Region::NTSC_J ? SHIFTJISToUTF8(string) : CP1252ToUTF8(string); } +std::string Volume::FilterGameID(std::span data) +{ + std::string string(data.data(), data.size()); + + // We don't want game IDs to contain characters that are unprintable or might cause path + // traversal. Game IDs normally only contain ASCII uppercase letters and numbers, + // but GNHE5d contains a lowercase letter, so let's allow all ASCII letters and numbers. + std::ranges::replace_if(string, std::not_fn(Common::IsAlnum), '-'); + + return string; +} + template static void AddToSyncHash(Common::SHA1::Context* context, const T& data) { diff --git a/Source/Core/DiscIO/Volume.h b/Source/Core/DiscIO/Volume.h index cdc9a940b0..a001c60c85 100644 --- a/Source/Core/DiscIO/Volume.h +++ b/Source/Core/DiscIO/Volume.h @@ -143,6 +143,7 @@ public: protected: std::string DecodeString(std::span data) const; + static std::string FilterGameID(std::span data); void ReadAndAddToSyncHash(Common::SHA1::Context* context, u64 offset, u64 length, const Partition& partition) const; diff --git a/Source/Core/DiscIO/VolumeDisc.cpp b/Source/Core/DiscIO/VolumeDisc.cpp index ecbb4a9dc9..2ec71b9fe1 100644 --- a/Source/Core/DiscIO/VolumeDisc.cpp +++ b/Source/Core/DiscIO/VolumeDisc.cpp @@ -50,13 +50,13 @@ std::string VolumeDisc::GetGameID(const Partition& partition) const const std::string maker_id{GetMakerID()}; memcpy(id + 4, maker_id.c_str(), std::min(maker_id.size(), 2)); - return DecodeString(id); + return FilterGameID(id); } if (!Read(0, sizeof(id), reinterpret_cast(id), partition)) return std::string(); - return DecodeString(id); + return FilterGameID(id); } Country VolumeDisc::GetCountry(const Partition& partition) const @@ -126,9 +126,9 @@ std::string VolumeDisc::GetMakerID(const Partition& partition) const { case 'S': // SEGA CORPORATION case 'H': // Hitmaker co,ltd - return DecodeString("6E"); + return "6E"; case 'N': // NAMCO CORPORATION - return DecodeString("82"); + return "82"; default: break; } @@ -138,7 +138,7 @@ std::string VolumeDisc::GetMakerID(const Partition& partition) const if (!Read(0x4, sizeof(maker_id), reinterpret_cast(&maker_id), partition)) return std::string(); - return DecodeString(maker_id); + return FilterGameID(maker_id); } std::optional VolumeDisc::GetRevision(const Partition& partition) const diff --git a/Source/Core/DiscIO/VolumeWad.cpp b/Source/Core/DiscIO/VolumeWad.cpp index ff98b55791..44e229e1ce 100644 --- a/Source/Core/DiscIO/VolumeWad.cpp +++ b/Source/Core/DiscIO/VolumeWad.cpp @@ -247,7 +247,7 @@ std::string VolumeWAD::GetMakerID(const Partition& partition) const if (!Common::IsPrintableCharacter(temp[0]) || !Common::IsPrintableCharacter(temp[1])) return "00"; - return DecodeString(temp); + return FilterGameID(temp); } std::optional VolumeWAD::GetTitleID(const Partition& partition) const