From d17c257fe130c295e030aa07fe4d06eaf8bf506d Mon Sep 17 00:00:00 2001 From: jt Date: Sun, 4 Oct 2026 13:18:39 -0700 Subject: [PATCH] Fix Unix Node extraction with safe relative symlinks Bundled 7-Zip 26.01 rejects Node's corepack/npm/npx links because their relative targets contain parent segments, aborting AI Toolkit setup. Extract only the Unix Node prerequisite through a constrained TAR/GZip reader that preserves file permissions and defers links until their archive-extracted regular targets exist. Reject traversal, existing links, link chains, hard links and special files. Cover these boundaries and the Linux/macOS Node layouts with 28 regression cases. Other archive consumers retain their 7-Zip checks. Co-Authored-By: Codex (GPT-6) --- CHANGELOG.md | 4 + .../Helpers/UnixPrerequisiteHelper.cs | 2 +- .../Helper/TarGZipExtractor.cs | 113 +++++++ .../Core/TarGZipExtractorTests.cs | 285 ++++++++++++++++++ 4 files changed, 403 insertions(+), 1 deletion(-) create mode 100644 StabilityMatrix.Core/Helper/TarGZipExtractor.cs create mode 100644 StabilityMatrix.Tests/Core/TarGZipExtractorTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index ae12ceda2..e9431119c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ All notable changes to Stability Matrix will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning 2.0](https://semver.org/spec/v2.0.0.html). +## Unreleased +### Fixed +- Fixed [#1767](https://github.com/LykosAI/StabilityMatrix/issues/1767) - **AI-Toolkit** installation failing on Linux while unpacking Node.js with a "Dangerous link path" error. + ## v2.16.4 ### Added - Added **Comfy Kitchen Attention** (`--use-ck-attention`) as a **Cross Attention Method** launch option for ComfyUI and ComfyUI-Zluda - thanks to @e-nord! diff --git a/StabilityMatrix.Avalonia/Helpers/UnixPrerequisiteHelper.cs b/StabilityMatrix.Avalonia/Helpers/UnixPrerequisiteHelper.cs index 5d1250493..701753292 100644 --- a/StabilityMatrix.Avalonia/Helpers/UnixPrerequisiteHelper.cs +++ b/StabilityMatrix.Avalonia/Helpers/UnixPrerequisiteHelper.cs @@ -560,7 +560,7 @@ public async Task InstallNodeIfNecessary(IProgress? progress = n ); // unzip - await ArchiveHelper.Extract7ZAuto(nodeDownloadPath, AssetsDir); + await TarGZipExtractor.ExtractAsync(nodeDownloadPath, AssetsDir); var nodeDir = Compat.IsMacOS ? AssetsDir.JoinDir("node-v20.19.3-darwin-arm64") diff --git a/StabilityMatrix.Core/Helper/TarGZipExtractor.cs b/StabilityMatrix.Core/Helper/TarGZipExtractor.cs new file mode 100644 index 000000000..e3f3ee497 --- /dev/null +++ b/StabilityMatrix.Core/Helper/TarGZipExtractor.cs @@ -0,0 +1,113 @@ +using System.Formats.Tar; +using System.IO.Compression; + +namespace StabilityMatrix.Core.Helper; + +/// +/// Extracts gzip-compressed tar archives containing directories, regular files, and relative file symlinks. +/// Links must target regular files inside the destination. Hard links and special files are rejected. +/// +public static class TarGZipExtractor +{ + /// + /// Extracts files with their Unix permissions and creates links after their targets have been extracted. + /// Existing symbolic links or junctions beneath the destination are never followed or overwritten. + /// + public static async Task ExtractAsync(string archivePath, string outputDirectory) + { + var root = Path.TrimEndingDirectorySeparator(Path.GetFullPath(outputDirectory)); + Directory.CreateDirectory(root); + EnsureNoLinks(root, root); + + var links = new List<(string Path, string Target, string Name)>(); + var files = new HashSet( + OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal + ); + await using var archive = File.OpenRead(archivePath); + await using var gzip = new GZipStream(archive, CompressionMode.Decompress); + await using var reader = new TarReader(gzip); + while (await reader.GetNextEntryAsync().ConfigureAwait(false) is { } entry) + { + var path = GetContainedPath(root, root, entry.Name); + EnsureNoLinks(root, path); + switch (entry.EntryType) + { + case TarEntryType.Directory: + Directory.CreateDirectory(path); + break; + case TarEntryType.RegularFile: + case TarEntryType.V7RegularFile: + Directory.CreateDirectory(Path.GetDirectoryName(path)!); + await entry.ExtractToFileAsync(path, overwrite: true).ConfigureAwait(false); + files.Add(path); + break; + case TarEntryType.SymbolicLink: + var target = GetContainedPath(root, Path.GetDirectoryName(path)!, entry.LinkName); + links.Add((path, target, entry.LinkName)); + break; + default: + throw new IOException($"Unsupported tar entry type: {entry.EntryType}."); + } + } + + // Create links after files so no archive entry can write through an archive-created link. + // Requiring regular targets also prevents link chains and directory-link traversal. + foreach (var (path, target, name) in links) + { + EnsureNoLinks(root, path); + EnsureNoLinks(root, target); + if (!files.Contains(target) || !File.Exists(target)) + { + throw new IOException($"Tar link target is not a regular file: {name}."); + } + Directory.CreateDirectory(Path.GetDirectoryName(path)!); + // Use the checked path: retaining segments such as "link/../file" could traverse + // an existing link even though the normalized target is inside the destination. + File.CreateSymbolicLink(path, Path.GetRelativePath(Path.GetDirectoryName(path)!, target)); + } + } + + private static string GetContainedPath(string root, string parent, string name) + { + // TAR uses forward slashes. Reject drive/UNC spellings as well, even on Unix. + if (string.IsNullOrEmpty(name) || name.StartsWith('/') || name.Contains('\\') || name.Contains(':')) + { + throw new IOException($"Invalid tar path: {name}."); + } + + var path = Path.GetFullPath(Path.Combine(parent, name)); + var comparison = OperatingSystem.IsWindows() + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + if ( + !path.Equals(root, comparison) && !path.StartsWith(root + Path.DirectorySeparatorChar, comparison) + ) + { + throw new IOException($"Tar path is outside the destination: {name}."); + } + return path; + } + + private static void EnsureNoLinks(string root, string path) + { + var current = path; + while (true) + { + try + { + if ((File.GetAttributes(current) & FileAttributes.ReparsePoint) != 0) + { + throw new IOException($"Tar extraction cannot traverse a link: {current}."); + } + } + catch (FileNotFoundException) { } + catch (DirectoryNotFoundException) { } + + if (current == root) + { + return; + } + current = Path.GetDirectoryName(current)!; + } + } +} diff --git a/StabilityMatrix.Tests/Core/TarGZipExtractorTests.cs b/StabilityMatrix.Tests/Core/TarGZipExtractorTests.cs new file mode 100644 index 000000000..5de9ea6d1 --- /dev/null +++ b/StabilityMatrix.Tests/Core/TarGZipExtractorTests.cs @@ -0,0 +1,285 @@ +using System.Formats.Tar; +using System.IO.Compression; +using System.Text; +using StabilityMatrix.Core.Helper; + +namespace StabilityMatrix.Tests.Core; + +[TestClass] +public class TarGZipExtractorTests +{ + private string workspace = null!; + private string output = null!; + private string archive = null!; + + [TestInitialize] + public void Initialize() + { + workspace = Path.Combine(Path.GetTempPath(), $"tar-extraction-{Guid.NewGuid():N}"); + output = Path.Combine(workspace, "output with spaces"); + archive = Path.Combine(workspace, "archive.tar.gz"); + Directory.CreateDirectory(workspace); + } + + [TestCleanup] + public void Cleanup() => Directory.Delete(workspace, recursive: true); + + [TestMethod] + public async Task ExtractAsync_RegularFilesAndDirectories() + { + WriteArchive( + new PaxTarEntry(TarEntryType.Directory, "node/lib/"), + FileEntry("node/lib/file with spaces", "contents") + ); + + await TarGZipExtractor.ExtractAsync(archive, output); + + Assert.AreEqual("contents", File.ReadAllText(Path.Combine(output, "node/lib/file with spaces"))); + } + + [DataTestMethod] + [DataRow("node-v20.19.3-linux-x64")] + [DataRow("node-v20.19.3-darwin-arm64")] + public async Task ExtractAsync_NodeLinksBeforeTargetsAndExecutableMode(string folder) + { + RequireSymlinks(); + var names = new[] { "npm", "npx", "corepack" }; + var targets = new[] + { + "../lib/node_modules/npm/bin/npm-cli.js", + "../lib/node_modules/npm/bin/npx-cli.js", + "../lib/node_modules/corepack/dist/corepack.js", + }; + var entries = names.Select((name, i) => LinkEntry($"{folder}/bin/{name}", targets[i])).ToList(); + entries.Add(FileEntry($"{folder}/bin/node", "node")); + entries.AddRange(targets.Select(target => FileEntry($"{folder}/bin/{target}", "script"))); + WriteArchive(entries.ToArray()); + + await TarGZipExtractor.ExtractAsync(archive, output); + + // The installer moves the whole distribution after extraction; relative links must survive it. + var installed = Path.Combine(output, "nodejs"); + Directory.Move(Path.Combine(output, folder), installed); + for (var i = 0; i < names.Length; i++) + { + var path = Path.Combine(installed, "bin", names[i]); + Assert.AreEqual(targets[i], new FileInfo(path).LinkTarget); + Assert.AreEqual("script", File.ReadAllText(path)); + } + if (!OperatingSystem.IsWindows()) + { + Assert.IsTrue( + File.GetUnixFileMode(Path.Combine(installed, "bin/node")).HasFlag(UnixFileMode.UserExecute) + ); + } + } + + [DataTestMethod] + [DataRow("../outside")] + [DataRow("../output with spaces-sibling/outside")] + [DataRow("/absolute")] + [DataRow("C:/outside")] + [DataRow("C:outside")] + [DataRow("\\\\server\\share\\outside")] + [DataRow("..\\outside")] + public async Task ExtractAsync_RejectsUnsafeEntryPaths(string name) + { + WriteArchive(FileEntry(name, "unsafe")); + + await Assert.ThrowsExceptionAsync(() => TarGZipExtractor.ExtractAsync(archive, output)); + + Assert.IsFalse(File.Exists(Path.Combine(workspace, "outside"))); + } + + [DataTestMethod] + [DataRow("../../../outside")] + [DataRow("/absolute")] + [DataRow("C:/outside")] + [DataRow("..\\..\\outside")] + public async Task ExtractAsync_RejectsUnsafeLinkTargets(string target) + { + WriteArchive(LinkEntry("node/bin/npm", target)); + + await Assert.ThrowsExceptionAsync(() => TarGZipExtractor.ExtractAsync(archive, output)); + } + + [DataTestMethod] + [DataRow(TarEntryType.HardLink)] + [DataRow(TarEntryType.Fifo)] + [DataRow(TarEntryType.CharacterDevice)] + [DataRow(TarEntryType.BlockDevice)] + public async Task ExtractAsync_RejectsHardLinksAndSpecialFiles(TarEntryType type) + { + var entry = new PaxTarEntry(type, "unsafe"); + if (type == TarEntryType.HardLink) + { + entry.LinkName = "../outside"; + } + WriteArchive(entry); + + await Assert.ThrowsExceptionAsync(() => TarGZipExtractor.ExtractAsync(archive, output)); + } + + [TestMethod] + public async Task ExtractAsync_RejectsMissingLinkTarget() + { + WriteArchive(LinkEntry("node/bin/npm", "../missing")); + + await Assert.ThrowsExceptionAsync(() => TarGZipExtractor.ExtractAsync(archive, output)); + } + + [TestMethod] + public async Task ExtractAsync_RejectsTargetNotExtractedFromArchive() + { + Directory.CreateDirectory(output); + File.WriteAllText(Path.Combine(output, "existing"), "sentinel"); + WriteArchive(LinkEntry("link", "existing")); + + await Assert.ThrowsExceptionAsync(() => TarGZipExtractor.ExtractAsync(archive, output)); + + Assert.AreEqual("sentinel", File.ReadAllText(Path.Combine(output, "existing"))); + } + + [TestMethod] + public async Task ExtractAsync_RejectsAbsoluteEntryInsideDestination() + { + WriteArchive(FileEntry(Path.Combine(output, "file").Replace('\\', '/'), "unsafe")); + + await Assert.ThrowsExceptionAsync(() => TarGZipExtractor.ExtractAsync(archive, output)); + } + + [TestMethod] + public async Task ExtractAsync_RejectsLinkedDestinationRoot() + { + RequireSymlinks(); + var outside = Path.Combine(workspace, "outside"); + Directory.CreateDirectory(outside); + Directory.CreateSymbolicLink(output, outside); + WriteArchive(FileEntry("file", "unsafe")); + + await Assert.ThrowsExceptionAsync(() => TarGZipExtractor.ExtractAsync(archive, output)); + + Assert.IsFalse(File.Exists(Path.Combine(outside, "file"))); + } + + [TestMethod] + public async Task ExtractAsync_RejectsArchiveDirectoryLinks() + { + WriteArchive( + new PaxTarEntry(TarEntryType.Directory, "node/lib/"), + LinkEntry("node/bin", "lib"), + FileEntry("node/bin/file", "unsafe") + ); + + await Assert.ThrowsExceptionAsync(() => TarGZipExtractor.ExtractAsync(archive, output)); + + Assert.IsFalse(File.Exists(Path.Combine(output, "node/lib/file"))); + } + + [TestMethod] + public async Task ExtractAsync_RejectsLinkChains() + { + RequireSymlinks(); + WriteArchive(FileEntry("file", "safe"), LinkEntry("link", "file"), LinkEntry("chain", "link")); + + await Assert.ThrowsExceptionAsync(() => TarGZipExtractor.ExtractAsync(archive, output)); + } + + [DataTestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task ExtractAsync_RejectsExistingDirectoryLink(bool broken) + { + RequireSymlinks(); + var outside = Path.Combine(workspace, "outside"); + if (!broken) + { + Directory.CreateDirectory(outside); + File.WriteAllText(Path.Combine(outside, "file"), "sentinel"); + } + Directory.CreateDirectory(output); + Directory.CreateSymbolicLink(Path.Combine(output, "node"), outside); + WriteArchive(FileEntry("node/file", "unsafe")); + + await Assert.ThrowsExceptionAsync(() => TarGZipExtractor.ExtractAsync(archive, output)); + + if (!broken) + { + Assert.AreEqual("sentinel", File.ReadAllText(Path.Combine(outside, "file"))); + } + else + { + Assert.IsFalse(Directory.Exists(outside)); + } + } + + [TestMethod] + public async Task ExtractAsync_RejectsExistingFileLink() + { + RequireSymlinks(); + var outside = Path.Combine(workspace, "outside"); + File.WriteAllText(outside, "sentinel"); + Directory.CreateDirectory(output); + File.CreateSymbolicLink(Path.Combine(output, "file"), outside); + WriteArchive(FileEntry("file", "unsafe")); + + await Assert.ThrowsExceptionAsync(() => TarGZipExtractor.ExtractAsync(archive, output)); + + Assert.AreEqual("sentinel", File.ReadAllText(outside)); + } + + [TestMethod] + public async Task ExtractAsync_NormalizesLinkTargetBeforeCreatingLink() + { + RequireSymlinks(); + var outside = Path.Combine(workspace, "outside"); + Directory.CreateDirectory(Path.Combine(outside, "directory")); + File.WriteAllText(Path.Combine(outside, "file"), "sentinel"); + Directory.CreateDirectory(output); + Directory.CreateSymbolicLink(Path.Combine(output, "existing"), Path.Combine(outside, "directory")); + WriteArchive(FileEntry("file", "safe"), LinkEntry("link", "existing/../file")); + + await TarGZipExtractor.ExtractAsync(archive, output); + + Assert.AreEqual("file", new FileInfo(Path.Combine(output, "link")).LinkTarget); + Assert.AreEqual("safe", File.ReadAllText(Path.Combine(output, "link"))); + Assert.AreEqual("sentinel", File.ReadAllText(Path.Combine(outside, "file"))); + } + + private void RequireSymlinks() + { + var target = Path.Combine(workspace, "symlink-probe-target"); + var link = Path.Combine(workspace, "symlink-probe"); + File.WriteAllText(target, "probe"); + try + { + File.CreateSymbolicLink(link, target); + } + catch (Exception e) when (e is IOException or UnauthorizedAccessException) + { + Assert.Inconclusive($"This host cannot create symlinks: {e.Message}"); + } + } + + private void WriteArchive(params TarEntry[] entries) + { + using var stream = File.Create(archive); + using var gzip = new GZipStream(stream, CompressionMode.Compress); + using var writer = new TarWriter(gzip, TarEntryFormat.Pax); + foreach (var entry in entries) + { + writer.WriteEntry(entry); + entry.DataStream?.Dispose(); + } + } + + private static PaxTarEntry FileEntry(string name, string contents) => + new(TarEntryType.RegularFile, name) + { + DataStream = new MemoryStream(Encoding.UTF8.GetBytes(contents)), + Mode = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute, + }; + + private static PaxTarEntry LinkEntry(string name, string target) => + new(TarEntryType.SymbolicLink, name) { LinkName = target }; +}