From de355a8521d47e8c86dffab0470a8acead1af746 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Thu, 24 Nov 2016 20:57:27 +0100 Subject: [PATCH 1/5] Revert "IOS HLE: Prevent accessing host file system" This reverts commit 141f3bfb3acee73569486258c21e4ed34fc32327. The implementation of getting absolute paths wasn't working on non-Windows systems, which is a huge problem for IOS HLE. --- Source/Core/Common/FileUtil.cpp | 15 +--------- Source/Core/Common/FileUtil.h | 4 --- .../IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp | 29 +++---------------- 3 files changed, 5 insertions(+), 43 deletions(-) diff --git a/Source/Core/Common/FileUtil.cpp b/Source/Core/Common/FileUtil.cpp index acf154f81a..e761f3ade1 100644 --- a/Source/Core/Common/FileUtil.cpp +++ b/Source/Core/Common/FileUtil.cpp @@ -5,7 +5,6 @@ #include #include #include -#include #include #include #include @@ -31,6 +30,7 @@ #include #include #include +#include #include #endif @@ -712,19 +712,6 @@ 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; diff --git a/Source/Core/Common/FileUtil.h b/Source/Core/Common/FileUtil.h index cc0ec5a651..f4a20f623d 100644 --- a/Source/Core/Common/FileUtil.h +++ b/Source/Core/Common/FileUtil.h @@ -133,10 +133,6 @@ 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); diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp index 84802318d4..119f243d95 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp @@ -10,8 +10,6 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" #include "Common/FileUtil.h" -#include "Common/Logging/Log.h" -#include "Common/MsgHandler.h" #include "Common/NandPaths.h" #include "Core/HW/Memmap.h" #include "Core/IPC_HLE/WII_IPC_HLE.h" @@ -25,6 +23,8 @@ static std::map> 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,30 +32,9 @@ std::string HLE_IPC_BuildFilename(std::string path_wii) path_wii.replace(j, 1, replacement.second); } - const std::string root_path = File::GetUserPath(D_SESSION_WIIROOT_IDX); - const std::string full_path = root_path + path_wii; + path_full += path_wii; - 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; + return path_full; } void HLE_IPC_CreateVirtualFATFilesystem() From c74c317ab574779524fe2e20136aed03044d6b3a Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sat, 26 Nov 2016 15:39:00 +0100 Subject: [PATCH 2/5] IOS HLE: More robust escaping of NAND paths Prevents path traversal without needing an absolute path function, and also improves accuracy (character sequences like ../ appear to have no special meaning in IOS). This removes the creation and usage of /sys/replace, because the new escapes are too complicated to all be representable in its format and because no other NAND handling software seems to use /sys/replace. --- Source/Core/Common/NandPaths.cpp | 75 +++++++++++++------ Source/Core/Common/NandPaths.h | 11 ++- Source/Core/Common/StringUtil.cpp | 17 +++++ Source/Core/Common/StringUtil.h | 1 + Source/Core/Core/HW/GCMemcard.h | 19 +---- Source/Core/Core/HW/WiiSaveCrypted.cpp | 23 +----- .../IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp | 20 ++--- .../Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h | 2 +- .../Core/IPC_HLE/WII_IPC_HLE_Device_fs.cpp | 10 +-- 9 files changed, 90 insertions(+), 88 deletions(-) diff --git a/Source/Core/Common/NandPaths.cpp b/Source/Core/Common/NandPaths.cpp index 3dc554dce9..60cb49fa26 100644 --- a/Source/Core/Common/NandPaths.cpp +++ b/Source/Core/Common/NandPaths.cpp @@ -2,10 +2,12 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. +#include #include #include #include #include +#include #include #include "Common/CommonFuncs.h" @@ -117,36 +119,61 @@ bool CheckTitleTIK(u64 _titleID, FromWhichRoot from) return false; } -static void CreateReplacementFile(std::string& filename) +std::string EscapeFileName(const std::string& filename) { - std::ofstream replace; - OpenFStream(replace, filename, std::ios_base::out); - replace << "\" __22__\n"; - replace << "* __2a__\n"; - // replace << "/ __2f__\n"; - replace << ": __3a__\n"; - replace << "< __3c__\n"; - replace << "> __3e__\n"; - replace << "? __3f__\n"; - // replace <<"\\ __5c__\n"; - replace << "| __7c__\n"; + // Prevent paths from containing special names like ., .., ..., ...., and so on + if (std::all_of(filename.begin(), filename.end(), [](char c) { return c == '.'; })) + return ReplaceAll(filename, ".", "__2e__"); + + // Escape all double underscores since we will use double underscores for our escape sequences + std::string filename_with_escaped_double_underscores = ReplaceAll(filename, "__", "__5f____5f__"); + + // Escape all other characters that need to be escaped + static const std::unordered_set chars_to_replace = {'\"', '*', '/', ':', '<', + '>', '?', '\\', '|', '\x7f'}; + std::string result; + result.reserve(filename_with_escaped_double_underscores.size()); + for (char c : filename_with_escaped_double_underscores) + { + if ((c >= 0 && c <= 0x1F) || chars_to_replace.find(c) != chars_to_replace.end()) + result.append(StringFromFormat("__%02x__", c)); + else + result.push_back(c); + } + + return result; } -void ReadReplacements(replace_v& replacements) +std::string EscapePath(const std::string& path) { - replacements.clear(); - const std::string replace_fname = "/sys/replace"; - std::string filename = File::GetUserPath(D_SESSION_WIIROOT_IDX) + replace_fname; + std::vector split_strings; + SplitString(path, '/', split_strings); - if (!File::Exists(filename)) - CreateReplacementFile(filename); + std::vector escaped_split_strings; + escaped_split_strings.reserve(split_strings.size()); + for (const std::string& split_string : split_strings) + escaped_split_strings.push_back(EscapeFileName(split_string)); - std::ifstream f; - OpenFStream(f, filename, std::ios_base::in); - char letter; - std::string replacement; + return JoinStrings(escaped_split_strings, "/"); +} - while (f >> letter >> replacement && replacement.size()) - replacements.emplace_back(letter, replacement); +std::string UnescapeFileName(const std::string& filename) +{ + std::string result = filename; + size_t pos = 0; + + // Replace escape sequences of the format "__3f__" with the ASCII + // character defined by the escape sequence's two hex digits. + while ((pos = result.find("__", pos)) != std::string::npos) + { + u32 character; + if (pos + 6 <= result.size() && result[pos + 4] == '_' && result[pos + 5] == '_') + if (AsciiToHex(result.substr(pos + 2, 2), character)) + result.replace(pos, 6, {static_cast(character)}); + + ++pos; + } + + return result; } } diff --git a/Source/Core/Common/NandPaths.h b/Source/Core/Common/NandPaths.h index b2dc143963..ab15d6f049 100644 --- a/Source/Core/Common/NandPaths.h +++ b/Source/Core/Common/NandPaths.h @@ -15,9 +15,6 @@ static const std::string TITLEID_SYSMENU_STRING = "0000000100000002"; namespace Common { -typedef std::pair replace_t; -typedef std::vector replace_v; - void InitializeWiiRoot(bool use_temporary); void ShutdownWiiRoot(); @@ -33,5 +30,11 @@ std::string GetTitleDataPath(u64 _titleID, FromWhichRoot from); std::string GetTitleContentPath(u64 _titleID, FromWhichRoot from); bool CheckTitleTMD(u64 _titleID, FromWhichRoot from); bool CheckTitleTIK(u64 _titleID, FromWhichRoot from); -void ReadReplacements(replace_v& replacements); + +// Escapes characters that are invalid or have special meanings in the host file system +std::string EscapeFileName(const std::string& filename); +// Escapes characters that are invalid or have special meanings in the host file system +std::string EscapePath(const std::string& path); +// Reverses escaping done by EscapeFileName +std::string UnescapeFileName(const std::string& filename); } diff --git a/Source/Core/Common/StringUtil.cpp b/Source/Core/Common/StringUtil.cpp index 5048446682..6f3567e782 100644 --- a/Source/Core/Common/StringUtil.cpp +++ b/Source/Core/Common/StringUtil.cpp @@ -10,7 +10,9 @@ #include #include #include +#include #include +#include #include #include @@ -328,6 +330,21 @@ void SplitString(const std::string& str, const char delim, std::vector& strings, const std::string& delimiter) +{ + // Check if we can return early, just for speed + if (strings.empty()) + return ""; + + std::stringstream res; + std::copy(strings.begin(), strings.end(), + std::ostream_iterator(res, delimiter.c_str())); + + // Drop the trailing delimiter. + std::string joined = res.str(); + return joined.substr(0, joined.length() - delimiter.length()); +} + std::string TabsToSpaces(int tab_size, const std::string& in) { const std::string spaces(tab_size, ' '); diff --git a/Source/Core/Common/StringUtil.h b/Source/Core/Common/StringUtil.h index 7e4c598d0d..fcc8c34483 100644 --- a/Source/Core/Common/StringUtil.h +++ b/Source/Core/Common/StringUtil.h @@ -106,6 +106,7 @@ bool AsciiToHex(const std::string& _szValue, u32& result); std::string TabsToSpaces(int tab_size, const std::string& in); void SplitString(const std::string& str, char delim, std::vector& output); +std::string JoinStrings(const std::vector& strings, const std::string& delimiter); // "C:/Windows/winhelp.exe" to "C:/Windows/", "winhelp", ".exe" bool SplitPath(const std::string& full_path, std::string* _pPath, std::string* _pFilename, diff --git a/Source/Core/Core/HW/GCMemcard.h b/Source/Core/Core/HW/GCMemcard.h index 6f35a703c3..2c49beefb1 100644 --- a/Source/Core/Core/HW/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard.h @@ -157,24 +157,7 @@ struct DEntry { std::string filename = std::string((char*)Makercode, 2) + '-' + std::string((char*)Gamecode, 4) + '-' + (char*)Filename + ".gci"; - static Common::replace_v replacements; - if (replacements.size() == 0) - { - Common::ReadReplacements(replacements); - // Cannot add \r to replacements file due to it being a line ending char - // / might be ok, but we need to verify that this is only used on filenames - // as it is a dir_sep - replacements.push_back(std::make_pair('\r', std::string("__0d__"))); - replacements.push_back(std::make_pair('/', std::string("__2f__"))); - } - - // Replaces chars that FAT32 can't support with strings defined in /sys/replace - for (auto& replacement : replacements) - { - for (size_t j = 0; (j = filename.find(replacement.first, j)) != filename.npos; ++j) - filename.replace(j, 1, replacement.second); - } - return filename; + return Common::EscapeFileName(filename); } u8 Gamecode[4]; // 0x00 0x04 Gamecode diff --git a/Source/Core/Core/HW/WiiSaveCrypted.cpp b/Source/Core/Core/HW/WiiSaveCrypted.cpp index 42b4155ec2..28fbf786df 100644 --- a/Source/Core/Core/HW/WiiSaveCrypted.cpp +++ b/Source/Core/Core/HW/WiiSaveCrypted.cpp @@ -30,8 +30,6 @@ #include "Core/HW/WiiSaveCrypted.h" -static Common::replace_v replacements; - const u8 CWiiSaveCrypted::s_sd_key[16] = {0xAB, 0x01, 0xB9, 0xD8, 0xE1, 0x62, 0x2B, 0x08, 0xAF, 0xBA, 0xD8, 0x4D, 0xBF, 0xC2, 0xA5, 0x5D}; const u8 CWiiSaveCrypted::s_md5_blanker[16] = {0x0E, 0x65, 0x37, 0x81, 0x99, 0xBE, 0x45, 0x17, @@ -102,7 +100,6 @@ void CWiiSaveCrypted::ExportAllSaves() CWiiSaveCrypted::CWiiSaveCrypted(const std::string& filename, u64 title_id) : m_encrypted_save_path(filename), m_title_id(title_id) { - Common::ReadReplacements(replacements); memcpy(m_sd_iv, "\x21\x67\x12\xE6\xAA\x1F\x68\x9F\x95\xC5\xA2\x23\x24\xDC\x6A\x98", 0x10); if (!title_id) // Import @@ -340,12 +337,8 @@ void CWiiSaveCrypted::ImportWiiSaveFiles() } else { - std::string filename((char*)file_hdr_tmp.name); - for (const Common::replace_t& replacement : replacements) - { - for (size_t j = 0; (j = filename.find(replacement.first, j)) != filename.npos; ++j) - filename.replace(j, 1, replacement.second); - } + std::string filename = + Common::EscapeFileName(reinterpret_cast(file_hdr_tmp.name)); std::string file_path_full = m_wii_title_path + filename; File::CreateFullPath(file_path_full); @@ -388,7 +381,6 @@ void CWiiSaveCrypted::ExportWiiSaveFiles() for (u32 i = 0; i < m_files_list_size; i++) { FileHDR file_hdr_tmp; - std::string name; memset(&file_hdr_tmp, 0, FILE_HDR_SZ); u32 file_size = 0; @@ -407,15 +399,8 @@ void CWiiSaveCrypted::ExportWiiSaveFiles() file_hdr_tmp.size = Common::swap32(file_size); file_hdr_tmp.Permissions = 0x3c; - name = m_files_list[i].substr(m_wii_title_path.length() + 1); - - for (const Common::replace_t& repl : replacements) - { - for (size_t j = 0; (j = name.find(repl.second, j)) != name.npos; ++j) - { - name.replace(j, repl.second.length(), 1, repl.first); - } - } + std::string name = + Common::UnescapeFileName(m_files_list[i].substr(m_wii_title_path.length() + 1)); if (name.length() > 0x44) { diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp index 119f243d95..dc659c244f 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp @@ -16,25 +16,16 @@ #include "Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h" #include "Core/IPC_HLE/WII_IPC_HLE_Device_fs.h" -static Common::replace_v replacements; - static std::map> openFiles; // This is used by several of the FileIO and /dev/fs functions -std::string HLE_IPC_BuildFilename(std::string path_wii) +std::string HLE_IPC_BuildFilename(const std::string& wii_path) { - std::string path_full = File::GetUserPath(D_SESSION_WIIROOT_IDX); + std::string nand_path = File::GetUserPath(D_SESSION_WIIROOT_IDX); + if (wii_path.empty() || wii_path[0] != '/') + return nand_path; - // Replaces chars that FAT32 can't support with strings defined in /sys/replace - for (auto& replacement : replacements) - { - for (size_t j = 0; (j = path_wii.find(replacement.first, j)) != path_wii.npos; ++j) - path_wii.replace(j, 1, replacement.second); - } - - path_full += path_wii; - - return path_full; + return nand_path + Common::EscapePath(wii_path); } void HLE_IPC_CreateVirtualFATFilesystem() @@ -76,7 +67,6 @@ CWII_IPC_HLE_Device_FileIO::CWII_IPC_HLE_Device_FileIO(u32 device_id, const std::string& device_name) : IWII_IPC_HLE_Device(device_id, device_name, false) // not a real hardware { - Common::ReadReplacements(replacements); } CWII_IPC_HLE_Device_FileIO::~CWII_IPC_HLE_Device_FileIO() diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h index 534b744f7f..3d49acc04b 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h @@ -18,7 +18,7 @@ namespace File class IOFile; } -std::string HLE_IPC_BuildFilename(std::string _pFilename); +std::string HLE_IPC_BuildFilename(const std::string& wii_path); void HLE_IPC_CreateVirtualFATFilesystem(); class CWII_IPC_HLE_Device_FileIO : public IWII_IPC_HLE_Device diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_fs.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_fs.cpp index d631b4e28e..15533cada5 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_fs.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_fs.cpp @@ -22,12 +22,9 @@ #include "Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h" #include "Core/IPC_HLE/WII_IPC_HLE_Device_fs.h" -static Common::replace_v replacements; - CWII_IPC_HLE_Device_fs::CWII_IPC_HLE_Device_fs(u32 _DeviceID, const std::string& _rDeviceName) : IWII_IPC_HLE_Device(_DeviceID, _rDeviceName) { - Common::ReadReplacements(replacements); } CWII_IPC_HLE_Device_fs::~CWII_IPC_HLE_Device_fs() @@ -132,10 +129,9 @@ IPCCommandResult CWII_IPC_HLE_Device_fs::IOCtlV(u32 _CommandAddress) { for (File::FSTEntry& child : entry.children) { - // Decode entities of invalid file system characters so that - // games (such as HP:HBP) will be able to find what they expect. - for (const Common::replace_t& r : replacements) - child.virtualName = ReplaceAll(child.virtualName, r.second, {r.first}); + // Decode escaped invalid file system characters so that games (such as + // Harry Potter and the Half-Blood Prince) can find what they expect. + child.virtualName = Common::UnescapeFileName(child.virtualName); } std::sort(entry.children.begin(), entry.children.end(), From 7a4f19ed98a86b8370155cf1c2a96e7c00541151 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sat, 26 Nov 2016 18:06:31 +0100 Subject: [PATCH 3/5] IOS HLE: Correct handling of paths that don't start with / --- Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp | 10 ++- .../IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp | 8 +- .../Core/IPC_HLE/WII_IPC_HLE_Device_fs.cpp | 82 +++++++++++++++++-- 3 files changed, 85 insertions(+), 15 deletions(-) diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp index a47b529b38..83d8397331 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp @@ -441,7 +441,7 @@ void ExecuteCommand(u32 address) result = IWII_IPC_HLE_Device::GetDefaultReply(); } } - else + else if (device_name.find('/') == 0) { device = CreateFileIO(DeviceID, device_name); result = device->Open(address, Mode); @@ -449,9 +449,13 @@ void ExecuteCommand(u32 address) DEBUG_LOG(WII_IPC_FILEIO, "IOP: Open File (Device=%s, ID=%08x, Mode=%i)", device->GetDeviceName().c_str(), DeviceID, Mode); if (Memory::Read_U32(address + 4) == (u32)DeviceID) - { s_fdmap[DeviceID] = device; - } + } + else + { + WARN_LOG(WII_IPC_HLE, "Invalid device: %s", device_name.c_str()); + Memory::Write_U32(FS_ENOENT, address + 4); + result = IWII_IPC_HLE_Device::GetDefaultReply(); } } else diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp index dc659c244f..3616bd39d1 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp @@ -7,6 +7,7 @@ #include #include +#include "Common/Assert.h" #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" #include "Common/FileUtil.h" @@ -22,10 +23,11 @@ static std::map> openFiles; std::string HLE_IPC_BuildFilename(const std::string& wii_path) { std::string nand_path = File::GetUserPath(D_SESSION_WIIROOT_IDX); - if (wii_path.empty() || wii_path[0] != '/') - return nand_path; + if (wii_path.compare(0, 1, "/") == 0) + return nand_path + Common::EscapePath(wii_path); - return nand_path + Common::EscapePath(wii_path); + _assert_(false); + return nand_path; } void HLE_IPC_CreateVirtualFATFilesystem() diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_fs.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_fs.cpp index 15533cada5..6d686b2797 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_fs.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_fs.cpp @@ -22,6 +22,11 @@ #include "Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h" #include "Core/IPC_HLE/WII_IPC_HLE_Device_fs.h" +static bool IsValidWiiPath(const std::string& path) +{ + return path.compare(0, 1, "/") == 0; +} + CWII_IPC_HLE_Device_fs::CWII_IPC_HLE_Device_fs(u32 _DeviceID, const std::string& _rDeviceName) : IWII_IPC_HLE_Device(_DeviceID, _rDeviceName) { @@ -93,9 +98,18 @@ IPCCommandResult CWII_IPC_HLE_Device_fs::IOCtlV(u32 _CommandAddress) { case IOCTLV_READ_DIR: { + const std::string relative_path = + Memory::GetString(CommandBuffer.InBuffer[0].m_Address, CommandBuffer.InBuffer[0].m_Size); + + if (!IsValidWiiPath(relative_path)) + { + WARN_LOG(WII_IPC_FILEIO, "Not a valid path: %s", relative_path.c_str()); + ReturnValue = FS_RESULT_FATAL; + break; + } + // the Wii uses this function to define the type (dir or file) - std::string DirName(HLE_IPC_BuildFilename( - Memory::GetString(CommandBuffer.InBuffer[0].m_Address, CommandBuffer.InBuffer[0].m_Size))); + std::string DirName(HLE_IPC_BuildFilename(relative_path)); INFO_LOG(WII_IPC_FILEIO, "FS: IOCTL_READ_DIR %s", DirName.c_str()); @@ -177,6 +191,14 @@ IPCCommandResult CWII_IPC_HLE_Device_fs::IOCtlV(u32 _CommandAddress) // It should be correct, but don't count on it... std::string relativepath = Memory::GetString(CommandBuffer.InBuffer[0].m_Address, CommandBuffer.InBuffer[0].m_Size); + + if (!IsValidWiiPath(relativepath)) + { + WARN_LOG(WII_IPC_FILEIO, "Not a valid path: %s", relativepath.c_str()); + ReturnValue = FS_RESULT_FATAL; + break; + } + std::string path(HLE_IPC_BuildFilename(relativepath)); u32 fsBlocks = 0; u32 iNodes = 0; @@ -291,7 +313,13 @@ s32 CWII_IPC_HLE_Device_fs::ExecuteCommand(u32 _Parameter, u32 _BufferIn, u32 _B Addr += 4; u16 GroupID = Memory::Read_U16(Addr); Addr += 2; - std::string DirName(HLE_IPC_BuildFilename(Memory::GetString(Addr, 64))); + const std::string wii_path = Memory::GetString(Addr, 64); + if (!IsValidWiiPath(wii_path)) + { + WARN_LOG(WII_IPC_FILEIO, "Not a valid path: %s", wii_path.c_str()); + return FS_RESULT_FATAL; + } + std::string DirName(HLE_IPC_BuildFilename(wii_path)); Addr += 64; Addr += 9; // owner attribs, permission u8 Attribs = Memory::Read_U8(Addr); @@ -316,7 +344,13 @@ s32 CWII_IPC_HLE_Device_fs::ExecuteCommand(u32 _Parameter, u32 _BufferIn, u32 _B Addr += 4; u16 GroupID = Memory::Read_U16(Addr); Addr += 2; - std::string Filename = HLE_IPC_BuildFilename(Memory::GetString(_BufferIn, 64)); + const std::string wii_path = Memory::GetString(_BufferIn, 64); + if (!IsValidWiiPath(wii_path)) + { + WARN_LOG(WII_IPC_FILEIO, "Not a valid path: %s", wii_path.c_str()); + return FS_RESULT_FATAL; + } + std::string Filename = HLE_IPC_BuildFilename(wii_path); Addr += 64; u8 OwnerPerm = Memory::Read_U8(Addr); Addr += 1; @@ -348,7 +382,13 @@ s32 CWII_IPC_HLE_Device_fs::ExecuteCommand(u32 _Parameter, u32 _BufferIn, u32 _B u32 OwnerID = 0; u16 GroupID = 0x3031; // this is also known as makercd, 01 (0x3031) for nintendo and 08 // (0x3038) for MH3 etc - std::string Filename = HLE_IPC_BuildFilename(Memory::GetString(_BufferIn, 64)); + const std::string wii_path = Memory::GetString(_BufferIn, 64); + if (!IsValidWiiPath(wii_path)) + { + WARN_LOG(WII_IPC_FILEIO, "Not a valid path: %s", wii_path.c_str()); + return FS_RESULT_FATAL; + } + std::string Filename = HLE_IPC_BuildFilename(wii_path); u8 OwnerPerm = 0x3; // read/write u8 GroupPerm = 0x3; // read/write u8 OtherPerm = 0x3; // read/write @@ -401,7 +441,13 @@ s32 CWII_IPC_HLE_Device_fs::ExecuteCommand(u32 _Parameter, u32 _BufferIn, u32 _B _dbg_assert_(WII_IPC_FILEIO, _BufferOutSize == 0); int Offset = 0; - std::string Filename = HLE_IPC_BuildFilename(Memory::GetString(_BufferIn + Offset, 64)); + const std::string wii_path = Memory::GetString(_BufferIn + Offset, 64); + if (!IsValidWiiPath(wii_path)) + { + WARN_LOG(WII_IPC_FILEIO, "Not a valid path: %s", wii_path.c_str()); + return FS_RESULT_FATAL; + } + std::string Filename = HLE_IPC_BuildFilename(wii_path); Offset += 64; if (File::Delete(Filename)) { @@ -425,10 +471,22 @@ s32 CWII_IPC_HLE_Device_fs::ExecuteCommand(u32 _Parameter, u32 _BufferIn, u32 _B _dbg_assert_(WII_IPC_FILEIO, _BufferOutSize == 0); int Offset = 0; - std::string Filename = HLE_IPC_BuildFilename(Memory::GetString(_BufferIn + Offset, 64)); + const std::string wii_path = Memory::GetString(_BufferIn + Offset, 64); + if (!IsValidWiiPath(wii_path)) + { + WARN_LOG(WII_IPC_FILEIO, "Not a valid path: %s", wii_path.c_str()); + return FS_RESULT_FATAL; + } + std::string Filename = HLE_IPC_BuildFilename(wii_path); Offset += 64; - std::string FilenameRename = HLE_IPC_BuildFilename(Memory::GetString(_BufferIn + Offset, 64)); + const std::string wii_path_rename = Memory::GetString(_BufferIn + Offset, 64); + if (!IsValidWiiPath(wii_path_rename)) + { + WARN_LOG(WII_IPC_FILEIO, "Not a valid path: %s", wii_path_rename.c_str()); + return FS_RESULT_FATAL; + } + std::string FilenameRename = HLE_IPC_BuildFilename(wii_path_rename); Offset += 64; // try to make the basis directory @@ -465,7 +523,13 @@ s32 CWII_IPC_HLE_Device_fs::ExecuteCommand(u32 _Parameter, u32 _BufferIn, u32 _B Addr += 4; u16 GroupID = Memory::Read_U16(Addr); Addr += 2; - std::string Filename(HLE_IPC_BuildFilename(Memory::GetString(Addr, 64))); + const std::string wii_path = Memory::GetString(Addr, 64); + if (!IsValidWiiPath(wii_path)) + { + WARN_LOG(WII_IPC_FILEIO, "Not a valid path: %s", wii_path.c_str()); + return FS_RESULT_FATAL; + } + std::string Filename(HLE_IPC_BuildFilename(wii_path)); Addr += 64; u8 OwnerPerm = Memory::Read_U8(Addr); Addr++; From 841e5893f4babccdd28a60ee4e63532f030daf53 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sat, 26 Nov 2016 21:55:01 +0100 Subject: [PATCH 4/5] IOS HLE: Add unit test for path/filename escaping --- Source/UnitTests/Common/CMakeLists.txt | 1 + Source/UnitTests/Common/NandPathsTest.cpp | 42 +++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 Source/UnitTests/Common/NandPathsTest.cpp diff --git a/Source/UnitTests/Common/CMakeLists.txt b/Source/UnitTests/Common/CMakeLists.txt index eba08f9453..238dbeddb7 100644 --- a/Source/UnitTests/Common/CMakeLists.txt +++ b/Source/UnitTests/Common/CMakeLists.txt @@ -8,4 +8,5 @@ add_dolphin_test(FifoQueueTest FifoQueueTest.cpp) add_dolphin_test(FixedSizeQueueTest FixedSizeQueueTest.cpp) add_dolphin_test(FlagTest FlagTest.cpp) add_dolphin_test(MathUtilTest MathUtilTest.cpp) +add_dolphin_test(NandPathsTest NandPathsTest.cpp) add_dolphin_test(x64EmitterTest x64EmitterTest.cpp) diff --git a/Source/UnitTests/Common/NandPathsTest.cpp b/Source/UnitTests/Common/NandPathsTest.cpp new file mode 100644 index 0000000000..f36b967de9 --- /dev/null +++ b/Source/UnitTests/Common/NandPathsTest.cpp @@ -0,0 +1,42 @@ +// Copyright 2016 Dolphin Emulator Project +// Licensed under GPLv2+ +// Refer to the license.txt file included. + +#include +#include + +#include "Common/NandPaths.h" + +static void TestEscapeAndUnescape(const std::string& unescaped, const std::string& escaped) +{ + EXPECT_EQ(escaped, Common::EscapeFileName(unescaped)); + EXPECT_EQ(unescaped, Common::UnescapeFileName(escaped)); +} + +TEST(NandPaths, EscapeUnescape) +{ + EXPECT_EQ("/a/__2e__/b/__3e__", Common::EscapePath("/a/./b/>")); + EXPECT_EQ("/a/__2e____2e__/b/__3e__", Common::EscapePath("/a/../b/>")); + EXPECT_EQ("/a/__2e____2e____2e__/b/__3e__", Common::EscapePath("/a/.../b/>")); + EXPECT_EQ("/a/__2e____2e____2e____2e__/b/__3e__", Common::EscapePath("/a/..../b/>")); + + EXPECT_EQ("", Common::EscapePath("")); + TestEscapeAndUnescape("", ""); + + TestEscapeAndUnescape("regular string", "regular string"); + TestEscapeAndUnescape("a/../1b|", "a__2f__..__2f__1b__7c__"); + TestEscapeAndUnescape("__22__", "__5f____5f__22__5f____5f__"); + + // The \0 can't be written in a regular string literal, otherwise it'll be treated as the end + TestEscapeAndUnescape(std::string{'\0'} + + "\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" + "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" + "\"*/:<>?\\_|\x7f", + "__00____01____02____03____04____05____06____07__" + "__08____09____0a____0b____0c____0d____0e____0f__" + "__10____11____12____13____14____15____16____17__" + "__18____19____1a____1b____1c____1d____1e____1f__" + "__22____2a____2f____3a____3c____3e____3f____5c_____7c____7f__"); + // ^ + // There is a normal underscore here (not part of an escape sequence): | +} From 0c6fd47460cb59c1b58c8870a4d2f05c23d8f357 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 27 Nov 2016 11:18:27 +0100 Subject: [PATCH 5/5] Add unit test for StringUtil's newly added JoinStrings --- Source/UnitTests/Common/CMakeLists.txt | 1 + Source/UnitTests/Common/StringUtilTest.cpp | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 Source/UnitTests/Common/StringUtilTest.cpp diff --git a/Source/UnitTests/Common/CMakeLists.txt b/Source/UnitTests/Common/CMakeLists.txt index 238dbeddb7..28ca1237b3 100644 --- a/Source/UnitTests/Common/CMakeLists.txt +++ b/Source/UnitTests/Common/CMakeLists.txt @@ -9,4 +9,5 @@ add_dolphin_test(FixedSizeQueueTest FixedSizeQueueTest.cpp) add_dolphin_test(FlagTest FlagTest.cpp) add_dolphin_test(MathUtilTest MathUtilTest.cpp) add_dolphin_test(NandPathsTest NandPathsTest.cpp) +add_dolphin_test(StringUtilTest StringUtilTest.cpp) add_dolphin_test(x64EmitterTest x64EmitterTest.cpp) diff --git a/Source/UnitTests/Common/StringUtilTest.cpp b/Source/UnitTests/Common/StringUtilTest.cpp new file mode 100644 index 0000000000..603c99d80c --- /dev/null +++ b/Source/UnitTests/Common/StringUtilTest.cpp @@ -0,0 +1,18 @@ +// Copyright 2016 Dolphin Emulator Project +// Licensed under GPLv2+ +// Refer to the license.txt file included. + +#include +#include +#include + +#include "Common/StringUtil.h" + +TEST(StringUtil, JoinStrings) +{ + EXPECT_EQ("", JoinStrings({}, ", ")); + EXPECT_EQ("a", JoinStrings({"a"}, ",")); + EXPECT_EQ("ab", JoinStrings({"a", "b"}, "")); + EXPECT_EQ("a, bb, c", JoinStrings({"a", "bb", "c"}, ", ")); + EXPECT_EQ("???", JoinStrings({"?", "?"}, "?")); +}