Merge pull request #10592 from AdmiralCurtiss/pointerwrap-protections

Common/PointerWrap: Prevent reads/writes past the end of the buffer.

Source/Core/Common/ChunkFile.h

@@ -46,11 +46,17 @@ public:
     MODE_VERIFY,    // compare
   };
 
-  u8** ptr;
+private:
+  u8** m_ptr_current;
+  u8* m_ptr_end;
   Mode mode;
 
 public:
-  PointerWrap(u8** ptr_, Mode mode_) : ptr(ptr_), mode(mode_) {}
+  PointerWrap(u8** ptr, size_t size, Mode mode_)
+      : m_ptr_current(ptr), m_ptr_end(*ptr + size), mode(mode_)
+  {
+  }
+
   void SetMode(Mode mode_) { mode = mode_; }
   Mode GetMode() const { return mode; }
   template <typename K, class V>
@@ -208,8 +214,13 @@ public:
   [[nodiscard]] u8* DoExternal(u32& count)
   {
     Do(count);
-    u8* current = *ptr;
-    *ptr += count;
+    u8* current = *m_ptr_current;
+    *m_ptr_current += count;
+    if (mode != MODE_MEASURE && *m_ptr_current > m_ptr_end)
+    {
+      // trying to read/write past the end of the buffer, prevent this
+      mode = MODE_MEASURE;
+    }
     return current;
   }
 
@@ -319,26 +330,32 @@ private:
 
   DOLPHIN_FORCE_INLINE void DoVoid(void* data, u32 size)
   {
+    if (mode != MODE_MEASURE && (*m_ptr_current + size) > m_ptr_end)
+    {
+      // trying to read/write past the end of the buffer, prevent this
+      mode = MODE_MEASURE;
+    }
+
     switch (mode)
     {
     case MODE_READ:
-      memcpy(data, *ptr, size);
+      memcpy(data, *m_ptr_current, size);
       break;
 
     case MODE_WRITE:
-      memcpy(*ptr, data, size);
+      memcpy(*m_ptr_current, data, size);
       break;
 
     case MODE_MEASURE:
       break;
 
     case MODE_VERIFY:
-      DEBUG_ASSERT_MSG(COMMON, !memcmp(data, *ptr, size),
+      DEBUG_ASSERT_MSG(COMMON, !memcmp(data, *m_ptr_current, size),
                        "Savestate verification failure: buf {} != {} (size {}).\n", fmt::ptr(data),
-                       fmt::ptr(*ptr), size);
+                       fmt::ptr(*m_ptr_current), size);
       break;
     }
 
-    *ptr += size;
+    *m_ptr_current += size;
   }
 };

Source/Core/Core/IOS/Network/Socket.cpp

