FileSystem: Don't follow symlinks when recursively deleting directories

common/FileSystem.cpp

@@ -1225,7 +1225,12 @@ bool FileSystem::RecursiveDeleteDirectory(const char* path)
 	{
 		for (const FILESYSTEM_FIND_DATA& fd : results)
 		{
-			if (fd.Attributes & FILESYSTEM_FILE_ATTRIBUTE_DIRECTORY)
+			if (IsSymbolicLink(fd.FileName.c_str()))
+			{
+				if (!DeleteSymbolicLink(fd.FileName.c_str()))
+					return false;
+			}
+			else if ((fd.Attributes & FILESYSTEM_FILE_ATTRIBUTE_DIRECTORY))
 			{
 				if (!RecursiveDeleteDirectory(fd.FileName.c_str()))
 					return false;
@@ -1650,21 +1655,6 @@ bool FileSystem::DirectoryExists(const char* path)
 		return false;
 }
 
-bool FileSystem::IsRealDirectory(const char* path)
-{
-	// convert to wide string
-	const std::wstring wpath = GetWin32Path(path);
-	if (wpath.empty())
-		return false;
-
-	// determine attributes for the path. if it's a directory, things have to be handled differently..
-	const DWORD fileAttributes = GetFileAttributesW(wpath.c_str());
-	if (fileAttributes == INVALID_FILE_ATTRIBUTES)
-		return false;
-
-	return ((fileAttributes & (FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_REPARSE_POINT)) != FILE_ATTRIBUTE_DIRECTORY);
-}
-
 bool FileSystem::DirectoryIsEmpty(const char* path)
 {
 	std::wstring wpath = GetWin32Path(path);
@@ -1935,6 +1925,52 @@ bool FileSystem::SetPathCompression(const char* path, bool enable)
 	return result;
 }
 
+bool FileSystem::IsSymbolicLink(const char* path)
+{
+	// convert to wide string
+	const std::wstring wpath = GetWin32Path(path);
+	if (wpath.empty())
+		return false;
+
+	// determine attributes for the path
+	const DWORD fileAttributes = GetFileAttributesW(wpath.c_str());
+	if (fileAttributes == INVALID_FILE_ATTRIBUTES)
+		return false;
+
+	return fileAttributes & FILE_ATTRIBUTE_REPARSE_POINT;
+}
+
+bool FileSystem::DeleteSymbolicLink(const char* path, Error* error)
+{
+	// convert to wide string
+	const std::wstring wpath = GetWin32Path(path);
+	if (wpath.empty())
+	{
+		Error::SetStringView(error, "Invalid path.");
+		return false;
+	}
+
+	// delete the symbolic link
+	if (DirectoryExists(path))
+	{
+		if (!RemoveDirectoryW(wpath.c_str()))
+		{
+			Error::SetWin32(error, "RemoveDirectoryW() failed: ", GetLastError());
+			return false;
+		}
+	}
+	else
+	{
+		if (!DeleteFileW(wpath.c_str()))
+		{
+			Error::SetWin32(error, "DeleteFileW() failed: ", GetLastError());
+			return false;
+		}
+	}
+
+	return true;
+}
+
 #else
 
 // No 32-bit file offsets breaking stuff please.
@@ -2216,15 +2252,6 @@ bool FileSystem::DirectoryExists(const char* path)
 		return false;
 }
 
-bool FileSystem::IsRealDirectory(const char* path)
-{
-	struct stat sysStatData;
-	if (lstat(path, &sysStatData) < 0)
-		return false;
-
-	return (S_ISDIR(sysStatData.st_mode) && !S_ISLNK(sysStatData.st_mode));
-}
-
 bool FileSystem::DirectoryIsEmpty(const char* path)
 {
 	DIR* pDir = opendir(path);
@@ -2478,6 +2505,26 @@ bool FileSystem::SetPathCompression(const char* path, bool enable)
 	return false;
 }
 
+bool FileSystem::IsSymbolicLink(const char* path)
+{
+	struct stat sysStatData;
+	if (lstat(path, &sysStatData) < 0)
+		return false;
+
+	return S_ISLNK(sysStatData.st_mode);
+}
+
+bool FileSystem::DeleteSymbolicLink(const char* path, Error* error)
+{
+	if (unlink(path) != 0)
+	{
+		Error::SetErrno(error, "unlink() failed: ", errno);
+		return false;
+	}
+
+	return true;
+}
+
 FileSystem::POSIXLock::POSIXLock(int fd)
 {
 	if (lockf(fd, F_LOCK, 0) == 0)

common/FileSystem.h

@@ -84,7 +84,6 @@ namespace FileSystem
 
 	/// Directory exists?
 	bool DirectoryExists(const char* path);
-	bool IsRealDirectory(const char* path);
 
 	/// Directory does not contain any files?
 	bool DirectoryIsEmpty(const char* path);
@@ -170,6 +169,12 @@ namespace FileSystem
 	/// Does nothing and returns false on non-Windows platforms.
 	bool SetPathCompression(const char* path, bool enable);
 
+	/// Checks if a file or directory is a symbolic link.
+	bool IsSymbolicLink(const char* path);
+
+	/// Deletes a symbolic link (either a file or directory).
+	bool DeleteSymbolicLink(const char* path, Error* error = nullptr);
+
 #ifdef _WIN32
 	// Path limit remover, but also converts to a wide string at the same time.
 	bool GetWin32Path(std::wstring* dest, std::string_view str);

tests/ctest/common/CMakeLists.txt

@@ -1,5 +1,6 @@
 add_pcsx2_test(common_test
 	byteswap_tests.cpp
+	filesystem_tests.cpp
 	path_tests.cpp
 	string_util_tests.cpp
 )

tests/ctest/common/filesystem_tests.cpp

@@ -0,0 +1,60 @@
+// SPDX-FileCopyrightText: 2002-2024 PCSX2 Dev Team
+// SPDX-License-Identifier: GPL-3.0+
+
+#include "common/FileSystem.h"
+#include "common/Path.h"
+#include <gtest/gtest.h>
+
+#ifdef __linux__
+
+#include <unistd.h>
+
+static std::optional<std::string> create_test_directory()
+{
+	for (u16 i = 0; i < UINT16_MAX; i++)
+	{
+		std::string path = std::string("/tmp/pcsx2_filesystem_test_") + std::to_string(i);
+		if (!FileSystem::DirectoryExists(path.c_str()))
+		{
+			if (!FileSystem::CreateDirectoryPath(path.c_str(), false))
+				break;
+
+			return path;
+		}
+	}
+
+	return std::nullopt;
+}
+
+TEST(FileSystem, RecursiveDeleteDirectoryDontFollowSymbolicLinks)
+{
+	// Find a suitable location to write some test files.
+	std::optional<std::string> test_dir = create_test_directory();
+	ASSERT_TRUE(test_dir.has_value());
+
+	// Create a target directory containing a file that shouldn't be deleted.
+	std::string target_dir = Path::Combine(*test_dir, "target_dir");
+	ASSERT_TRUE(FileSystem::CreateDirectoryPath(target_dir.c_str(), false));
+	std::string file_path = Path::Combine(target_dir, "file.txt");
+	ASSERT_TRUE(FileSystem::WriteStringToFile(file_path.c_str(), "Lorem ipsum!"));
+
+	// Create a directory containing a symlink to the target directory.
+	std::string dir_to_delete = Path::Combine(*test_dir, "dir_to_delete");
+	ASSERT_TRUE(FileSystem::CreateDirectoryPath(dir_to_delete.c_str(), false));
+	std::string symlink_path = Path::Combine(dir_to_delete, "link");
+	ASSERT_EQ(symlink(target_dir.c_str(), symlink_path.c_str()), 0);
+
+	// Delete the directory containing the symlink.
+	ASSERT_TRUE(dir_to_delete.starts_with("/tmp/"));
+	ASSERT_TRUE(FileSystem::RecursiveDeleteDirectory(dir_to_delete.c_str()));
+
+	// Make sure the target file didn't get deleted.
+	ASSERT_TRUE(FileSystem::FileExists(file_path.c_str()));
+
+	// Clean up.
+	ASSERT_TRUE(FileSystem::DeleteFilePath(file_path.c_str()));
+	ASSERT_TRUE(FileSystem::DeleteDirectory(target_dir.c_str()));
+	ASSERT_TRUE(FileSystem::DeleteDirectory(test_dir->c_str()));
+}
+
+#endif