diff --git a/WheelWizard/Features/CustomDistributions/RetroRewind.cs b/WheelWizard/Features/CustomDistributions/RetroRewind.cs index c63e34ca..ae6a7a16 100644 --- a/WheelWizard/Features/CustomDistributions/RetroRewind.cs +++ b/WheelWizard/Features/CustomDistributions/RetroRewind.cs @@ -352,20 +352,14 @@ private OperationResult ExtractZipFile(string path, string destinationDirectory, progressWindow.SetExtraText(Common.State_Extracting).SetGoal($"Extracting {total} files"); }); - // Absolute path of the destination directory - var absoluteDestinationPath = _fileSystem.Path.GetFullPath(destinationDirectory + Path.AltDirectorySeparatorChar); - for (var i = 0; i < total; i++) { var entry = entries[i]; - var destinationPath = _fileSystem.Path.GetFullPath(Path.Combine(destinationDirectory, entry.FullName)); - - // Directory traversal check - if (!destinationPath.StartsWith(absoluteDestinationPath, StringComparison.Ordinal)) + if (!PathSafetyHelper.TryGetPathWithinDirectory(destinationDirectory, entry.FullName, out var destinationPath)) return Fail("The file path is outside the destination directory. Please contact the developers."); // If it’s a directory, create it - if (entry.FullName.EndsWith(Path.AltDirectorySeparatorChar)) + if (entry.FullName.EndsWith(Path.AltDirectorySeparatorChar) || entry.FullName.EndsWith(Path.DirectorySeparatorChar)) { _fileSystem.Directory.CreateDirectory(destinationPath); } @@ -406,21 +400,15 @@ private async Task ApplyFileDeletionsBetweenVersions(SemVersion foreach (var file in deletionsToApply) { - var absoluteDestinationPath = _fileSystem.Path.GetFullPath( - PathManager.RiivolutionWhWzFolderPath + _fileSystem.Path.AltDirectorySeparatorChar - ); - var filePath = _fileSystem.Path.GetFullPath(_fileSystem.Path.Combine(absoluteDestinationPath, file.Path.TrimStart('/'))); - //because we are actually getting the path from the server, - //we need to make sure we are not getting hacked, so we check if the path is in the riivolution folder - var resolvedPath = _fileSystem.Path.GetFullPath(new FileInfo(filePath).FullName); + // The deletion list is server-controlled, so keep every resolved path inside the riivolution folder. if ( - !resolvedPath.StartsWith(absoluteDestinationPath, StringComparison.Ordinal) - || !filePath.StartsWith(absoluteDestinationPath, StringComparison.Ordinal) - || file.Path.Contains("..") + !PathSafetyHelper.TryGetPathWithinDirectory( + PathManager.RiivolutionWhWzFolderPath, + file.Path.TrimStart('/', '\\'), + out var filePath + ) ) - { - return Fail("Invalid file path detected. Please contact the developers.\n Server error: " + resolvedPath); - } + return Fail("Invalid file path detected. Please contact the developers.\n Server error: " + file.Path); if (_fileSystem.File.Exists(filePath)) _fileSystem.File.Delete(filePath); diff --git a/WheelWizard/Features/CustomDistributions/RetroRewindBeta.cs b/WheelWizard/Features/CustomDistributions/RetroRewindBeta.cs index de759416..9e734b58 100644 --- a/WheelWizard/Features/CustomDistributions/RetroRewindBeta.cs +++ b/WheelWizard/Features/CustomDistributions/RetroRewindBeta.cs @@ -129,12 +129,10 @@ public async Task InstallAsync(ProgressWindow progressWindow) public Task RemoveAsync(ProgressWindow progressWindow) { var rootPath = PathManager.RiivolutionWhWzFolderPath; - var rootFullPath = _fileSystem.Path.GetFullPath(rootPath + Path.AltDirectorySeparatorChar); foreach (var entry in LoadManifest()) { - var fullPath = _fileSystem.Path.GetFullPath(_fileSystem.Path.Combine(rootPath, entry)); - if (!fullPath.StartsWith(rootFullPath, StringComparison.Ordinal)) + if (!PathSafetyHelper.TryGetPathWithinDirectory(rootPath, entry, out var fullPath)) continue; if (_fileSystem.File.Exists(fullPath)) @@ -195,7 +193,7 @@ out bool badPassword badPassword = false; try { - using var archive = ArchiveFactory.Open(zipPath, new ReaderOptions { Password = password }); + using var archive = ArchiveFactory.OpenArchive(zipPath, new ReaderOptions { Password = password }); var entries = archive.Entries.Where(entry => !entry.IsDirectory).ToList(); if (entries.Count == 0) return Ok(); @@ -205,20 +203,16 @@ out bool badPassword progressWindow.SetExtraText(Common.State_Extracting).SetGoal($"Extracting {entries.Count} files"); }); - var absoluteDestinationPath = _fileSystem.Path.GetFullPath(destinationDirectory + Path.AltDirectorySeparatorChar); - for (var i = 0; i < entries.Count; i++) { var entry = entries[i]; - var normalized = NormalizeEntryPath(entry.Key ?? string.Empty); - if (string.IsNullOrWhiteSpace(normalized)) + if (!PathSafetyHelper.TryNormalizeRelativePath(entry.Key ?? string.Empty, out var normalized)) continue; if (!TryGetRelativeExtractionPath(normalized, out var relativePath)) return Fail("Unexpected file in the test archive. Please contact the developers."); - var destinationPath = _fileSystem.Path.GetFullPath(_fileSystem.Path.Combine(destinationDirectory, relativePath)); - if (!destinationPath.StartsWith(absoluteDestinationPath, StringComparison.Ordinal)) + if (!PathSafetyHelper.TryGetPathWithinDirectory(destinationDirectory, relativePath, out var destinationPath)) return Fail("The file path is outside the destination directory. Please contact the developers."); var destinationDir = _fileSystem.Path.GetDirectoryName(destinationPath); @@ -256,8 +250,6 @@ private static bool IsBadPasswordException(Exception ex) return ex.InnerException != null && IsBadPasswordException(ex.InnerException); } - private static string NormalizeEntryPath(string path) => path.Replace('\\', '/').TrimStart('/'); - private bool TryGetRelativeExtractionPath(string normalizedPath, out string relativePath) { relativePath = string.Empty; @@ -308,8 +300,6 @@ private OperationResult> MoveExtractedFiles(string tempExtractionPa .Directory.EnumerateFiles(betaFolderSource, "*", SearchOption.AllDirectories) .Concat(_fileSystem.Directory.EnumerateFiles(xmlFolderSource, "*", SearchOption.AllDirectories)); - var absoluteDestinationRoot = _fileSystem.Path.GetFullPath(destinationRoot + Path.AltDirectorySeparatorChar); - foreach (var file in sourceFiles) { var relativePath = _fileSystem.Path.GetRelativePath(tempExtractionPath, file); @@ -319,9 +309,7 @@ private OperationResult> MoveExtractedFiles(string tempExtractionPa continue; } - var destinationPath = _fileSystem.Path.Combine(destinationRoot, relativePath); - var fullDestinationPath = _fileSystem.Path.GetFullPath(destinationPath); - if (!fullDestinationPath.StartsWith(absoluteDestinationRoot, StringComparison.Ordinal)) + if (!PathSafetyHelper.TryGetPathWithinDirectory(destinationRoot, relativePath, out var destinationPath)) return Fail("The file path is outside the destination directory. Please contact the developers."); var destinationDirectory = _fileSystem.Path.GetDirectoryName(destinationPath); diff --git a/WheelWizard/Features/Mods/ModInstallationService.cs b/WheelWizard/Features/Mods/ModInstallationService.cs index 53754508..171baa49 100644 --- a/WheelWizard/Features/Mods/ModInstallationService.cs +++ b/WheelWizard/Features/Mods/ModInstallationService.cs @@ -1,4 +1,4 @@ -using System.Collections.ObjectModel; +using System.Collections.ObjectModel; using Avalonia.Threading; using SharpCompress.Archives; using WheelWizard.Helpers; @@ -116,9 +116,6 @@ private static OperationResult ExtractModArchive(string file, string destination using var archive = archiveResult.Value; var totalEntries = archive.Entries.Count(entry => !entry.IsDirectory); var processedEntries = 0; - var fullRoot = Path.GetFullPath(destinationDirectory); - if (!Path.EndsInDirectorySeparator(fullRoot)) - fullRoot += Path.DirectorySeparatorChar; foreach (var entry in archive.Entries.Where(entry => !entry.IsDirectory)) { @@ -131,17 +128,7 @@ private static OperationResult ExtractModArchive(string file, string destination }); var entryKey = entry.Key ?? string.Empty; - var sanitizedKey = string.Join( - Path.DirectorySeparatorChar.ToString(), - entryKey - .Split(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar) - .Where(segment => !string.IsNullOrWhiteSpace(segment)) - ); - - var entryDestinationPath = Path.Combine(destinationDirectory, sanitizedKey); - var fullEntry = Path.GetFullPath(entryDestinationPath); - - if (!fullEntry.StartsWith(fullRoot, StringComparison.OrdinalIgnoreCase)) + if (!PathSafetyHelper.TryGetPathWithinDirectory(destinationDirectory, entryKey, out var fullEntry)) return Fail("Archive entry is outside of the destination directory."); var directoryPath = Path.GetDirectoryName(fullEntry); @@ -172,7 +159,7 @@ private static OperationResult OpenArchive(string filePath, string ext try { - return Ok(ArchiveFactory.Open(filePath)); + return Ok(ArchiveFactory.OpenArchive(filePath)); } catch (Exception ex) { diff --git a/WheelWizard/Helpers/DownloadHelper.cs b/WheelWizard/Helpers/DownloadHelper.cs index 4b933191..59fd8d33 100644 --- a/WheelWizard/Helpers/DownloadHelper.cs +++ b/WheelWizard/Helpers/DownloadHelper.cs @@ -1,4 +1,4 @@ -using WheelWizard.Views.Popups.Generic; +using WheelWizard.Views.Popups.Generic; namespace WheelWizard.Helpers; @@ -74,12 +74,17 @@ public static class DownloadHelper if (!ForceGivenFilePath) { - var finalUrl = response.RequestMessage.RequestUri.ToString(); - // Check for filename in Content-Disposition or fallback to URL var contentDisposition = response.Content.Headers.ContentDisposition; - var fileName = contentDisposition?.FileName?.Trim('"') ?? Path.GetFileName(new Uri(url).AbsolutePath); - fileName = Path.ChangeExtension(fileName, Path.GetExtension(finalUrl)); + var fileName = + contentDisposition?.FileNameStar ?? contentDisposition?.FileName ?? Path.GetFileName(new Uri(url).AbsolutePath); + + if (!PathSafetyHelper.TryGetSafeFileName(fileName, out fileName)) + throw new InvalidOperationException("The server returned an invalid download filename."); + + var finalExtension = Path.GetExtension(response.RequestMessage.RequestUri.AbsolutePath); + if (!string.IsNullOrWhiteSpace(finalExtension)) + fileName = Path.ChangeExtension(fileName, finalExtension); // Add extension if missing in file path if (!Path.HasExtension(fileName)) @@ -92,7 +97,8 @@ public static class DownloadHelper } // Update resolvedFilePath with resolved fileName - resolvedFilePath = Path.Combine(directory, fileName); + if (!PathSafetyHelper.TryGetPathWithinDirectory(directory, fileName, out resolvedFilePath)) + throw new InvalidOperationException("The download path escaped the target directory."); } var totalBytes = response.Content.Headers.ContentLength ?? -1; diff --git a/WheelWizard/Helpers/PathSafetyHelper.cs b/WheelWizard/Helpers/PathSafetyHelper.cs new file mode 100644 index 00000000..8915a5db --- /dev/null +++ b/WheelWizard/Helpers/PathSafetyHelper.cs @@ -0,0 +1,87 @@ +namespace WheelWizard.Helpers; + +public static class PathSafetyHelper +{ + public static bool TryGetPathWithinDirectory(string directory, string relativePath, out string fullPath) + { + fullPath = string.Empty; + + if (!TryNormalizeRelativePath(relativePath, out var normalizedRelativePath)) + return false; + + var destinationPath = Path.Combine(directory, normalizedRelativePath); + var fullDirectory = Path.GetFullPath(directory); + var candidatePath = Path.GetFullPath(destinationPath); + + if (!IsPathWithinDirectory(fullDirectory, candidatePath)) + return false; + + fullPath = candidatePath; + return true; + } + + public static bool IsPathWithinDirectory(string directory, string path) + { + if (string.IsNullOrWhiteSpace(directory) || string.IsNullOrWhiteSpace(path)) + return false; + + var fullDirectory = Path.GetFullPath(directory); + var fullPath = Path.GetFullPath(path); + var relativePath = Path.GetRelativePath(fullDirectory, fullPath); + + return relativePath == "." + || ( + !Path.IsPathRooted(relativePath) + && !relativePath.Equals("..", StringComparison.Ordinal) + && !relativePath.StartsWith($"..{Path.DirectorySeparatorChar}", StringComparison.Ordinal) + && !relativePath.StartsWith($"..{Path.AltDirectorySeparatorChar}", StringComparison.Ordinal) + ); + } + + public static bool TryNormalizeRelativePath(string path, out string normalizedPath) + { + normalizedPath = string.Empty; + + if (string.IsNullOrWhiteSpace(path)) + return false; + + var trimmedPath = path.Trim(); + if (Path.IsPathFullyQualified(trimmedPath) || trimmedPath.StartsWith('/') || trimmedPath.StartsWith('\\')) + return false; + + var segments = trimmedPath.Replace('\\', '/').Split('/', StringSplitOptions.RemoveEmptyEntries).Where(segment => segment != "."); + + var safeSegments = new List(); + foreach (var segment in segments) + { + if (segment == ".." || segment.Contains(Path.VolumeSeparatorChar)) + return false; + + safeSegments.Add(segment); + } + + if (safeSegments.Count == 0) + return false; + + normalizedPath = Path.Combine(safeSegments.ToArray()); + return true; + } + + public static bool TryGetSafeFileName(string fileName, out string safeFileName) + { + safeFileName = string.Empty; + + if (string.IsNullOrWhiteSpace(fileName)) + return false; + + var leafName = fileName.Trim().Trim('"').Replace('\\', '/').Split('/', StringSplitOptions.RemoveEmptyEntries).LastOrDefault(); + if (string.IsNullOrWhiteSpace(leafName) || leafName is "." or "..") + return false; + + if (leafName.IndexOfAny(Path.GetInvalidFileNameChars()) >= 0) + return false; + + safeFileName = leafName; + return true; + } +} diff --git a/WheelWizard/WheelWizard.csproj b/WheelWizard/WheelWizard.csproj index 4ae8ca8d..7220c99d 100644 --- a/WheelWizard/WheelWizard.csproj +++ b/WheelWizard/WheelWizard.csproj @@ -1,4 +1,4 @@ - + WheelWizard.Program @@ -56,7 +56,8 @@ - + + all runtime; build; native; contentfiles; analyzers; buildtransitive