Merge bd72ae62a2 into 1ae0b23265

Source/Core/Common/JsonUtil.cpp

@@ -60,6 +60,7 @@ bool JsonFromFile(const std::string& filename, picojson::value* root, std::strin
   std::string json_data;
   if (!File::ReadFileToString(filename, json_data))
   {
+    *error = "Failed to read " + filename;
     return false;
   }
 

Source/Core/Core/AchievementManager.cpp

@@ -79,29 +79,29 @@ void AchievementManager::Init()
   }
 }
 
-picojson::value AchievementManager::LoadApprovedList()
+auto AchievementManager::LoadApprovedList() -> std::variant<picojson::value, ErrorString>
 {
   picojson::value temp;
   std::string error;
   if (!JsonFromFile(fmt::format("{}{}{}", File::GetSysDirectory(), DIR_SEP, APPROVED_LIST_FILENAME),
                     &temp, &error))
   {
-    WARN_LOG_FMT(ACHIEVEMENTS, "Failed to load approved game settings list {}",
-                 APPROVED_LIST_FILENAME);
-    WARN_LOG_FMT(ACHIEVEMENTS, "Error: {}", error);
-    return {};
+    error = fmt::format("Failed to load approved game settings list {}. Error: {}",
+                        APPROVED_LIST_FILENAME, error);
+    WARN_LOG_FMT(ACHIEVEMENTS, "{}", error);
+    return error;
   }
   auto context = Common::SHA1::CreateContext();
   context->Update(temp.serialize());
   auto digest = context->Finish();
   if (digest != APPROVED_LIST_HASH)
   {
-    WARN_LOG_FMT(ACHIEVEMENTS, "Failed to verify approved game settings list {}",
-                 APPROVED_LIST_FILENAME);
-    WARN_LOG_FMT(ACHIEVEMENTS, "Expected hash {}, found hash {}",
-                 Common::SHA1::DigestToString(APPROVED_LIST_HASH),
-                 Common::SHA1::DigestToString(digest));
-    return {};
+    error = fmt::format(
+        "Failed to verify approved game settings list {}. Expected hash {}, found hash {}",
+        APPROVED_LIST_FILENAME, Common::SHA1::DigestToString(APPROVED_LIST_HASH),
+        Common::SHA1::DigestToString(digest));
+    WARN_LOG_FMT(ACHIEVEMENTS, "{}", error);
+    return error;
   }
   return temp;
 }
@@ -386,6 +386,15 @@ bool AchievementManager::IsHardcoreModeActive() const
   return rc_client_is_processing_required(m_client);
 }
 
+bool AchievementManager::IsApprovedCodesListValid(std::string* error_out) const
+{
+  std::lock_guard lg{m_lock};
+  const bool is_valid = std::holds_alternative<picojson::value>(*m_ini_root);
+  if (error_out && !is_valid)
+    *error_out = std::get<std::string>(*m_ini_root);
+  return is_valid;
+}
+
 template <typename T>
 void AchievementManager::FilterApprovedIni(std::vector<T>& codes, const std::string& game_id,
                                            u16 revision) const
