Merge pull request #4447 from JosJuice/hthh-hle-issues

Fix exploitable HLE problems reported by hthh

Source/Core/Common/FileUtil.cpp

@@ -5,6 +5,7 @@
 #include <algorithm>
 #include <cstddef>
 #include <cstdio>
+#include <cstdlib>
 #include <cstring>
 #include <fcntl.h>
 #include <limits.h>
@@ -30,7 +31,6 @@
 #include <dirent.h>
 #include <errno.h>
 #include <libgen.h>
-#include <stdlib.h>
 #include <unistd.h>
 #endif
 
@@ -712,6 +712,19 @@ std::string GetBundleDirectory()
 }
 #endif
 
+std::string GetAbsolutePath(const std::string& path)
+{
+#ifdef _WIN32
+  wchar_t absolute_path[_MAX_PATH];
+  wchar_t* result = _wfullpath(absolute_path, UTF8ToTStr(path).c_str(), _MAX_PATH);
+  return result ? TStrToUTF8(result) : "";
+#else
+  char absolute_path[MAX_PATH + 1];
+  char* result = realpath(path.c_str(), absolute_path);
+  return result ? result : "";
+#endif
+}
+
 std::string& GetExeDirectory()
 {
   static std::string DolphinPath;

Source/Core/Common/FileUtil.h

@@ -133,6 +133,10 @@ std::string CreateTempDir();
 // Get a filename that can hopefully be atomically renamed to the given path.
 std::string GetTempFilenameForAtomicWrite(const std::string& path);
 
+// Converts the given path into an absolute path.
+// An empty string is returned if an error occurs.
+std::string GetAbsolutePath(const std::string& path);
+
 // Gets a set user directory path
 // Don't call prior to setting the base user directory
 const std::string& GetUserPath(unsigned int dir_index);

Source/Core/Core/HLE/HLE_OS.cpp

@@ -87,8 +87,14 @@ std::string GetStringVA(u32 strReg)
         result += '%';
         continue;
       }
-      while (string[i] < 'A' || string[i] > 'z' || string[i] == 'l' || string[i] == '-')
+
+      while (i < string.size() &&
+             (string[i] < 'A' || string[i] > 'z' || string[i] == 'l' || string[i] == '-'))
+      {
         ArgumentBuffer += string[i++];
+      }
+      if (i >= string.size())
+        break;
 
       ArgumentBuffer += string[i];
 
@@ -137,6 +143,11 @@ std::string GetStringVA(u32 strReg)
         result += StringFromFormat("%x", (u32)Parameter);
         break;
 
+      case 'n':
+        PowerPC::HostWrite_U32(static_cast<u32>(result.size()), static_cast<u32>(Parameter));
+        // %n doesn't output anything, so the result variable is untouched
+        break;
+
       default:
         result += StringFromFormat(ArgumentBuffer.c_str(), Parameter);
         break;

Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp

@@ -8,6 +8,8 @@
 #include "Common/CommonPaths.h"
 #include "Common/CommonTypes.h"
 #include "Common/FileUtil.h"
+#include "Common/Logging/Log.h"
+#include "Common/MsgHandler.h"
 #include "Common/NandPaths.h"
 #include "Common/StringUtil.h"
 
@@ -23,8 +25,6 @@ static std::map<std::string, std::weak_ptr<File::IOFile>> openFiles;
 // This is used by several of the FileIO and /dev/fs functions
 std::string HLE_IPC_BuildFilename(std::string path_wii)
 {
-  std::string path_full = File::GetUserPath(D_SESSION_WIIROOT_IDX);
-
   // Replaces chars that FAT32 can't support with strings defined in /sys/replace
   for (auto& replacement : replacements)
   {
@@ -32,9 +32,30 @@ std::string HLE_IPC_BuildFilename(std::string path_wii)
       path_wii.replace(j, 1, replacement.second);
   }
 
-  path_full += path_wii;
+  const std::string root_path = File::GetUserPath(D_SESSION_WIIROOT_IDX);
+  const std::string full_path = root_path + path_wii;
 
-  return path_full;
+  const std::string absolute_root_path = File::GetAbsolutePath(root_path);
+  const std::string absolute_full_path = File::GetAbsolutePath(full_path);
+  if (absolute_root_path.empty() || absolute_full_path.empty())
+  {
+    PanicAlert("IOS HLE: Couldn't get an absolute path; the root directory will be returned. "
+               "This will most likely lead to failures.");
+    return root_path;
+  }
+
+  if (path_wii.empty() || path_wii[0] != '/' ||
+      absolute_full_path.compare(0, absolute_root_path.size(), absolute_root_path) != 0)
+  {
+    // Prevent the emulated system from accessing files that aren't in the NAND directory.
+    // (Emulated software that tries to exploit Dolphin might access a path like "/../..".)
+    WARN_LOG(WII_IPC_FILEIO,
+             "The emulated software tried to access a file outside of the NAND directory: %s",
+             absolute_full_path.c_str());
+    return root_path;
+  }
+
+  return full_path;
 }
 
 void HLE_IPC_CreateVirtualFATFilesystem()

Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_net.cpp

@@ -2,7 +2,9 @@
 // Licensed under GPLv2+
 // Refer to the license.txt file included.
 
+#include <algorithm>
 #include <cinttypes>
+#include <cstddef>
 #include <cstdio>
 #include <cstdlib>
 #include <string>
@@ -14,6 +16,7 @@
 
 #include "Common/CommonPaths.h"
 #include "Common/FileUtil.h"
+#include "Common/Logging/Log.h"
 #include "Common/NandPaths.h"
 #include "Common/Network.h"
 #include "Common/SettingsHandler.h"
