Skip to content

Commit 322d2c8

Browse files
patchzyypatchzyy
authored
Merge pull request #264 from TeamWheelWizard/fix-SharpCompress
Add PathSafetyHelper and update old package
2 parents 8c33e89 + de5275e commit 322d2c8

6 files changed

Lines changed: 119 additions & 62 deletions

File tree

‎WheelWizard/Features/CustomDistributions/RetroRewind.cs‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -352,20 +352,14 @@ private OperationResult ExtractZipFile(string path, string destinationDirectory,
352352
progressWindow.SetExtraText(Common.State_Extracting).SetGoal($"Extracting {total} files");
353353
});
354354

355-
// Absolute path of the destination directory
356-
var absoluteDestinationPath = _fileSystem.Path.GetFullPath(destinationDirectory + Path.AltDirectorySeparatorChar);
357-
358355
for (var i = 0; i < total; i++)
359356
{
360357
var entry = entries[i];
361-
var destinationPath = _fileSystem.Path.GetFullPath(Path.Combine(destinationDirectory, entry.FullName));
362-
363-
// Directory traversal check
364-
if (!destinationPath.StartsWith(absoluteDestinationPath, StringComparison.Ordinal))
358+
if (!PathSafetyHelper.TryGetPathWithinDirectory(destinationDirectory, entry.FullName, out var destinationPath))
365359
return Fail("The file path is outside the destination directory. Please contact the developers.");
366360

367361
// If it’s a directory, create it
368-
if (entry.FullName.EndsWith(Path.AltDirectorySeparatorChar))
362+
if (entry.FullName.EndsWith(Path.AltDirectorySeparatorChar) || entry.FullName.EndsWith(Path.DirectorySeparatorChar))
369363
{
370364
_fileSystem.Directory.CreateDirectory(destinationPath);
371365
}
@@ -406,21 +400,15 @@ private async Task<OperationResult> ApplyFileDeletionsBetweenVersions(SemVersion
406400

407401
foreach (var file in deletionsToApply)
408402
{
409-
var absoluteDestinationPath = _fileSystem.Path.GetFullPath(
410-
PathManager.RiivolutionWhWzFolderPath + _fileSystem.Path.AltDirectorySeparatorChar
411-
);
412-
var filePath = _fileSystem.Path.GetFullPath(_fileSystem.Path.Combine(absoluteDestinationPath, file.Path.TrimStart('/')));
413-
//because we are actually getting the path from the server,
414-
//we need to make sure we are not getting hacked, so we check if the path is in the riivolution folder
415-
var resolvedPath = _fileSystem.Path.GetFullPath(new FileInfo(filePath).FullName);
403+
// The deletion list is server-controlled, so keep every resolved path inside the riivolution folder.
416404
if (
417-
!resolvedPath.StartsWith(absoluteDestinationPath, StringComparison.Ordinal)
418-
|| !filePath.StartsWith(absoluteDestinationPath, StringComparison.Ordinal)
419-
|| file.Path.Contains("..")
405+
!PathSafetyHelper.TryGetPathWithinDirectory(
406+
PathManager.RiivolutionWhWzFolderPath,
407+
file.Path.TrimStart('/', '\\'),
408+
out var filePath
409+
)
420410
)
421-
{
422-
return Fail("Invalid file path detected. Please contact the developers.\n Server error: " + resolvedPath);
423-
}
411+
return Fail("Invalid file path detected. Please contact the developers.\n Server error: " + file.Path);
424412

425413
if (_fileSystem.File.Exists(filePath))
426414
_fileSystem.File.Delete(filePath);

‎WheelWizard/Features/CustomDistributions/RetroRewindBeta.cs‎

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,10 @@ public async Task<OperationResult> InstallAsync(ProgressWindow progressWindow)
129129
public Task<OperationResult> RemoveAsync(ProgressWindow progressWindow)
130130
{
131131
var rootPath = PathManager.RiivolutionWhWzFolderPath;
132-
var rootFullPath = _fileSystem.Path.GetFullPath(rootPath + Path.AltDirectorySeparatorChar);
133132

134133
foreach (var entry in LoadManifest())
135134
{
136-
var fullPath = _fileSystem.Path.GetFullPath(_fileSystem.Path.Combine(rootPath, entry));
137-
if (!fullPath.StartsWith(rootFullPath, StringComparison.Ordinal))
135+
if (!PathSafetyHelper.TryGetPathWithinDirectory(rootPath, entry, out var fullPath))
138136
continue;
139137

140138
if (_fileSystem.File.Exists(fullPath))
@@ -195,7 +193,7 @@ out bool badPassword
195193
badPassword = false;
196194
try
197195
{
198-
using var archive = ArchiveFactory.Open(zipPath, new ReaderOptions { Password = password });
196+
using var archive = ArchiveFactory.OpenArchive(zipPath, new ReaderOptions { Password = password });
199197
var entries = archive.Entries.Where(entry => !entry.IsDirectory).ToList();
200198
if (entries.Count == 0)
201199
return Ok();
@@ -205,20 +203,16 @@ out bool badPassword
205203
progressWindow.SetExtraText(Common.State_Extracting).SetGoal($"Extracting {entries.Count} files");
206204
});
207205

208-
var absoluteDestinationPath = _fileSystem.Path.GetFullPath(destinationDirectory + Path.AltDirectorySeparatorChar);
209-
210206
for (var i = 0; i < entries.Count; i++)
211207
{
212208
var entry = entries[i];
213-
var normalized = NormalizeEntryPath(entry.Key ?? string.Empty);
214-
if (string.IsNullOrWhiteSpace(normalized))
209+
if (!PathSafetyHelper.TryNormalizeRelativePath(entry.Key ?? string.Empty, out var normalized))
215210
continue;
216211

217212
if (!TryGetRelativeExtractionPath(normalized, out var relativePath))
218213
return Fail("Unexpected file in the test archive. Please contact the developers.");
219214

220-
var destinationPath = _fileSystem.Path.GetFullPath(_fileSystem.Path.Combine(destinationDirectory, relativePath));
221-
if (!destinationPath.StartsWith(absoluteDestinationPath, StringComparison.Ordinal))
215+
if (!PathSafetyHelper.TryGetPathWithinDirectory(destinationDirectory, relativePath, out var destinationPath))
222216
return Fail("The file path is outside the destination directory. Please contact the developers.");
223217

224218
var destinationDir = _fileSystem.Path.GetDirectoryName(destinationPath);
@@ -256,8 +250,6 @@ private static bool IsBadPasswordException(Exception ex)
256250
return ex.InnerException != null && IsBadPasswordException(ex.InnerException);
257251
}
258252

259-
private static string NormalizeEntryPath(string path) => path.Replace('\\', '/').TrimStart('/');
260-
261253
private bool TryGetRelativeExtractionPath(string normalizedPath, out string relativePath)
262254
{
263255
relativePath = string.Empty;
@@ -308,8 +300,6 @@ private OperationResult<List<string>> MoveExtractedFiles(string tempExtractionPa
308300
.Directory.EnumerateFiles(betaFolderSource, "*", SearchOption.AllDirectories)
309301
.Concat(_fileSystem.Directory.EnumerateFiles(xmlFolderSource, "*", SearchOption.AllDirectories));
310302

311-
var absoluteDestinationRoot = _fileSystem.Path.GetFullPath(destinationRoot + Path.AltDirectorySeparatorChar);
312-
313303
foreach (var file in sourceFiles)
314304
{
315305
var relativePath = _fileSystem.Path.GetRelativePath(tempExtractionPath, file);
@@ -319,9 +309,7 @@ private OperationResult<List<string>> MoveExtractedFiles(string tempExtractionPa
319309
continue;
320310
}
321311

322-
var destinationPath = _fileSystem.Path.Combine(destinationRoot, relativePath);
323-
var fullDestinationPath = _fileSystem.Path.GetFullPath(destinationPath);
324-
if (!fullDestinationPath.StartsWith(absoluteDestinationRoot, StringComparison.Ordinal))
312+
if (!PathSafetyHelper.TryGetPathWithinDirectory(destinationRoot, relativePath, out var destinationPath))
325313
return Fail("The file path is outside the destination directory. Please contact the developers.");
326314

327315
var destinationDirectory = _fileSystem.Path.GetDirectoryName(destinationPath);

‎WheelWizard/Features/Mods/ModInstallationService.cs‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System.Collections.ObjectModel;
1+
using System.Collections.ObjectModel;
22
using Avalonia.Threading;
33
using SharpCompress.Archives;
44
using WheelWizard.Helpers;
@@ -116,9 +116,6 @@ private static OperationResult ExtractModArchive(string file, string destination
116116
using var archive = archiveResult.Value;
117117
var totalEntries = archive.Entries.Count(entry => !entry.IsDirectory);
118118
var processedEntries = 0;
119-
var fullRoot = Path.GetFullPath(destinationDirectory);
120-
if (!Path.EndsInDirectorySeparator(fullRoot))
121-
fullRoot += Path.DirectorySeparatorChar;
122119

123120
foreach (var entry in archive.Entries.Where(entry => !entry.IsDirectory))
124121
{
@@ -131,17 +128,7 @@ private static OperationResult ExtractModArchive(string file, string destination
131128
});
132129

133130
var entryKey = entry.Key ?? string.Empty;
134-
var sanitizedKey = string.Join(
135-
Path.DirectorySeparatorChar.ToString(),
136-
entryKey
137-
.Split(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)
138-
.Where(segment => !string.IsNullOrWhiteSpace(segment))
139-
);
140-
141-
var entryDestinationPath = Path.Combine(destinationDirectory, sanitizedKey);
142-
var fullEntry = Path.GetFullPath(entryDestinationPath);
143-
144-
if (!fullEntry.StartsWith(fullRoot, StringComparison.OrdinalIgnoreCase))
131+
if (!PathSafetyHelper.TryGetPathWithinDirectory(destinationDirectory, entryKey, out var fullEntry))
145132
return Fail("Archive entry is outside of the destination directory.");
146133

147134
var directoryPath = Path.GetDirectoryName(fullEntry);
@@ -172,7 +159,7 @@ private static OperationResult<IArchive> OpenArchive(string filePath, string ext
172159

173160
try
174161
{
175-
return Ok(ArchiveFactory.Open(filePath));
162+
return Ok(ArchiveFactory.OpenArchive(filePath));
176163
}
177164
catch (Exception ex)
178165
{

‎WheelWizard/Helpers/DownloadHelper.cs‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using WheelWizard.Views.Popups.Generic;
1+
using WheelWizard.Views.Popups.Generic;
22

33
namespace WheelWizard.Helpers;
44

@@ -74,12 +74,17 @@ public static class DownloadHelper
7474

7575
if (!ForceGivenFilePath)
7676
{
77-
var finalUrl = response.RequestMessage.RequestUri.ToString();
78-
7977
// Check for filename in Content-Disposition or fallback to URL
8078
var contentDisposition = response.Content.Headers.ContentDisposition;
81-
var fileName = contentDisposition?.FileName?.Trim('"') ?? Path.GetFileName(new Uri(url).AbsolutePath);
82-
fileName = Path.ChangeExtension(fileName, Path.GetExtension(finalUrl));
79+
var fileName =
80+
contentDisposition?.FileNameStar ?? contentDisposition?.FileName ?? Path.GetFileName(new Uri(url).AbsolutePath);
81+
82+
if (!PathSafetyHelper.TryGetSafeFileName(fileName, out fileName))
83+
throw new InvalidOperationException("The server returned an invalid download filename.");
84+
85+
var finalExtension = Path.GetExtension(response.RequestMessage.RequestUri.AbsolutePath);
86+
if (!string.IsNullOrWhiteSpace(finalExtension))
87+
fileName = Path.ChangeExtension(fileName, finalExtension);
8388

8489
// Add extension if missing in file path
8590
if (!Path.HasExtension(fileName))
@@ -92,7 +97,8 @@ public static class DownloadHelper
9297
}
9398

9499
// Update resolvedFilePath with resolved fileName
95-
resolvedFilePath = Path.Combine(directory, fileName);
100+
if (!PathSafetyHelper.TryGetPathWithinDirectory(directory, fileName, out resolvedFilePath))
101+
throw new InvalidOperationException("The download path escaped the target directory.");
96102
}
97103

98104
var totalBytes = response.Content.Headers.ContentLength ?? -1;

‎WheelWizard/Helpers/PathSafetyHelper.cs‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
namespace WheelWizard.Helpers;
2+
3+
public static class PathSafetyHelper
4+
{
5+
public static bool TryGetPathWithinDirectory(string directory, string relativePath, out string fullPath)
6+
{
7+
fullPath = string.Empty;
8+
9+
if (!TryNormalizeRelativePath(relativePath, out var normalizedRelativePath))
10+
return false;
11+
12+
var destinationPath = Path.Combine(directory, normalizedRelativePath);
13+
var fullDirectory = Path.GetFullPath(directory);
14+
var candidatePath = Path.GetFullPath(destinationPath);
15+
16+
if (!IsPathWithinDirectory(fullDirectory, candidatePath))
17+
return false;
18+
19+
fullPath = candidatePath;
20+
return true;
21+
}
22+
23+
public static bool IsPathWithinDirectory(string directory, string path)
24+
{
25+
if (string.IsNullOrWhiteSpace(directory) || string.IsNullOrWhiteSpace(path))
26+
return false;
27+
28+
var fullDirectory = Path.GetFullPath(directory);
29+
var fullPath = Path.GetFullPath(path);
30+
var relativePath = Path.GetRelativePath(fullDirectory, fullPath);
31+
32+
return relativePath == "."
33+
|| (
34+
!Path.IsPathRooted(relativePath)
35+
&& !relativePath.Equals("..", StringComparison.Ordinal)
36+
&& !relativePath.StartsWith($"..{Path.DirectorySeparatorChar}", StringComparison.Ordinal)
37+
&& !relativePath.StartsWith($"..{Path.AltDirectorySeparatorChar}", StringComparison.Ordinal)
38+
);
39+
}
40+
41+
public static bool TryNormalizeRelativePath(string path, out string normalizedPath)
42+
{
43+
normalizedPath = string.Empty;
44+
45+
if (string.IsNullOrWhiteSpace(path))
46+
return false;
47+
48+
var trimmedPath = path.Trim();
49+
if (Path.IsPathFullyQualified(trimmedPath) || trimmedPath.StartsWith('/') || trimmedPath.StartsWith('\\'))
50+
return false;
51+
52+
var segments = trimmedPath.Replace('\\', '/').Split('/', StringSplitOptions.RemoveEmptyEntries).Where(segment => segment != ".");
53+
54+
var safeSegments = new List<string>();
55+
foreach (var segment in segments)
56+
{
57+
if (segment == ".." || segment.Contains(Path.VolumeSeparatorChar))
58+
return false;
59+
60+
safeSegments.Add(segment);
61+
}
62+
63+
if (safeSegments.Count == 0)
64+
return false;
65+
66+
normalizedPath = Path.Combine(safeSegments.ToArray());
67+
return true;
68+
}
69+
70+
public static bool TryGetSafeFileName(string fileName, out string safeFileName)
71+
{
72+
safeFileName = string.Empty;
73+
74+
if (string.IsNullOrWhiteSpace(fileName))
75+
return false;
76+
77+
var leafName = fileName.Trim().Trim('"').Replace('\\', '/').Split('/', StringSplitOptions.RemoveEmptyEntries).LastOrDefault();
78+
if (string.IsNullOrWhiteSpace(leafName) || leafName is "." or "..")
79+
return false;
80+
81+
if (leafName.IndexOfAny(Path.GetInvalidFileNameChars()) >= 0)
82+
return false;
83+
84+
safeFileName = leafName;
85+
return true;
86+
}
87+
}

‎WheelWizard/WheelWizard.csproj‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
33
<!-- Program Config -->
44
<StartupObject>WheelWizard.Program</StartupObject>
@@ -56,7 +56,8 @@
5656
<PackageReference Include="Serilog.Extensions.Logging" Version="8.0.0"/>
5757
<PackageReference Include="Serilog.Sinks.Console" Version="6.0.0"/>
5858
<PackageReference Include="Serilog.Sinks.File" Version="6.0.0"/>
59-
<PackageReference Include="SharpCompress" Version="0.39.0"/>
59+
<PackageReference Include="SharpCompress" Version="0.48.1"/>
60+
<PackageReference Include="Tmds.DBus.Protocol" Version="0.21.3"/>
6061
<PackageReference Include="TestableIO.System.IO.Abstractions.Analyzers" Version="2022.0.0">
6162
<PrivateAssets>all</PrivateAssets>
6263
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>

0 commit comments

Comments
 (0)