Skip to content

Commit 3e93416

Browse files
authored
Merge commit from fork
* Admit only valid OCI layout files when loading image. - Adds `AdmissionMapper` protocol for validating archive member paths and normalizing them to relative paths under the extraction root directory. - Renames `Reader.swift` to `ArchiveReader.swift` to match type name. - Rework `TempDir` to address `NSString.utf8String` deprecation warning in Swift 6.2.3. - Rework `ArchiveReader.extractContent()` to use an `AdmissionMapper` to validate and remap archive members before extracting. - Adds `IdentityAdmissionWrapper` for naive extraction. - Adds `NoSymlinkAdmissionWrapper` that only extracts regular files and directories under the extraction root. - Adds `OCIImageAdmissionWrapper` that only extracts valid OCI image layout paths. - Use `OCIImageAdmissionWrapper` for `cctl image load` and print rejected paths. * PR feedback. * Adds public init() for TrustedAdmissionMapper. * Replace AdmissionMapper with more secure extraction. - Adds FileDescriptor.mkdirSecure() to prevent root escapes on member pathnames, and to prevent symlink traversal. - Adds FileDescriptor.unlinkRecursive() to facilitate overwrites when there are multiple archive entries with the same member path. - Adds FileDescriptor.validateSymlinkTargetInRoot() to validate that extracted symlink targets do not escape the root. - Rewrite ArchiveReader.extractContents() to use secure path functions. * Remove unneeded symlink check, rename files. * Simplify the lexical normalizer workaround. * Reject member paths containing parent traversal components. * Remove unused lexical normalization workaround. * Fix leaking fds, extract absolute members as relative.
1 parent bbe4159 commit 3e93416

File tree

11 files changed

+1755
-36
lines changed

11 files changed

+1755
-36
lines changed

