Skip to content

Commit 9c42c54

Browse files
stephentyronestephentyrone
authored
Merge pull request #189 from stephentyrone/enforce-file-path-invariants-in-decode
Ensure that FilePath and SystemString's invariants are enforced when …
2 parents 19e7f0f + 3841a75 commit 9c42c54

File tree

4 files changed

+189
-9
lines changed

4 files changed

+189
-9
lines changed

‎Sources/System/FilePath/FilePath.swift‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,22 @@ extension FilePath {
6666
}
6767

6868
@available(/*System 0.0.1: macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0*/iOS 8, *)
69-
extension FilePath: Hashable, Codable {}
69+
extension FilePath: Hashable, Codable {
70+
// Encoder is synthesized; it probably should have been explicit and used
71+
// a single-value container, but making that change now is somewhat risky.
72+
73+
// Decoder is written explicitly to ensure that we validate invariants on
74+
// untrusted input.
75+
public init(from decoder: any Decoder) throws {
76+
let container = try decoder.container(keyedBy: CodingKeys.self)
77+
self._storage = try container.decode(SystemString.self, forKey: ._storage)
78+
guard _invariantsSatisfied() else {
79+
throw DecodingError.dataCorruptedError(
80+
forKey: ._storage,
81+
in: container,
82+
debugDescription:
83+
"Encoding does not satisfy the invariants of FilePath"
84+
)
85+
}
86+
}
87+
}