@@ -402,7 +411,7 @@ void AchievementManager::FilterApprovedIni(std::vector<T>& codes, const std::str
     return;
 
   // Approved codes list failed to hash
-  if (!m_ini_root->is<picojson::value::object>())
+  if (!std::holds_alternative<picojson::value>(*m_ini_root))
   {
     codes.clear();
     return;
@@ -423,11 +432,13 @@ bool AchievementManager::CheckApprovedCode(const T& code, const std::string& gam
     return true;
 
   // Approved codes list failed to hash
-  if (!m_ini_root->is<picojson::value::object>())
+  if (!std::holds_alternative<picojson::value>(*m_ini_root))
     return false;
 
   INFO_LOG_FMT(ACHIEVEMENTS, "Verifying code {}", code.name);
 
+  const picojson::value& ini_root = std::get<picojson::value>(*m_ini_root);
+
   bool verified = false;
 
   auto hash = Common::SHA1::DigestToString(GetCodeHash(code));
@@ -435,7 +446,7 @@ bool AchievementManager::CheckApprovedCode(const T& code, const std::string& gam
   for (const std::string& filename : ConfigLoaders::GetGameIniFilenames(game_id, revision))
   {
     auto config = filename.substr(0, filename.length() - 4);
-    if (m_ini_root->contains(config) && m_ini_root->get(config).contains(hash))
+    if (ini_root.contains(config) && ini_root.get(config).contains(hash))
       verified = true;
   }
 

Source/Core/Core/AchievementManager.h

@@ -18,6 +18,7 @@
 #include <thread>
 #include <unordered_map>
 #include <unordered_set>
+#include <variant>
 #include <vector>
 
 #include <rcheevos/include/rc_api_runtime.h>
@@ -134,6 +135,7 @@ public:
   std::recursive_mutex& GetLock();
   bool IsHardcoreModeActive() const;
 
+  bool IsApprovedCodesListValid(std::string* error_out = nullptr) const;
   void FilterApprovedPatches(std::vector<PatchEngine::Patch>& patches, const std::string& game_id,
                              u16 revision) const;
   void FilterApprovedGeckoCodes(std::vector<Gecko::GeckoCode>& codes, const std::string& game_id,
@@ -176,7 +178,8 @@ private:
     std::unique_ptr<DiscIO::Volume> volume;
   };
 
-  static picojson::value LoadApprovedList();
+  using ErrorString = std::string;
+  static std::variant<picojson::value, ErrorString> LoadApprovedList();
 
   static void* FilereaderOpenByFilepath(const char* path_utf8);
   static void* FilereaderOpenByVolume(const char* path_utf8);
@@ -259,7 +262,7 @@ private:
   std::chrono::steady_clock::time_point m_last_rp_time = std::chrono::steady_clock::now();
   std::chrono::steady_clock::time_point m_last_progress_message = std::chrono::steady_clock::now();
 
-  Common::Lazy<picojson::value> m_ini_root{LoadApprovedList};
+  Common::Lazy<std::variant<picojson::value, ErrorString>> m_ini_root{LoadApprovedList};
 
   std::unordered_map<AchievementId, LeaderboardStatus> m_leaderboard_map;
   bool m_challenges_updated = false;
@@ -302,6 +305,8 @@ public:
 
   constexpr bool IsHardcoreModeActive() { return false; }
 
+  constexpr bool IsApprovedCodesListValid(std::string* error_out = nullptr) { return true; }
+
   constexpr bool CheckApprovedGeckoCode(const Gecko::GeckoCode& code, const std::string& game_id)
   {
     return true;

Source/UnitTests/Core/PatchAllowlistTest.cpp

@@ -18,6 +18,7 @@
 #include "Common/IOFile.h"
 #include "Common/IniFile.h"
 #include "Common/JsonUtil.h"
+#include "Core/AchievementManager.h"
 #include "Core/ActionReplay.h"
 #include "Core/CheatCodes.h"
 #include "Core/GeckoCode.h"
@@ -38,7 +39,14 @@ void ReadVerified(const Common::IniFile& ini, const std::string& filename,
 void CheckHash(const std::string& game_id, GameHashes* game_hashes, const std::string& hash,
                const std::string& patch_name);
 
-TEST(PatchAllowlist, VerifyHashes)
+TEST(PatchAllowlist, VerifyJsonMatchesExecutable)
+{
+  std::string error;
+  if (!AchievementManager::GetInstance().IsApprovedCodesListValid(&error))
+    ADD_FAILURE() << error;
+}
+
+TEST(PatchAllowlist, VerifyInisMatchJson)
 {
   // Load allowlist
   static constexpr std::string_view APPROVED_LIST_FILENAME = "ApprovedInis.json";

Source/UnitTests/UnitTests.vcxproj

@@ -103,6 +103,7 @@
   <Import Project="$(ExternalsDir)Bochs_disasm\exports.props" />
   <Import Project="$(ExternalsDir)fmt\exports.props" />
   <Import Project="$(ExternalsDir)picojson\exports.props" />
+  <Import Project="$(ExternalsDir)rcheevos\exports.props" />
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets">
   </ImportGroup>