Merge pull request #14416 from JosJuice/game-id-ascii

DiscIO: Only allow alphanumeric ASCII in game IDs

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());

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;

Source/Core/DiscIO/Volume.cpp

@@ -3,10 +3,13 @@
 
 #include "DiscIO/Volume.h"
 
+#include <cstring>
 #include <functional>
 #include <map>
 #include <memory>
 #include <optional>
+#include <ranges>
+#include <span>
 #include <string>
 #include <type_traits>
 #include <utility>
@@ -34,6 +37,25 @@ const IOS::ES::TicketReader Volume::INVALID_TICKET{};
 const IOS::ES::TMDReader Volume::INVALID_TMD{};
 const std::vector<u8> Volume::INVALID_CERT_CHAIN{};
 
+std::string Volume::DecodeString(std::span<const char> data) const
+{
+  // strnlen to trim null bytes
+  std::string string(data.data(), strnlen(data.data(), data.size()));
+  return GetRegion() == Region::NTSC_J ? SHIFTJISToUTF8(string) : CP1252ToUTF8(string);
+}
+
+std::string Volume::FilterGameID(std::span<const char> 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 <typename T>
 static void AddToSyncHash(Common::SHA1::Context* context, const T& data)
 {

Source/Core/DiscIO/Volume.h

@@ -3,7 +3,6 @@
 
 #pragma once
 
-#include <cstring>
 #include <limits>
 #include <map>
 #include <memory>
@@ -14,7 +13,6 @@
 
 #include "Common/CommonTypes.h"
 #include "Common/Crypto/SHA1.h"
-#include "Common/StringUtil.h"
 #include "Common/Swap.h"
 #include "Core/IOS/ES/Formats.h"
 #include "DiscIO/Enums.h"
@@ -144,16 +142,8 @@ public:
   virtual std::array<u8, 20> GetSyncHash() const = 0;
 
 protected:
-  std::string DecodeString(std::span<const char> data) const
+  std::string DecodeString(std::span<const char> data) const;
-  {
+  static std::string FilterGameID(std::span<const char> data);
-    // strnlen to trim NULLs
-    std::string string(data.data(), strnlen(data.data(), data.size()));
-
-    if (GetRegion() == Region::NTSC_J)
-      return SHIFTJISToUTF8(string);
-
-    return CP1252ToUTF8(string);
-  }
 
   void ReadAndAddToSyncHash(Common::SHA1::Context* context, u64 offset, u64 length,
                             const Partition& partition) const;

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<std::size_t>(maker_id.size(), 2));
 
-    return DecodeString(id);
+    return FilterGameID(id);
   }
 
   if (!Read(0, sizeof(id), reinterpret_cast<u8*>(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<u8*>(&maker_id), partition))
     return std::string();
 
-  return DecodeString(maker_id);
+  return FilterGameID(maker_id);
 }
 
 std::optional<u16> VolumeDisc::GetRevision(const Partition& partition) const

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<u64> VolumeWAD::GetTitleID(const Partition& partition) const