diff --git a/src/WingetCreateCore/Common/Constants.cs b/src/WingetCreateCore/Common/Constants.cs
index 4a6ca81a..6119c4fe 100644
--- a/src/WingetCreateCore/Common/Constants.cs
+++ b/src/WingetCreateCore/Common/Constants.cs
@@ -31,7 +31,7 @@ public static class Constants
///
/// App Id for the Winget-Create GitHub App.
///
- public const int GitHubAppId = 965520;
+ public const int GitHubAppId = 100205;
///
/// Link to the GitHub releases page for the winget-create tool.
diff --git a/src/WingetCreateCore/Common/PackageParser.cs b/src/WingetCreateCore/Common/PackageParser.cs
index 560c1a81..d64d617b 100644
--- a/src/WingetCreateCore/Common/PackageParser.cs
+++ b/src/WingetCreateCore/Common/PackageParser.cs
@@ -178,7 +178,7 @@ public static async Task DownloadFileAsync(string url, bool allowHttp, l
}
string urlFile = Path.GetFileName(url.Split('?').Last());
- string contentDispositionFile = response.Content.Headers.ContentDisposition?.FileName?.Trim('"');
+ string contentDispositionFile = Path.GetFileName(response.Content.Headers.ContentDisposition?.FileName?.Trim('"'));
string requestUrlFileName = Path.GetFileName(response.RequestMessage?.RequestUri?.ToString());
if (!Directory.Exists(InstallerDownloadPath))
@@ -186,9 +186,18 @@ public static async Task DownloadFileAsync(string url, bool allowHttp, l
Directory.CreateDirectory(InstallerDownloadPath);
}
- // If no relevant filename can be obtained for the installer download, use a temporary filename as last option.
- string targetFileName = contentDispositionFile.NullIfEmpty() ?? urlFile.NullIfEmpty() ?? requestUrlFileName.NullIfEmpty() ?? Path.GetTempFileName();
+ // If no relevant filename can be obtained for the installer download, use a random filename as last option.
+ string targetFileName = contentDispositionFile.NullIfEmpty() ?? urlFile.NullIfEmpty() ?? requestUrlFileName.NullIfEmpty() ?? Path.GetRandomFileName();
string targetFile = GetNumericFilename(Path.Combine(InstallerDownloadPath, targetFileName));
+
+ // Defense in depth: ensure the resolved path stays under the installer download directory.
+ string downloadRoot = Path.GetFullPath(InstallerDownloadPath).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
+ string fullTargetFile = Path.GetFullPath(targetFile);
+ if (!fullTargetFile.StartsWith(downloadRoot + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase))
+ {
+ throw new InvalidOperationException("Resolved download path escapes the installer download directory.");
+ }
+
using var targetFileStream = File.OpenWrite(targetFile);
var contentStream = await response.Content.ReadAsStreamAsync();
diff --git a/src/WingetCreateTests/WingetCreateTests/TestUtils.cs b/src/WingetCreateTests/WingetCreateTests/TestUtils.cs
index 99f5e916..3d05435a 100644
--- a/src/WingetCreateTests/WingetCreateTests/TestUtils.cs
+++ b/src/WingetCreateTests/WingetCreateTests/TestUtils.cs
@@ -97,6 +97,23 @@ public static void SetMockHttpResponseContent(string installerName)
PackageParser.SetHttpMessageHandler(httpMessageHandler);
}
+ ///
+ /// Sets the mock http response content along with a server-supplied Content-Disposition filename.
+ ///
+ /// File name of the installer.
+ /// The filename value to advertise in the Content-Disposition header.
+ public static void SetMockHttpResponseContent(string installerName, string contentDispositionFileName)
+ {
+ var content = new ByteArrayContent(File.ReadAllBytes(GetTestFile(installerName)));
+ content.Headers.ContentType = new MediaTypeHeaderValue("application/octet-stream");
+ content.Headers.ContentDisposition = new ContentDispositionHeaderValue("attachment")
+ {
+ FileName = contentDispositionFileName,
+ };
+ httpResponseMessage.Content = content;
+ PackageParser.SetHttpMessageHandler(httpMessageHandler);
+ }
+
///
/// Obtains the relative filepath of the resources test data directory.
///
@@ -170,6 +187,23 @@ public static List CreateResourceCopy(string resource, int numberOfCopie
return copyPaths;
}
+ ///
+ /// Deletes existing copies of the specified resource (base and numbered variants) from the test resources directory.
+ ///
+ /// Name of the resource file whose copies should be deleted.
+ public static void DeleteResourceCopies(string resourceName)
+ {
+ string resourcePath = GetTestFile(resourceName);
+ string directory = Path.GetDirectoryName(resourcePath);
+ string fileName = Path.GetFileNameWithoutExtension(resourcePath);
+ string fileExt = Path.GetExtension(resourcePath);
+
+ foreach (string file in Directory.GetFiles(directory, fileName + "*" + fileExt))
+ {
+ File.Delete(file);
+ }
+ }
+
///
/// Adds files to an existing test zip archive.
///
@@ -211,9 +245,21 @@ public static void RemoveFilesFromZip(string zipResourceName, List fileN
/// Name of the test files to delete.
public static void DeleteCachedFiles(List testFileNames)
{
+ string downloadDirectory = PackageParser.InstallerDownloadPath;
+ if (!Directory.Exists(downloadDirectory))
+ {
+ return;
+ }
+
foreach (string fileName in testFileNames)
{
- File.Delete(Path.Combine(PackageParser.InstallerDownloadPath, fileName));
+ string baseName = Path.GetFileNameWithoutExtension(fileName);
+ string fileExt = Path.GetExtension(fileName);
+
+ foreach (string filePath in Directory.GetFiles(downloadDirectory, baseName + "*" + fileExt))
+ {
+ File.Delete(filePath);
+ }
}
}
diff --git a/src/WingetCreateTests/WingetCreateTests/UnitTests/PackageParserTests.cs b/src/WingetCreateTests/WingetCreateTests/UnitTests/PackageParserTests.cs
index cc8575b1..ec8079ac 100644
--- a/src/WingetCreateTests/WingetCreateTests/UnitTests/PackageParserTests.cs
+++ b/src/WingetCreateTests/WingetCreateTests/UnitTests/PackageParserTests.cs
@@ -66,6 +66,37 @@ public void ParseExeInstallerFile()
ClassicAssert.AreEqual(InstallerType.Exe, manifests.InstallerManifest.Installers.First().InstallerType);
}
+ ///
+ /// Verifies that a server-supplied Content-Disposition filename containing directory
+ /// traversal sequences cannot cause the installer to be written outside the intended download
+ /// directory.
+ ///
+ [Test]
+ public void DownloadFileDispositionStaysInDownloadDirectory()
+ {
+ string downloadedPath = null;
+ try
+ {
+ string url = $"https://fakedomain.com/{TestConstants.TestExeInstaller}";
+ string invalidFileName = @"..\..\..\..\example.exe";
+ TestUtils.SetMockHttpResponseContent(TestConstants.TestExeInstaller, invalidFileName);
+
+ downloadedPath = PackageParser.DownloadFileAsync(url, false).Result;
+
+ string downloadRoot = Path.GetFullPath(PackageParser.InstallerDownloadPath).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
+ string fullDownloadedPath = Path.GetFullPath(downloadedPath);
+ Assert.That(fullDownloadedPath, Does.StartWith(downloadRoot + Path.DirectorySeparatorChar), "Downloaded file escaped the installer download directory.");
+ ClassicAssert.AreEqual("example.exe", Path.GetFileName(downloadedPath));
+ }
+ finally
+ {
+ if (downloadedPath != null && File.Exists(downloadedPath))
+ {
+ File.Delete(downloadedPath);
+ }
+ }
+ }
+
///
/// Downloads the MSI installer file from HTTPS localhost and parses the package to create a manifest object.
///
diff --git a/src/WingetCreateTests/WingetCreateTests/UnitTests/UpdateCommandTests.cs b/src/WingetCreateTests/WingetCreateTests/UnitTests/UpdateCommandTests.cs
index 5a35975e..544581c7 100644
--- a/src/WingetCreateTests/WingetCreateTests/UnitTests/UpdateCommandTests.cs
+++ b/src/WingetCreateTests/WingetCreateTests/UnitTests/UpdateCommandTests.cs
@@ -1381,24 +1381,41 @@ public async Task UpdateMultipleZipInstallers()
[Test]
public async Task UpdateZipWithMultipleNestedInstallers()
{
+ // Ensure a clean slate
+ TestUtils.DeleteResourceCopies(TestConstants.TestPortableInstaller);
+
// Create copies of test exe installer to be used as portable installers
List portableFilePaths = TestUtils.CreateResourceCopy(TestConstants.TestExeInstaller, 4, TestConstants.TestPortableInstaller);
- // Add the generated portable installers to the test zip installer
- TestUtils.AddFilesToZip(TestConstants.TestZipInstaller, portableFilePaths);
+ Manifests updatedManifests;
+ List initialManifestContent;
- // Delete cached zip installer from other test runs so that the modified zip installer is downloaded
- TestUtils.DeleteCachedFiles(new List { TestConstants.TestZipInstaller });
+ try
+ {
+ // Add the generated portable installers to the test zip installer
+ TestUtils.AddFilesToZip(TestConstants.TestZipInstaller, portableFilePaths);
- TestUtils.InitializeMockDownloads(TestConstants.TestZipInstaller);
- string installerUrl = $"https://fakedomain.com/{TestConstants.TestZipInstaller}";
- (UpdateCommand command, var initialManifestContent) = GetUpdateCommandAndManifestData("TestPublisher.ZipMultipleNestedInstallers", null, this.tempPath, new[] { $"{installerUrl}|x64", $"{installerUrl}|x86", $"{installerUrl}|arm", $"{installerUrl}|arm64|user", $"{installerUrl}|arm64|machine" });
+ // Delete cached zip installer from other test runs so that the modified zip installer is downloaded
+ TestUtils.DeleteCachedFiles(new List { TestConstants.TestZipInstaller });
- var updatedManifests = await RunUpdateCommand(command, initialManifestContent);
+ TestUtils.InitializeMockDownloads(TestConstants.TestZipInstaller);
+ string installerUrl = $"https://fakedomain.com/{TestConstants.TestZipInstaller}";
+ (UpdateCommand command, initialManifestContent) = GetUpdateCommandAndManifestData("TestPublisher.ZipMultipleNestedInstallers", null, this.tempPath, new[] { $"{installerUrl}|x64", $"{installerUrl}|x86", $"{installerUrl}|arm", $"{installerUrl}|arm64|user", $"{installerUrl}|arm64|machine" });
- // Perform test clean up before any assertions
- portableFilePaths.ForEach(File.Delete);
- TestUtils.RemoveFilesFromZip(TestConstants.TestZipInstaller, portableFilePaths.Select(Path.GetFileName).ToList());
+ updatedManifests = await RunUpdateCommand(command, initialManifestContent);
+ }
+ finally
+ {
+ // Perform test clean up before any assertions
+ portableFilePaths.ForEach(path =>
+ {
+ if (File.Exists(path))
+ {
+ File.Delete(path);
+ }
+ });
+ TestUtils.RemoveFilesFromZip(TestConstants.TestZipInstaller, portableFilePaths.Select(Path.GetFileName).ToList());
+ }
ClassicAssert.IsNotNull(updatedManifests, "Command should have succeeded");