‎Sources/System/FilePath/FilePathParsing.swift‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -375,13 +375,18 @@ extension FilePath {
375375
// MARK: - Invariants
376376
@available(/*System 0.0.1: macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0*/iOS 8, *)
377377
extension FilePath {
378-
internal func _invariantCheck() {
379-
#if DEBUG
378+
internal func _invariantsSatisfied() -> Bool {
380379
var normal = self
381380
normal._normalizeSeparators()
382-
precondition(self == normal)
383-
precondition(!self._storage._hasTrailingSeparator())
384-
precondition(_hasRoot == (self.root != nil))
381+
guard self == normal else { return false }
382+
guard !self._storage._hasTrailingSeparator() else { return false }
383+
guard _hasRoot == (self.root != nil) else { return false }
384+
return true
385+
}
386+
387+
internal func _invariantCheck() {
388+
#if DEBUG
389+
precondition(_invariantsSatisfied())
385390
#endif // DEBUG
386391
}
387392
}

‎Sources/System/SystemString.swift‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,18 @@ extension SystemString {
9696
}
9797

9898
extension SystemString {
99+
fileprivate func _invariantsSatisfied() -> Bool {
100+
guard !nullTerminatedStorage.isEmpty else { return false }
101+
guard nullTerminatedStorage.last! == .null else { return false }
102+
guard nullTerminatedStorage.firstIndex(of: .null) == length else {
103+
return false
104+
}
105+
return true
106+
}
107+
99108
fileprivate func _invariantCheck() {
100109
#if DEBUG
101-
precondition(nullTerminatedStorage.last! == .null)
102-
precondition(nullTerminatedStorage.firstIndex(of: .null) == length)
110+
precondition(_invariantsSatisfied())
103111
#endif // DEBUG
104112
}
105113
}
@@ -165,7 +173,27 @@ extension SystemString: RangeReplaceableCollection {
165173
}
166174
}
167175

168-
extension SystemString: Hashable, Codable {}
176+
extension SystemString: Hashable, Codable {
177+
// Encoder is synthesized; it probably should have been explicit and used
178+
// a single-value container, but making that change now is somewhat risky.
179+
180+
// Decoder is written explicitly to ensure that we validate invariants on
181+
// untrusted input.
182+
public init(from decoder: any Decoder) throws {
183+
let container = try decoder.container(keyedBy: CodingKeys.self)
184+
self.nullTerminatedStorage = try container.decode(
185+
Storage.self, forKey: .nullTerminatedStorage
186+
)
187+
guard _invariantsSatisfied() else {
188+
throw DecodingError.dataCorruptedError(
189+
forKey: .nullTerminatedStorage,
190+
in: container,
191+
debugDescription:
192+
"Encoding does not satisfy the invariants of SystemString"
193+
)
194+
}
195+
}
196+
}
169197

170198
extension SystemString {
171199

‎Tests/SystemTests/FilePathTests/FilePathDecodable.swift‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/*
2+
This source file is part of the Swift System open source project
3+
4+
Copyright (c)2024 Apple Inc. and the Swift System project authors
5+
Licensed under Apache License v2.0 with Runtime Library Exception
6+
7+
See https://swift.org/LICENSE.txt for license information
8+
*/
9+
10+
import XCTest
11+
12+
#if SYSTEM_PACKAGE
13+
@testable import SystemPackage
14+
#else
15+
@testable import System
16+
#endif
17+
18+
@available(/*System 0.0.1: macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0*/iOS 8, *)
19+
final class FilePathDecodableTest: XCTestCase {
20+
func testInvalidFilePath() {
21+
// _storage is a valid SystemString, but the invariants of FilePath are
22+
// violated (specifically, _storage is not normal).
23+
let input: [UInt8] = [
24+
123, 34, 95,115,116,111,114, 97,103,101, 34, 58,123, 34,110,117,108,108,
25+
84,101,114,109,105,110, 97,116,101,100, 83,116,111,114, 97,103,101, 34,
26+
58, 91, 49, 48, 57, 44, 45, 55, 54, 44, 53, 53, 44, 55, 49, 44, 49, 52,
27+
44, 53, 57, 44, 45, 49, 49, 50, 44, 45, 56, 52, 44, 52, 50, 44, 45, 55,
28+
48, 44, 45, 49, 48, 52, 44, 55, 51, 44, 45, 54, 44, 50, 44, 53, 55, 44,
29+
54, 50, 44, 45, 56, 55, 44, 45, 53, 44, 45, 54, 53, 44, 45, 51, 57, 44,
30+
45, 49, 48, 57, 44, 45, 55, 54, 44, 51, 48, 44, 53, 50, 44, 45, 56, 50,
31+
44, 45, 54, 48, 44, 45, 50, 44, 56, 53, 44, 49, 50, 51, 44, 45, 56, 52,
32+
44, 45, 53, 56, 44, 49, 49, 52, 44, 49, 44, 45, 49, 49, 54, 44, 56, 48,
33+
44, 49, 48, 52, 44, 45, 55, 56, 44, 45, 52, 53, 44, 49, 54, 44, 45, 52,
34+
54, 44, 55, 44, 49, 49, 56, 44, 45, 50, 52, 44, 54, 50, 44, 54, 52, 44,
35+
45, 52, 49, 44, 45, 49, 48, 51, 44, 53, 44, 45, 55, 53, 44, 50, 50, 44,
36+
45, 49, 48, 53, 44, 45, 49, 54, 44, 52, 55, 44, 52, 55, 44, 49, 50, 52,
37+
44, 45, 53, 55, 44, 53, 51, 44, 49, 49, 49, 44, 49, 53, 44, 45, 50, 55,
38+
44, 54, 54, 44, 45, 49, 54, 44, 49, 48, 50, 44, 49, 48, 54, 44, 49, 51,
39+
44, 49, 48, 53, 44, 45, 49, 49, 50, 44, 55, 56, 44, 45, 53, 48, 44, 50,
40+
48, 44, 56, 44, 45, 50, 55, 44, 52, 52, 44, 52, 44, 56, 44, 54, 53, 44,
41+
50, 51, 44, 57, 55, 44, 45, 50, 56, 44, 56, 56, 44, 52, 50, 44, 45, 51,
42+
54, 44, 45, 50, 51, 44, 49, 48, 51, 44, 57, 57, 44, 45, 53, 56, 44, 45,
43+
49, 49, 48, 44, 45, 53, 52, 44, 45, 49, 49, 55, 44, 45, 57, 52, 44, 45,
44+
55, 50, 44, 50, 57, 44, 45, 50, 52, 44, 45, 56, 52, 44, 53, 55, 44, 45,
45+
49, 50, 54, 44, 52, 52, 44, 55, 53, 44, 55, 54, 44, 52, 57, 44, 45, 52,
46+
49, 44, 45, 50, 53, 44, 50, 52, 44, 45, 49, 50, 54, 44, 55, 44, 50, 56,
47+
44, 45, 52, 56, 44, 56, 55, 44, 51, 49, 44, 45, 49, 49, 53, 44, 55, 44,
48+
45, 54, 48, 44, 53, 57, 44, 49, 51, 44, 55, 57, 44, 53, 48, 44, 45, 57,
49+
54, 44, 45, 50, 44, 45, 50, 52, 44, 45, 57, 49, 44, 55, 49, 44, 45, 49,
50+
50, 53, 44, 52, 50, 44, 45, 56, 52, 44, 52, 44, 53, 57, 44, 49, 50, 53,
51+
44, 49, 50, 49, 44, 45, 50, 54, 44, 45, 49, 50, 44, 45, 49, 48, 53, 44,
52+
53, 54, 44, 49, 49, 48, 44, 49, 52, 44, 45, 49, 48, 52, 44, 45, 53, 50,
53+
44, 45, 53, 56, 44, 45, 54, 44, 45, 50, 54, 44, 45, 52, 55, 44, 53, 57,
54+
44, 52, 50, 44, 49, 50, 51, 44, 52, 52, 44, 45, 57, 50, 44, 45, 50, 57,
55+
44, 45, 51, 54, 44, 45, 54, 50, 44, 50, 54, 44, 45, 49, 55, 44, 45, 49,
56+
48, 44, 45, 56, 49, 44, 54, 49, 44, 52, 55, 44, 45, 57, 52, 44, 45, 49,
57+
48, 54, 44, 49, 53, 44, 49, 48, 48, 44, 45, 49, 50, 49, 44, 45, 49, 49,
58+
49, 44, 51, 44, 45, 57, 44, 52, 54, 44, 45, 55, 48, 44, 45, 49, 57, 44,
59+
52, 56, 44, 45, 49, 50, 44, 45, 57, 49, 44, 45, 50, 48, 44, 49, 51, 44,
60+
54, 53, 44, 45, 55, 48, 44, 52, 49, 44, 45, 57, 53, 44, 49, 48, 52, 44,
61+
45, 55, 53, 44, 45, 49, 49, 53, 44, 49, 48, 49, 44, 45, 57, 52, 44, 45,
62+
49, 50, 51, 44, 45, 51, 53, 44, 45, 50, 49, 44, 45, 52, 50, 44, 45, 51,
63+
48, 44, 45, 55, 49, 44, 45, 49, 49, 57, 44, 52, 52, 44, 49, 49, 49, 44,
64+
49, 48, 53, 44, 54, 54, 44, 45, 49, 50, 54, 44, 55, 50, 44, 45, 52, 48,
65+
44, 49, 50, 49, 44, 45, 50, 49, 44, 52, 50, 44, 45, 55, 56, 44, 49, 50,
66+
54, 44, 56, 49, 44, 45, 57, 52, 44, 55, 52, 44, 49, 49, 50, 44, 45, 56,
67+
54, 44, 51, 50, 44, 55, 54, 44, 49, 49, 55, 44, 45, 56, 44, 56, 54, 44,
68+
49, 48, 51, 44, 54, 50, 44, 49, 49, 55, 44, 54, 55, 44, 45, 56, 54, 44,
69+
45, 49, 48, 48, 44, 45, 49, 48, 57, 44, 45, 53, 52, 44, 45, 51, 49, 44,
70+
45, 56, 57, 44, 48, 93,125,125,
71+
]
72+
73+
XCTAssertThrowsError(try JSONDecoder().decode(
74+
FilePath.self,
75+
from: Data(input)
76+
))
77+
}
78+
79+
func testInvalidSystemString() {
80+
// _storage is a SystemString whose invariants are violated; it contains
81+
// a non-terminating null byte.
82+
let input: [UInt8] = [
83+
123, 34, 95,115,116,111,114, 97,103,101, 34, 58,123, 34,110,117,108,108,
84+
84,101,114,109,105,110, 97,116,101,100, 83,116,111,114, 97,103,101, 34,
85+
58, 91, 49, 49, 49, 44, 48, 44, 45, 49, 54, 44, 57, 49, 44, 52, 54, 44,
86+
45, 49, 48, 50, 44, 49, 49, 53, 44, 45, 50, 49, 44, 45, 49, 49, 56, 44,
87+
52, 57, 44, 57, 50, 44, 45, 49, 48, 44, 53, 56, 44, 45, 55, 48, 44, 57,
88+
55, 44, 56, 44, 57, 57, 44, 48, 93,125, 125
89+
]
90+
91+
XCTAssertThrowsError(try JSONDecoder().decode(
92+
FilePath.self,
93+
from: Data(input)
94+
))
95+
}
96+
97+
func testInvalidExample() {
98+
// Another misformed example from Johannes that violates FilePath's
99+
// invariants by virtue of not being normalized.
100+
let input: [UInt8] = [
101+
123, 34, 95,115,116,111,114, 97,103,101, 34, 58,123, 34,110,117,108,108,
102+
84,101,114,109,105,110, 97,116,101,100, 83,116,111,114, 97,103,101, 34,
103+
58, 91, 56, 55, 44, 50, 52, 44, 45, 49, 49, 53, 44, 45, 49, 57, 44, 49,
104+
50, 50, 44, 45, 54, 56, 44, 57, 49, 44, 45, 49, 48, 54, 44, 45, 49, 48,
105+
48, 44, 45, 49, 49, 52, 44, 53, 54, 44, 45, 54, 53, 44, 49, 49, 56, 44,
106+
45, 54, 48, 44, 54, 54, 44, 45, 52, 50, 44, 55, 55, 44, 45, 54, 44, 45,
107+
52, 50, 44, 45, 56, 56, 44, 52, 55, 44, 48, 93,125, 125
108+
]
109+
110+
XCTAssertThrowsError(try JSONDecoder().decode(
111+
FilePath.self,
112+
from: Data(input)
113+
))
114+
}
115+
116+
func testEmptyString() {
117+
// FilePath with an empty (and hence not null-terminated) SystemString.
118+
let input: [UInt8] = [
119+
123, 34, 95,115,116,111,114, 97,103,101, 34, 58,123, 34,110,117,108,108,
120+
84,101,114,109,105,110, 97,116,101,100, 83,116,111,114, 97,103,101, 34,
121+
58, 91, 93,125,125
122+
]
123+
124+
XCTAssertThrowsError(try JSONDecoder().decode(
125+
FilePath.self,
126+
from: Data(input)
127+
))
128+
}
129+
}

0 commit comments

Comments
 (0)