Merge pull request #9321 from Pokechu22/sw-copyregion

Software: Fix out of bounds accesses in CopyRegion

Source/Core/VideoBackends/Software/CopyRegion.h

@@ -1,33 +1,53 @@
 #pragma once
 
+#include "Common/Assert.h"
 #include "Common/MathUtil.h"
 
 #include <cmath>
 
 namespace SW
 {
-// Modified from
+// Copies a region of source to a region of destination, performing nearest-neighbor rescaling
-// http://tech-algorithm.com/articles/nearest-neighbor-image-scaling/
 template <typename T>
-void CopyRegion(const T* const source, const MathUtil::Rectangle<int>& srcrect, T* destination,
+void CopyRegion(const T* const source, const MathUtil::Rectangle<int>& srcrect, const int src_width,
-                const MathUtil::Rectangle<int>& dstrect)
+                const int src_height, T* destination, const MathUtil::Rectangle<int>& dstrect,
+                const int dst_width, const int dst_height)
 {
+  ASSERT(srcrect.top >= 0 && srcrect.bottom <= src_height);
+  ASSERT(srcrect.left >= 0 && srcrect.right <= src_width);
+  ASSERT(dstrect.top >= 0 && dstrect.bottom <= dst_height);
+  ASSERT(dstrect.left >= 0 && dstrect.right <= dst_width);
+
+  int copy_width = dstrect.GetWidth();
+  int copy_height = dstrect.GetHeight();
+
   double x_ratio = srcrect.GetWidth() / static_cast<double>(dstrect.GetWidth());
   double y_ratio = srcrect.GetHeight() / static_cast<double>(dstrect.GetHeight());
-  for (int i = 0; i < dstrect.GetHeight(); i++)
+
+  for (int y_off = 0; y_off < copy_height; y_off++)
   {
-    for (int j = 0; j < dstrect.GetWidth(); j++)
+    for (int x_off = 0; x_off < copy_width; x_off++)
     {
-      int destination_x = j + dstrect.left;
+      int dst_x = dstrect.left + x_off;
-      int destination_y = i + dstrect.top;
+      int dst_y = dstrect.top + y_off;
-      int destination_offset = (destination_y * dstrect.GetWidth()) + destination_x;
+      int dst_offset = (dst_y * dst_width) + dst_x;
 
-      double src_x = std::round(destination_x * x_ratio) + srcrect.left;
+      int src_x = srcrect.left + static_cast<int>(std::round(x_off * x_ratio));
-      double src_y = std::round(destination_y * y_ratio) + srcrect.top;
+      int src_y = srcrect.top + static_cast<int>(std::round(y_off * y_ratio));
-      int src_offset = static_cast<int>((src_y * srcrect.GetWidth()) + src_x);
+      int src_offset = (src_y * src_width) + src_x;
 
-      destination[destination_offset] = source[src_offset];
+      destination[dst_offset] = source[src_offset];
     }
   }
 }
+
+// Copies the entire image from source to destination, performing nearest-neighbor rescaling
+template <typename T>
+void CopyRegion(const T* const source, const int src_width, const int src_height, T* destination,
+                const int dst_width, const int dst_height)
+{
+  MathUtil::Rectangle<int> srcrect{0, 0, src_width, src_height};
+  MathUtil::Rectangle<int> dstrect{0, 0, dst_width, dst_height};
+  CopyRegion(source, srcrect, src_width, src_height, destination, dstrect, dst_width, dst_height);
+}
 }  // namespace SW

Source/Core/VideoBackends/Software/EfbInterface.cpp

@@ -634,17 +634,13 @@ void EncodeXFB(u8* xfb_in_ram, u32 memory_stride, const MathUtil::Rectangle<int>
     src_ptr += memory_stride;
   }
 
-  auto dest_rect =
+  const int src_width = source_rect.GetWidth();
-      MathUtil::Rectangle<int>{source_rect.left, source_rect.top, source_rect.right,
+  const int src_height = source_rect.GetHeight();
-                               static_cast<int>(static_cast<float>(source_rect.bottom) * y_scale)};
+  const int dst_width = src_width;
+  const int dst_height = src_height * y_scale;
 
-  const std::size_t destination_size = dest_rect.GetWidth() * dest_rect.GetHeight() * 2;
+  SW::CopyRegion(source.data(), src_width, src_height, reinterpret_cast<yuv422_packed*>(xfb_in_ram),
-  static std::vector<yuv422_packed> destination;
+                 dst_width, dst_height);
-  destination.resize(dest_rect.GetWidth() * dest_rect.GetHeight());
-
-  SW::CopyRegion(source.data(), source_rect, destination.data(), dest_rect);
-
-  memcpy(xfb_in_ram, destination.data(), destination_size);
 }
 
 bool ZCompare(u16 x, u16 y, u32 z)

Source/Core/VideoBackends/Software/SWTexture.cpp

@@ -56,15 +56,10 @@ void SWRenderer::ScaleTexture(AbstractFramebuffer* dst_framebuffer,
   const SWTexture* software_source_texture = static_cast<const SWTexture*>(src_texture);
   SWTexture* software_dest_texture = static_cast<SWTexture*>(dst_framebuffer->GetColorAttachment());
 
-  std::vector<Pixel> source_pixels;
+  CopyRegion(reinterpret_cast<const Pixel*>(software_source_texture->GetData()), src_rect,
-  source_pixels.resize(src_rect.GetHeight() * src_rect.GetWidth() * 4);
+             src_texture->GetWidth(), src_texture->GetHeight(),
-  memcpy(source_pixels.data(), software_source_texture->GetData(), source_pixels.size());
+             reinterpret_cast<Pixel*>(software_dest_texture->GetData()), dst_rect,
-
+             dst_framebuffer->GetWidth(), dst_framebuffer->GetHeight());
-  std::vector<Pixel> destination_pixels;
-  destination_pixels.resize(dst_rect.GetHeight() * dst_rect.GetWidth() * 4);
-
-  CopyRegion(source_pixels.data(), src_rect, destination_pixels.data(), dst_rect);
-  memcpy(software_dest_texture->GetData(), destination_pixels.data(), destination_pixels.size());
 }
 
 SWTexture::SWTexture(const TextureConfig& tex_config) : AbstractTexture(tex_config)