Package.resolved

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Package.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,10 @@ let package = Package(
130130
.target(
131131
name: "ContainerizationArchive",
132132
dependencies: [
133-
"CArchive",
134133
.product(name: "SystemPackage", package: "swift-system"),
134+
"CArchive",
135135
"ContainerizationExtras",
136+
"ContainerizationOS",
136137
],
137138
exclude: [
138139
"CArchive"
@@ -203,6 +204,7 @@ let package = Package(
203204
name: "ContainerizationOS",
204205
dependencies: [
205206
.product(name: "Logging", package: "swift-log"),
207+
.product(name: "SystemPackage", package: "swift-system"),
206208
"CShim",
207209
"ContainerizationError",
208210
],
@@ -213,6 +215,7 @@ let package = Package(
213215
.testTarget(
214216
name: "ContainerizationOSTests",
215217
dependencies: [
218+
.product(name: "SystemPackage", package: "swift-system"),
216219
"ContainerizationOS",
217220
"ContainerizationExtras",
218221
]

Sources/ContainerizationArchive/Reader.swift renamed to Sources/ContainerizationArchive/ArchiveReader.swift

Lines changed: 132 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//===----------------------------------------------------------------------===//
2-
// Copyright © 2025-2026 Apple Inc. and the Containerization project authors.
2+
// Copyright © 2026 Apple Inc. and the Containerization project authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -15,7 +15,9 @@
1515
//===----------------------------------------------------------------------===//
1616

1717
import CArchive
18+
import ContainerizationOS
1819
import Foundation
20+
import SystemPackage
1921

2022
/// A protocol for reading data in chunks, compatible with both `InputStream` and zero-allocation archive readers.
2123
public protocol ReadableStream {
@@ -45,6 +47,8 @@ public struct ArchiveEntryReader: ReadableStream {
4547

4648
/// A class responsible for reading entries from an archive file.
4749
public final class ArchiveReader {
50+
private static let blockSize = 65536
51+
4852
/// A pointer to the underlying `archive` C structure.
4953
var underlying: OpaquePointer?
5054
/// The file handle associated with the archive file being read.
@@ -176,36 +180,44 @@ extension ArchiveReader {
176180
}
177181

178182
/// Extracts the contents of an archive to the provided directory.
179-
/// Currently only handles regular files and directories present in the archive.
180-
public func extractContents(to directory: URL) throws {
183+
/// Rejects member paths that escape the root directory or traverse
184+
/// symbolic links, and uses a "last entry wins" replacement policy
185+
/// for an existing file at a path to be extracted.
186+
public func extractContents(to directory: URL) throws -> [String] {
187+
// Create the root directory with standard permissions
188+
// and create a FileDescriptor for secure path traveral.
181189
let fm = FileManager.default
190+
let rootFilePath = FilePath(directory.path)
191+
try fm.createDirectory(atPath: directory.path, withIntermediateDirectories: true)
192+
let rootFileDescriptor = try FileDescriptor.open(rootFilePath, .readOnly)
193+
defer { try? rootFileDescriptor.close() }
194+
195+
// Iterate and extract archive entries, collecting rejected paths.
182196
var foundEntry = false
183-
for (entry, data) in self {
184-
guard let p = entry.path else { continue }
185-
foundEntry = true
186-
let type = entry.fileType
187-
let target = directory.appending(path: p)
188-
switch type {
189-
case .regular:
190-
try data.write(to: target, options: .atomic)
191-
case .directory:
192-
try fm.createDirectory(at: target, withIntermediateDirectories: true)
193-
case .symbolicLink:
194-
guard let symlinkTarget = entry.symlinkTarget, let linkTargetURL = URL(string: symlinkTarget, relativeTo: target) else {
195-
continue
196-
}
197-
try fm.createSymbolicLink(at: target, withDestinationURL: linkTargetURL)
198-
default:
197+
var rejectedPaths = [String]()
198+
for (entry, dataReader) in self.makeStreamingIterator() {
199+
guard let memberPath = (entry.path.map { FilePath($0) }) else {
199200
continue
200201
}
201-
chmod(target.path(), entry.permissions)
202-
if let owner = entry.owner, let group = entry.group {
203-
chown(target.path(), owner, group)
202+
foundEntry = true
203+
204+
// Try to extract the entry, catching path validation errors
205+
let extracted = try extractEntry(
206+
entry: entry,
207+
dataReader: dataReader,
208+
memberPath: memberPath,
209+
rootFileDescriptor: rootFileDescriptor
210+
)
211+
212+
if !extracted {
213+
rejectedPaths.append(memberPath.string)
204214
}
205215
}
206216
guard foundEntry else {
207217
throw ArchiveError.failedToExtractArchive("no entries found in archive")
208218
}
219+
220+
return rejectedPaths
209221
}
210222

211223
/// This method extracts a given file from the archive.
@@ -226,4 +238,102 @@ extension ArchiveReader {
226238
}
227239
throw ArchiveError.failedToExtractArchive(" \(path) not found in archive")
228240
}
241+
242+
/// Extracts a single archive entry.
243+
/// Returns false if the entry was rejected due to path validation errors.
244+
/// Throws on system errors.
245+
private func extractEntry(
246+
entry: WriteEntry,
247+
dataReader: ArchiveEntryReader,
248+
memberPath: FilePath,
249+
rootFileDescriptor: FileDescriptor
250+
) throws -> Bool {
251+
guard let lastComponent = memberPath.lastComponent else {
252+
return false
253+
}
254+
let relativePath = memberPath.removingLastComponent()
255+
let type = entry.fileType
256+
257+
do {
258+
switch type {
259+
case .regular:
260+
try rootFileDescriptor.mkdirSecure(relativePath, makeIntermediates: true) { fd in
261+
// Remove existing entry if present (mimics containerd's "last entry wins" behavior)
262+
try? fd.unlinkRecursiveSecure(filename: lastComponent)
263+
264+
// Open file for writing using openat with O_NOFOLLOW to prevent TOC-TOU attacks
265+
let fileMode = entry.permissions & 0o777 // Mask to permission bits only
266+
let fileFd = openat(fd.rawValue, lastComponent.string, O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW, fileMode)
267+
guard fileFd >= 0 else {
268+
throw ArchiveError.failedToExtractArchive("failed to create file: \(memberPath)")
269+
}
270+
defer { close(fileFd) }
271+
272+
try Self.copyDataReaderToFd(dataReader: dataReader, fileFd: fileFd, memberPath: memberPath)
273+
setFileAttributes(fd: fileFd, entry: entry)
274+
}
275+
case .directory:
276+
try rootFileDescriptor.mkdirSecure(memberPath, makeIntermediates: true) { fd in
277+
setFileAttributes(fd: fd.rawValue, entry: entry)
278+
}
279+
case .symbolicLink:
280+
guard let targetPath = (entry.symlinkTarget.map { FilePath($0) }) else {
281+
return false
282+
}
283+
var symlinkCreated = false
284+
try rootFileDescriptor.mkdirSecure(relativePath, makeIntermediates: true) { fd in
285+
// Remove existing entry if present (mimics containerd's "last entry wins" behavior)
286+
try? fd.unlinkRecursiveSecure(filename: lastComponent)
287+
288+
guard symlinkat(targetPath.string, fd.rawValue, lastComponent.string) == 0 else {
289+
throw ArchiveError.failedToExtractArchive("failed to create symlink: \(targetPath) <- \(memberPath)")
290+
}
291+
symlinkCreated = true
292+
}
293+
return symlinkCreated
294+
default:
295+
return false
296+
}
297+
298+
return true
299+
} catch let error as SecurePathError {
300+
// Just reject path validation errors, don't fail the extraction
301+
switch error {
302+
case .systemError:
303+
// Fail for system errors
304+
throw error
305+
case .invalidRelativePath, .invalidPathComponent, .cannotFollowSymlink:
306+
return false
307+
}
308+
}
309+
}
310+
311+
private func setFileAttributes(fd: Int32, entry: WriteEntry) {
312+
fchmod(fd, entry.permissions)
313+
if let owner = entry.owner, let group = entry.group {
314+
fchown(fd, owner, group)
315+
}
316+
}
317+
318+
private static func copyDataReaderToFd(dataReader: ArchiveEntryReader, fileFd: Int32, memberPath: FilePath) throws {
319+
var buffer = [UInt8](repeating: 0, count: ArchiveReader.blockSize)
320+
while true {
321+
let bytesRead = buffer.withUnsafeMutableBufferPointer { bufferPtr in
322+
guard let baseAddress = bufferPtr.baseAddress else { return 0 }
323+
return dataReader.read(baseAddress, maxLength: bufferPtr.count)
324+
}
325+
326+
if bytesRead < 0 {
327+
throw ArchiveError.failedToExtractArchive("failed to read data for: \(memberPath)")
328+
}
329+
if bytesRead == 0 {
330+
break // EOF
331+
}
332+
333+
let bytesWritten = write(fileFd, buffer, bytesRead)
334+
guard bytesWritten == bytesRead else {
335+
throw ArchiveError.failedToExtractArchive("failed to write data for: \(memberPath)")
336+
}
337+
}
338+
}
229339
}

Sources/ContainerizationArchive/TempDir.swift

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,15 @@ import Foundation
2020
internal func createTemporaryDirectory(baseName: String) -> URL? {
2121
let url = FileManager.default.uniqueTemporaryDirectory().appendingPathComponent(
2222
"\(baseName).XXXXXX")
23-
guard let templatePathData = (url.absoluteURL.path as NSString).utf8String else {
24-
return nil
25-
}
26-
27-
let pathData = UnsafeMutablePointer(mutating: templatePathData)
28-
mkdtemp(pathData)
2923

30-
return URL(fileURLWithPath: String(cString: pathData), isDirectory: true)
24+
var path = url.absoluteURL.path
25+
return path.withUTF8 { utf8Bytes in
26+
var mutablePath = Array(utf8Bytes) + [0]
27+
return mutablePath.withUnsafeMutableBufferPointer { buffer -> URL? in
28+
guard let baseAddress = buffer.baseAddress else { return nil }
29+
mkdtemp(baseAddress)
30+
let resultPath = String(decoding: buffer[..<(buffer.count - 1)], as: UTF8.self)
31+
return URL(fileURLWithPath: resultPath, isDirectory: true)
32+
}
33+
}
3134
}

0 commit comments

Comments
 (0)