Skip to content

Commit d2ba781

Browse files
glessardglessard
authored
Merge pull request #190 from glessard/enforce-file-path-invariants-in-decode-1.3.x
Cherry-pick: ensure invariants are enforced when decoding
2 parents 6a9e38e + a1f77d0 commit d2ba781

File tree

4 files changed

+189
-10
lines changed

4 files changed

+189
-10
lines changed

‎Sources/System/FilePath/FilePath.swift‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,5 +67,22 @@ extension FilePath {
6767
}
6868

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

‎Sources/System/FilePath/FilePathParsing.swift‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -359,13 +359,18 @@ extension FilePath {
359359

360360
// MARK: - Invariants
361361
extension FilePath {
362-
internal func _invariantCheck() {
363-
#if DEBUG
362+
internal func _invariantsSatisfied() -> Bool {
364363
var normal = self
365364
normal._normalizeSeparators()
366-
precondition(self == normal)
367-
precondition(!self._storage._hasTrailingSeparator())
368-
precondition(_hasRoot == (self.root != nil))
365+
guard self == normal else { return false }
366+
guard !self._storage._hasTrailingSeparator() else { return false }
367+
guard _hasRoot == (self.root != nil) else { return false }
368+
return true
369+
}
370+
371+
internal func _invariantCheck() {
372+
#if DEBUG
373+
precondition(_invariantsSatisfied())
369374
#endif // DEBUG
370375
}
371376
}

‎Sources/System/SystemString.swift‎

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

9797
extension SystemString {
98+
fileprivate func _invariantsSatisfied() -> Bool {
99+
guard !nullTerminatedStorage.isEmpty else { return false }
100+
guard nullTerminatedStorage.last! == .null else { return false }
101+
guard nullTerminatedStorage.firstIndex(of: .null) == length else {
102+
return false
103+
}
104+
return true
105+
}
106+
98107
fileprivate func _invariantCheck() {
99108
#if DEBUG
100-
precondition(nullTerminatedStorage.last! == .null)
101-
precondition(nullTerminatedStorage.firstIndex(of: .null) == length)
109+
precondition(_invariantsSatisfied())
102110
#endif // DEBUG
103111
}
104112
}
@@ -164,7 +172,27 @@ extension SystemString: RangeReplaceableCollection {
164172
}
165173
}
166174

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

169197
extension SystemString {
170198

‎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)