@@ -848,9 +851,16 @@ IPCCommandResult CWII_IPC_HLE_Device_net_ip_top::IOCtl(u32 _CommandAddress)
     sa_len = sizeof(sa);
     int ret = getsockname(fd, &sa, &sa_len);
 
-    Memory::Write_U8(BufferOutSize, BufferOut);
+    if (BufferOutSize < 2 + sizeof(sa.sa_data))
-    Memory::Write_U8(sa.sa_family & 0xFF, BufferOut + 1);
+      WARN_LOG(WII_IPC_NET, "IOCTL_SO_GETSOCKNAME output buffer is too small. Truncating");
-    Memory::CopyToEmu(BufferOut + 2, &sa.sa_data, BufferOutSize - 2);
+
+    if (BufferOutSize > 0)
+      Memory::Write_U8(BufferOutSize, BufferOut);
+    if (BufferOutSize > 1)
+      Memory::Write_U8(sa.sa_family & 0xFF, BufferOut + 1);
+    if (BufferOutSize > 2)
+      Memory::CopyToEmu(BufferOut + 2, &sa.sa_data,
+                        std::min<size_t>(sizeof(sa.sa_data), BufferOutSize - 2));
     ReturnValue = ret;
     break;
   }
@@ -864,9 +874,16 @@ IPCCommandResult CWII_IPC_HLE_Device_net_ip_top::IOCtl(u32 _CommandAddress)
 
     int ret = getpeername(fd, &sa, &sa_len);
 
-    Memory::Write_U8(BufferOutSize, BufferOut);
+    if (BufferOutSize < 2 + sizeof(sa.sa_data))
-    Memory::Write_U8(AF_INET, BufferOut + 1);
+      WARN_LOG(WII_IPC_NET, "IOCTL_SO_GETPEERNAME output buffer is too small. Truncating");
-    Memory::CopyToEmu(BufferOut + 2, &sa.sa_data, BufferOutSize - 2);
+
+    if (BufferOutSize > 0)
+      Memory::Write_U8(BufferOutSize, BufferOut);
+    if (BufferOutSize > 1)
+      Memory::Write_U8(AF_INET, BufferOut + 1);
+    if (BufferOutSize > 2)
+      Memory::CopyToEmu(BufferOut + 2, &sa.sa_data,
+                        std::min<size_t>(sizeof(sa.sa_data), BufferOutSize - 2));
 
     INFO_LOG(WII_IPC_NET, "IOCTL_SO_GETPEERNAME(%x)", fd);
 

Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_sdio_slot0.cpp

@@ -44,7 +44,7 @@ void CWII_IPC_HLE_Device_sdio_slot0::DoState(PointerWrap& p)
   p.Do(m_Status);
   p.Do(m_BlockLength);
   p.Do(m_BusWidth);
-  p.Do(m_Registers);
+  p.Do(m_registers);
 }
 
 void CWII_IPC_HLE_Device_sdio_slot0::EventNotify()
@@ -87,7 +87,7 @@ IPCCommandResult CWII_IPC_HLE_Device_sdio_slot0::Open(u32 _CommandAddress, u32 _
   OpenInternal();
 
   Memory::Write_U32(GetDeviceID(), _CommandAddress + 0x4);
-  memset(m_Registers, 0, sizeof(m_Registers));
+  m_registers.fill(0);
   m_Active = true;
   return GetDefaultReply();
 }
@@ -130,7 +130,7 @@ IPCCommandResult CWII_IPC_HLE_Device_sdio_slot0::IOCtl(u32 _CommandAddress)
 
     INFO_LOG(WII_IPC_SD, "IOCTL_WRITEHCR 0x%08x - 0x%08x", reg, val);
 
-    if (reg >= 0x200)
+    if (reg >= m_registers.size())
     {
       WARN_LOG(WII_IPC_SD, "IOCTL_WRITEHCR out of range");
       break;
@@ -139,17 +139,17 @@ IPCCommandResult CWII_IPC_HLE_Device_sdio_slot0::IOCtl(u32 _CommandAddress)
     if ((reg == HCR_CLOCKCONTROL) && (val & 1))
     {
       // Clock is set to oscillate, enable bit 1 to say it's stable
-      m_Registers[reg] = val | 2;
+      m_registers[reg] = val | 2;
     }
     else if ((reg == HCR_SOFTWARERESET) && val)
     {
       // When a reset is specified, the register gets cleared
-      m_Registers[reg] = 0;
+      m_registers[reg] = 0;
     }
     else
     {
       // Default to just storing the new value
-      m_Registers[reg] = val;
+      m_registers[reg] = val;
     }
   }
   break;
@@ -158,13 +158,13 @@ IPCCommandResult CWII_IPC_HLE_Device_sdio_slot0::IOCtl(u32 _CommandAddress)
   {
     u32 reg = Memory::Read_U32(BufferIn);
 
-    if (reg >= 0x200)
+    if (reg >= m_registers.size())
     {
       WARN_LOG(WII_IPC_SD, "IOCTL_READHCR out of range");
       break;
     }
 
-    u32 val = m_Registers[reg];
+    u32 val = m_registers[reg];
     INFO_LOG(WII_IPC_SD, "IOCTL_READHCR 0x%08x - 0x%08x", reg, val);
 
     // Just reading the register

Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_sdio_slot0.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <array>
 #include <string>
 #include "Core/IPC_HLE/WII_IPC_HLE_Device.h"
 
@@ -119,7 +120,7 @@ private:
   u32 m_BlockLength;
   u32 m_BusWidth;
 
-  u32 m_Registers[0x200 / 4];
+  std::array<u32, 0x200 / sizeof(u32)> m_registers;
 
   File::IOFile m_Card;