@@ -994,8 +994,8 @@ void WiiSockMan::Convert(sockaddr_in const& from, WiiSockAddrIn& to, s32 addrlen
 
 void WiiSockMan::DoState(PointerWrap& p)
 {
-  bool saving =
-      p.mode == PointerWrap::Mode::MODE_WRITE || p.mode == PointerWrap::Mode::MODE_MEASURE;
+  bool saving = p.GetMode() == PointerWrap::Mode::MODE_WRITE ||
+                p.GetMode() == PointerWrap::Mode::MODE_MEASURE;
   auto size = pending_polls.size();
   p.Do(size);
   if (!saving)

Source/Core/Core/State.cpp

@@ -225,8 +225,8 @@ void LoadFromBuffer(std::vector<u8>& buffer)
 
   Core::RunOnCPUThread(
       [&] {
-        u8* ptr = &buffer[0];
-        PointerWrap p(&ptr, PointerWrap::MODE_READ);
+        u8* ptr = buffer.data();
+        PointerWrap p(&ptr, buffer.size(), PointerWrap::MODE_READ);
         DoState(p);
       },
       true);
@@ -237,14 +237,14 @@ void SaveToBuffer(std::vector<u8>& buffer)
   Core::RunOnCPUThread(
       [&] {
         u8* ptr = nullptr;
-        PointerWrap p(&ptr, PointerWrap::MODE_MEASURE);
+        PointerWrap p_measure(&ptr, 0, PointerWrap::MODE_MEASURE);
 
-        DoState(p);
+        DoState(p_measure);
         const size_t buffer_size = reinterpret_cast<size_t>(ptr);
         buffer.resize(buffer_size);
 
-        ptr = &buffer[0];
-        p.SetMode(PointerWrap::MODE_WRITE);
+        ptr = buffer.data();
+        PointerWrap p(&ptr, buffer_size, PointerWrap::MODE_WRITE);
         DoState(p);
       },
       true);
@@ -412,20 +412,22 @@ void SaveAs(const std::string& filename, bool wait)
       [&] {
         // Measure the size of the buffer.
         u8* ptr = nullptr;
-        PointerWrap p(&ptr, PointerWrap::MODE_MEASURE);
-        DoState(p);
+        PointerWrap p_measure(&ptr, 0, PointerWrap::MODE_MEASURE);
+        DoState(p_measure);
         const size_t buffer_size = reinterpret_cast<size_t>(ptr);
 
         // Then actually do the write.
+        PointerWrap::Mode p_mode;
         {
           std::lock_guard lk(g_cs_current_buffer);
           g_current_buffer.resize(buffer_size);
-          ptr = &g_current_buffer[0];
-          p.SetMode(PointerWrap::MODE_WRITE);
+          ptr = g_current_buffer.data();
+          PointerWrap p(&ptr, buffer_size, PointerWrap::MODE_WRITE);
           DoState(p);
+          p_mode = p.GetMode();
         }
 
-        if (p.GetMode() == PointerWrap::MODE_WRITE)
+        if (p_mode == PointerWrap::MODE_WRITE)
         {
           Core::DisplayMessage("Saving State...", 1000);
 
@@ -588,8 +590,8 @@ void LoadAs(const std::string& filename)
 
           if (!buffer.empty())
           {
-            u8* ptr = &buffer[0];
-            PointerWrap p(&ptr, PointerWrap::MODE_READ);
+            u8* ptr = buffer.data();
+            PointerWrap p(&ptr, buffer.size(), PointerWrap::MODE_READ);
             DoState(p);
             loaded = true;
             loadedSuccessfully = (p.GetMode() == PointerWrap::MODE_READ);

Source/Core/UICommon/GameFileCache.cpp

@@ -230,14 +230,14 @@ bool GameFileCache::SyncCacheFile(bool save)
   {
     // Measure the size of the buffer.
     u8* ptr = nullptr;
-    PointerWrap p(&ptr, PointerWrap::MODE_MEASURE);
-    DoState(&p);
+    PointerWrap p_measure(&ptr, 0, PointerWrap::MODE_MEASURE);
+    DoState(&p_measure);
     const size_t buffer_size = reinterpret_cast<size_t>(ptr);
 
     // Then actually do the write.
     std::vector<u8> buffer(buffer_size);
-    ptr = &buffer[0];
-    p.SetMode(PointerWrap::MODE_WRITE);
+    ptr = buffer.data();
+    PointerWrap p(&ptr, buffer_size, PointerWrap::MODE_WRITE);
     DoState(&p, buffer_size);
     if (f.WriteBytes(buffer.data(), buffer.size()))
       success = true;
@@ -248,7 +248,7 @@ bool GameFileCache::SyncCacheFile(bool save)
     if (!buffer.empty() && f.ReadBytes(buffer.data(), buffer.size()))
     {
       u8* ptr = buffer.data();
-      PointerWrap p(&ptr, PointerWrap::MODE_READ);
+      PointerWrap p(&ptr, buffer.size(), PointerWrap::MODE_READ);
       DoState(&p, buffer.size());
       if (p.GetMode() == PointerWrap::MODE_READ)
         success = true;

Source/Core/VideoCommon/CPMemory.cpp

@@ -26,7 +26,7 @@ void DoCPState(PointerWrap& p)
   p.Do(g_main_cp_state.vtx_desc);
   p.DoArray(g_main_cp_state.vtx_attr);
   p.DoMarker("CP Memory");
-  if (p.mode == PointerWrap::MODE_READ)
+  if (p.GetMode() == PointerWrap::MODE_READ)
   {
     CopyPreprocessCPStateFromMain();
     VertexLoaderManager::g_bases_dirty = true;

Source/Core/VideoCommon/Fifo.cpp

@@ -94,7 +94,7 @@ void DoState(PointerWrap& p)
   p.DoPointer(write_ptr, s_video_buffer);
   s_video_buffer_write_ptr = write_ptr;
   p.DoPointer(s_video_buffer_read_ptr, s_video_buffer);
-  if (p.mode == PointerWrap::MODE_READ && s_use_deterministic_gpu_thread)
+  if (p.GetMode() == PointerWrap::MODE_READ && s_use_deterministic_gpu_thread)
   {
     // We're good and paused, right?
     s_video_buffer_seen_ptr = s_video_buffer_pp_read_ptr = s_video_buffer_read_ptr;

Source/Core/VideoCommon/FreeLookCamera.cpp

@@ -298,7 +298,7 @@ Common::Vec2 FreeLookCamera::GetFieldOfViewMultiplier() const
 
 void FreeLookCamera::DoState(PointerWrap& p)
 {
-  if (p.mode == PointerWrap::MODE_WRITE || p.mode == PointerWrap::MODE_MEASURE)
+  if (p.GetMode() == PointerWrap::MODE_WRITE || p.GetMode() == PointerWrap::MODE_MEASURE)
   {
     p.Do(m_current_type);
     if (m_camera_controller)

Source/Core/VideoCommon/TextureCacheBase.cpp

@@ -459,6 +459,12 @@ void TextureCacheBase::SerializeTexture(AbstractTexture* tex, const TextureConfi
     // needing to allocate/free an extra buffer.
     u8* texture_data = p.DoExternal(total_size);
 
+    if (p.GetMode() == PointerWrap::MODE_MEASURE)
+    {
+      ERROR_LOG_FMT(VIDEO, "Couldn't acquire {} bytes for serializing texture.", total_size);
+      return;
+    }
+
     if (!skip_readback)
     {
       // Save out each layer of the texture to the pointer.