From 2faaf9b4aff48a4745ef3d26c3f1450c1228fdf0 Mon Sep 17 00:00:00 2001 From: J Logan Date: Fri, 28 Aug 2026 13:32:16 -0700 Subject: [PATCH] EXT4: reject '..' components that escape the image root during unpack (#897) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #896. libarchive permits path traversal components (`.` and `..`) in entry pathnames during entry iteration. The library will only extract (in `archive_write_disk`) an entry if the normalized pathname resolves under the extraction root. Our EXT4 unarchiver currently prepends '/' to a pathname — an entry's own path, or a hardlink entry's link target, both of which are resolved through the same normalization function — and then attempts to create the corresponding directory entry or link regardless of whether the original relative path would escape the root, yielding a filesystem image that is incorrect at least and corrupt at worst. Valid OCI archive-type content blobs should never contain escaping pathnames. The unarchiver should treat such pathnames as garbage input and throw an exception on the first such pathname, whether it originates from an entry path or a hardlink target. --- .../Formatter+Unpack.swift | 30 ++-- .../TestFormatterUnpack.swift | 128 ++++++++++++++++++ 2 files changed, 145 insertions(+), 13 deletions(-) diff --git a/Sources/ContainerizationEXT4/Formatter+Unpack.swift b/Sources/ContainerizationEXT4/Formatter+Unpack.swift index 0833eb32..d957cd7e 100644 --- a/Sources/ContainerizationEXT4/Formatter+Unpack.swift +++ b/Sources/ContainerizationEXT4/Formatter+Unpack.swift @@ -110,12 +110,11 @@ extension EXT4.Formatter { for (entry, streamReader) in reader.makeStreamingIterator() { try Task.checkCancellation() - guard var pathEntry = entry.path else { + guard let pathEntry = entry.path else { continue } - pathEntry = preProcessPath(s: pathEntry) - let path = FilePath(pathEntry) + let path = try preProcessPath(s: pathEntry) if path.base.hasPrefix(".wh.") { if path.base == ".wh..wh..opq" { // whiteout directory @@ -136,8 +135,7 @@ extension EXT4.Formatter { } if let hardlink = entry.hardlink { - let hl = preProcessPath(s: hardlink) - hardlinks[path] = FilePath(hl) + hardlinks[path] = try preProcessPath(s: hardlink) if let progress { await progress([.addItems(1)]) } @@ -190,15 +188,17 @@ extension EXT4.Formatter { } } - private func preProcessPath(s: String) -> String { - var p = s - if p.hasPrefix("./") { - p = String(p.dropFirst()) + private func preProcessPath(s: String) throws -> FilePath { + // Normalize as a relative path (root stripped) before checking: lexicallyNormalized() + // silently clamps an unresolvable leading ".." to "/" on an already-absolute path, + // which would hide a traversal attempt instead of surfacing it as a leading + // .parentDirectory component. Any ".." that survives normalization of the relative + // form is a genuine attempt to escape above the image root. + let relative = FilePath(root: nil, FilePath(s).components).lexicallyNormalized() + if relative.components.first?.kind == .parentDirectory { + throw UnpackError.pathTraversal(s) } - if !p.hasPrefix("/") { - p = "/" + p - } - return p + return FilePath("/").pushing(relative) } } @@ -208,6 +208,8 @@ public enum UnpackError: Swift.Error, CustomStringConvertible, Sendable, Equatab case invalidName(_ name: String) /// A circular link is found. case circularLinks + /// The path attempts to traverse above the image root. + case pathTraversal(_ path: String) /// The description of the error. public var description: String { @@ -216,6 +218,8 @@ public enum UnpackError: Swift.Error, CustomStringConvertible, Sendable, Equatab return "'\(name)' is an invalid name" case .circularLinks: return "circular links found" + case .pathTraversal(let path): + return "'\(path)' attempts to traverse above the image root" } } } diff --git a/Tests/ContainerizationEXT4Tests/TestFormatterUnpack.swift b/Tests/ContainerizationEXT4Tests/TestFormatterUnpack.swift index 3b8590f9..0a5fd2f6 100644 --- a/Tests/ContainerizationEXT4Tests/TestFormatterUnpack.swift +++ b/Tests/ContainerizationEXT4Tests/TestFormatterUnpack.swift @@ -452,6 +452,134 @@ struct UnpackProgressTest { #expect(try reader.stat("/var/lib/data/file.txt").inode.mode.isReg()) #expect(try reader.readFile(at: "/var/lib/data/file.txt") == payload) } + + @Test func unpackRejectsPathTraversalInEntryPath() async throws { + let tempDir = FileManager.default.uniqueTemporaryDirectory() + let archivePath = tempDir.appendingPathComponent("traversal.tar.gz", isDirectory: false) + let fsPath = FilePath(tempDir.appendingPathComponent("traversal.ext4.img", isDirectory: false)) + defer { try? FileManager.default.removeItem(at: tempDir) } + + let archiver = try ArchiveWriter( + configuration: ArchiveWriterConfiguration(format: .paxRestricted, filter: .gzip)) + try archiver.open(file: archivePath) + try archiver.writeEntry( + entry: WriteEntry.file(path: "../../etc/passwd", permissions: 0o644), data: nil) + try archiver.finishEncoding() + + let formatter = try EXT4.Formatter(fsPath) + await #expect(throws: UnpackError.self) { + try await formatter.unpack(source: archivePath) + } + } + + @Test func unpackRejectsPathTraversalInHardlinkTarget() async throws { + let tempDir = FileManager.default.uniqueTemporaryDirectory() + let archivePath = tempDir.appendingPathComponent("traversal-hardlink.tar.gz", isDirectory: false) + let fsPath = FilePath(tempDir.appendingPathComponent("traversal-hardlink.ext4.img", isDirectory: false)) + defer { try? FileManager.default.removeItem(at: tempDir) } + + let archiver = try ArchiveWriter( + configuration: ArchiveWriterConfiguration(format: .paxRestricted, filter: .gzip)) + try archiver.open(file: archivePath) + try archiver.writeEntry( + entry: WriteEntry.hardlink(path: "/bin/app", target: "../../etc/shadow"), data: nil) + try archiver.finishEncoding() + + let formatter = try EXT4.Formatter(fsPath) + await #expect(throws: UnpackError.self) { + try await formatter.unpack(source: archivePath) + } + } + + @Test func unpackToleratesLeadingDotSlashAndTrailingSlash() async throws { + let tempDir = FileManager.default.uniqueTemporaryDirectory() + let archivePath = tempDir.appendingPathComponent("stylistic.tar.gz", isDirectory: false) + let fsPath = FilePath(tempDir.appendingPathComponent("stylistic.ext4.img", isDirectory: false)) + defer { try? FileManager.default.removeItem(at: tempDir) } + + let archiver = try ArchiveWriter( + configuration: ArchiveWriterConfiguration(format: .paxRestricted, filter: .gzip)) + try archiver.open(file: archivePath) + try archiver.writeEntry(entry: WriteEntry.dir(path: "./etc/", permissions: 0o755), data: nil) + try archiver.writeEntry( + entry: WriteEntry.file(path: "./etc/hosts", permissions: 0o644), data: nil) + try archiver.finishEncoding() + + let formatter = try EXT4.Formatter(fsPath) + try await formatter.unpack(source: archivePath) // must not throw + try formatter.close() + + let reader = try EXT4.EXT4Reader(blockDevice: fsPath) + #expect(try reader.stat("/etc").inode.mode.isDir()) + #expect(try reader.stat("/etc/hosts").inode.mode.isReg()) + } + + @Test func unpackResolvesHarmlessInternalDotDot() async throws { + // "foo/../bar" never actually leaves the image root once resolved; it should + // land at "/bar", not be rejected outright. + let tempDir = FileManager.default.uniqueTemporaryDirectory() + let archivePath = tempDir.appendingPathComponent("internal-dotdot.tar.gz", isDirectory: false) + let fsPath = FilePath(tempDir.appendingPathComponent("internal-dotdot.ext4.img", isDirectory: false)) + defer { try? FileManager.default.removeItem(at: tempDir) } + + let archiver = try ArchiveWriter( + configuration: ArchiveWriterConfiguration(format: .paxRestricted, filter: .gzip)) + try archiver.open(file: archivePath) + try archiver.writeEntry( + entry: WriteEntry.file(path: "foo/../bar/baz", permissions: 0o644), data: nil) + try archiver.finishEncoding() + + let formatter = try EXT4.Formatter(fsPath) + try await formatter.unpack(source: archivePath) // must not throw + try formatter.close() + + let reader = try EXT4.EXT4Reader(blockDevice: fsPath) + #expect(try reader.stat("/bar/baz").inode.mode.isReg()) + #expect(throws: (any Error).self) { + try reader.stat("/foo") + } + } + + @Test func unpackRejectsDotDotEscapingBeyondPrefix() async throws { + // "foo/../../etc/passwd" only has one real component to cancel against, so + // the second ".." is a genuine attempt to escape above the image root. + let tempDir = FileManager.default.uniqueTemporaryDirectory() + let archivePath = tempDir.appendingPathComponent("escape-beyond-prefix.tar.gz", isDirectory: false) + let fsPath = FilePath(tempDir.appendingPathComponent("escape-beyond-prefix.ext4.img", isDirectory: false)) + defer { try? FileManager.default.removeItem(at: tempDir) } + + let archiver = try ArchiveWriter( + configuration: ArchiveWriterConfiguration(format: .paxRestricted, filter: .gzip)) + try archiver.open(file: archivePath) + try archiver.writeEntry( + entry: WriteEntry.file(path: "foo/../../etc/passwd", permissions: 0o644), data: nil) + try archiver.finishEncoding() + + let formatter = try EXT4.Formatter(fsPath) + await #expect(throws: UnpackError.self) { + try await formatter.unpack(source: archivePath) + } + } + + @Test func unpackRejectsAlreadyAbsoluteTraversal() async throws { + // A leading "/" must not let lexical normalization silently clamp the escape away. + let tempDir = FileManager.default.uniqueTemporaryDirectory() + let archivePath = tempDir.appendingPathComponent("absolute-traversal.tar.gz", isDirectory: false) + let fsPath = FilePath(tempDir.appendingPathComponent("absolute-traversal.ext4.img", isDirectory: false)) + defer { try? FileManager.default.removeItem(at: tempDir) } + + let archiver = try ArchiveWriter( + configuration: ArchiveWriterConfiguration(format: .paxRestricted, filter: .gzip)) + try archiver.open(file: archivePath) + try archiver.writeEntry( + entry: WriteEntry.file(path: "/../../etc/passwd", permissions: 0o644), data: nil) + try archiver.finishEncoding() + + let formatter = try EXT4.Formatter(fsPath) + await #expect(throws: UnpackError.self) { + try await formatter.unpack(source: archivePath) + } + } } extension ContainerizationArchive.WriteEntry {