From a1f77d0e1ab7c70871fb175091efc248b557a46b Mon Sep 17 00:00:00 2001 From: Stephen Canon Date: Wed, 17 Jul 2024 14:49:19 -0400 Subject: [PATCH] Cherry-pick: ensure invariants are enforced when decoding Ensure that FilePath and SystemString's invariants are enforced when decoding. Cherry-pick of merge-commit 9c42c54644f7bd78b8f2f3715060a01356e2a21d from PR #189 # Conflicts: # Sources/System/FilePath/FilePath.swift --- Sources/System/FilePath/FilePath.swift | 21 ++- Sources/System/FilePath/FilePathParsing.swift | 15 +- Sources/System/SystemString.swift | 34 ++++- .../FilePathTests/FilePathDecodable.swift | 129 ++++++++++++++++++ 4 files changed, 189 insertions(+), 10 deletions(-) create mode 100644 Tests/SystemTests/FilePathTests/FilePathDecodable.swift diff --git a/Sources/System/FilePath/FilePath.swift b/Sources/System/FilePath/FilePath.swift index 1aaa27ec..54eed38f 100644 --- a/Sources/System/FilePath/FilePath.swift +++ b/Sources/System/FilePath/FilePath.swift @@ -67,5 +67,22 @@ extension FilePath { } /*System 0.0.1, @available(macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0, *)*/ -extension FilePath: Hashable, Codable {} - +extension FilePath: Hashable, Codable { + // Encoder is synthesized; it probably should have been explicit and used + // a single-value container, but making that change now is somewhat risky. + + // Decoder is written explicitly to ensure that we validate invariants on + // untrusted input. + public init(from decoder: any Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + self._storage = try container.decode(SystemString.self, forKey: ._storage) + guard _invariantsSatisfied() else { + throw DecodingError.dataCorruptedError( + forKey: ._storage, + in: container, + debugDescription: + "Encoding does not satisfy the invariants of FilePath" + ) + } + } +} diff --git a/Sources/System/FilePath/FilePathParsing.swift b/Sources/System/FilePath/FilePathParsing.swift index 26753989..dff0409a 100644 --- a/Sources/System/FilePath/FilePathParsing.swift +++ b/Sources/System/FilePath/FilePathParsing.swift @@ -359,13 +359,18 @@ extension FilePath { // MARK: - Invariants extension FilePath { - internal func _invariantCheck() { - #if DEBUG + internal func _invariantsSatisfied() -> Bool { var normal = self normal._normalizeSeparators() - precondition(self == normal) - precondition(!self._storage._hasTrailingSeparator()) - precondition(_hasRoot == (self.root != nil)) + guard self == normal else { return false } + guard !self._storage._hasTrailingSeparator() else { return false } + guard _hasRoot == (self.root != nil) else { return false } + return true + } + + internal func _invariantCheck() { + #if DEBUG + precondition(_invariantsSatisfied()) #endif // DEBUG } } diff --git a/Sources/System/SystemString.swift b/Sources/System/SystemString.swift index c26d904d..54d12699 100644 --- a/Sources/System/SystemString.swift +++ b/Sources/System/SystemString.swift @@ -95,10 +95,18 @@ extension SystemString { } extension SystemString { + fileprivate func _invariantsSatisfied() -> Bool { + guard !nullTerminatedStorage.isEmpty else { return false } + guard nullTerminatedStorage.last! == .null else { return false } + guard nullTerminatedStorage.firstIndex(of: .null) == length else { + return false + } + return true + } + fileprivate func _invariantCheck() { #if DEBUG - precondition(nullTerminatedStorage.last! == .null) - precondition(nullTerminatedStorage.firstIndex(of: .null) == length) + precondition(_invariantsSatisfied()) #endif // DEBUG } } @@ -164,7 +172,27 @@ extension SystemString: RangeReplaceableCollection { } } -extension SystemString: Hashable, Codable {} +extension SystemString: Hashable, Codable { + // Encoder is synthesized; it probably should have been explicit and used + // a single-value container, but making that change now is somewhat risky. + + // Decoder is written explicitly to ensure that we validate invariants on + // untrusted input. + public init(from decoder: any Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + self.nullTerminatedStorage = try container.decode( + Storage.self, forKey: .nullTerminatedStorage + ) + guard _invariantsSatisfied() else { + throw DecodingError.dataCorruptedError( + forKey: .nullTerminatedStorage, + in: container, + debugDescription: + "Encoding does not satisfy the invariants of SystemString" + ) + } + } +} extension SystemString { diff --git a/Tests/SystemTests/FilePathTests/FilePathDecodable.swift b/Tests/SystemTests/FilePathTests/FilePathDecodable.swift new file mode 100644 index 00000000..290bef2f --- /dev/null +++ b/Tests/SystemTests/FilePathTests/FilePathDecodable.swift @@ -0,0 +1,129 @@ +/* + This source file is part of the Swift System open source project + + Copyright (c)2024 Apple Inc. and the Swift System project authors + Licensed under Apache License v2.0 with Runtime Library Exception + + See https://swift.org/LICENSE.txt for license information + */ + +import XCTest + +#if SYSTEM_PACKAGE +@testable import SystemPackage +#else +@testable import System +#endif + +@available(/*System 0.0.1: macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0*/iOS 8, *) +final class FilePathDecodableTest: XCTestCase { + func testInvalidFilePath() { + // _storage is a valid SystemString, but the invariants of FilePath are + // violated (specifically, _storage is not normal). + let input: [UInt8] = [ + 123, 34, 95,115,116,111,114, 97,103,101, 34, 58,123, 34,110,117,108,108, + 84,101,114,109,105,110, 97,116,101,100, 83,116,111,114, 97,103,101, 34, + 58, 91, 49, 48, 57, 44, 45, 55, 54, 44, 53, 53, 44, 55, 49, 44, 49, 52, + 44, 53, 57, 44, 45, 49, 49, 50, 44, 45, 56, 52, 44, 52, 50, 44, 45, 55, + 48, 44, 45, 49, 48, 52, 44, 55, 51, 44, 45, 54, 44, 50, 44, 53, 55, 44, + 54, 50, 44, 45, 56, 55, 44, 45, 53, 44, 45, 54, 53, 44, 45, 51, 57, 44, + 45, 49, 48, 57, 44, 45, 55, 54, 44, 51, 48, 44, 53, 50, 44, 45, 56, 50, + 44, 45, 54, 48, 44, 45, 50, 44, 56, 53, 44, 49, 50, 51, 44, 45, 56, 52, + 44, 45, 53, 56, 44, 49, 49, 52, 44, 49, 44, 45, 49, 49, 54, 44, 56, 48, + 44, 49, 48, 52, 44, 45, 55, 56, 44, 45, 52, 53, 44, 49, 54, 44, 45, 52, + 54, 44, 55, 44, 49, 49, 56, 44, 45, 50, 52, 44, 54, 50, 44, 54, 52, 44, + 45, 52, 49, 44, 45, 49, 48, 51, 44, 53, 44, 45, 55, 53, 44, 50, 50, 44, + 45, 49, 48, 53, 44, 45, 49, 54, 44, 52, 55, 44, 52, 55, 44, 49, 50, 52, + 44, 45, 53, 55, 44, 53, 51, 44, 49, 49, 49, 44, 49, 53, 44, 45, 50, 55, + 44, 54, 54, 44, 45, 49, 54, 44, 49, 48, 50, 44, 49, 48, 54, 44, 49, 51, + 44, 49, 48, 53, 44, 45, 49, 49, 50, 44, 55, 56, 44, 45, 53, 48, 44, 50, + 48, 44, 56, 44, 45, 50, 55, 44, 52, 52, 44, 52, 44, 56, 44, 54, 53, 44, + 50, 51, 44, 57, 55, 44, 45, 50, 56, 44, 56, 56, 44, 52, 50, 44, 45, 51, + 54, 44, 45, 50, 51, 44, 49, 48, 51, 44, 57, 57, 44, 45, 53, 56, 44, 45, + 49, 49, 48, 44, 45, 53, 52, 44, 45, 49, 49, 55, 44, 45, 57, 52, 44, 45, + 55, 50, 44, 50, 57, 44, 45, 50, 52, 44, 45, 56, 52, 44, 53, 55, 44, 45, + 49, 50, 54, 44, 52, 52, 44, 55, 53, 44, 55, 54, 44, 52, 57, 44, 45, 52, + 49, 44, 45, 50, 53, 44, 50, 52, 44, 45, 49, 50, 54, 44, 55, 44, 50, 56, + 44, 45, 52, 56, 44, 56, 55, 44, 51, 49, 44, 45, 49, 49, 53, 44, 55, 44, + 45, 54, 48, 44, 53, 57, 44, 49, 51, 44, 55, 57, 44, 53, 48, 44, 45, 57, + 54, 44, 45, 50, 44, 45, 50, 52, 44, 45, 57, 49, 44, 55, 49, 44, 45, 49, + 50, 53, 44, 52, 50, 44, 45, 56, 52, 44, 52, 44, 53, 57, 44, 49, 50, 53, + 44, 49, 50, 49, 44, 45, 50, 54, 44, 45, 49, 50, 44, 45, 49, 48, 53, 44, + 53, 54, 44, 49, 49, 48, 44, 49, 52, 44, 45, 49, 48, 52, 44, 45, 53, 50, + 44, 45, 53, 56, 44, 45, 54, 44, 45, 50, 54, 44, 45, 52, 55, 44, 53, 57, + 44, 52, 50, 44, 49, 50, 51, 44, 52, 52, 44, 45, 57, 50, 44, 45, 50, 57, + 44, 45, 51, 54, 44, 45, 54, 50, 44, 50, 54, 44, 45, 49, 55, 44, 45, 49, + 48, 44, 45, 56, 49, 44, 54, 49, 44, 52, 55, 44, 45, 57, 52, 44, 45, 49, + 48, 54, 44, 49, 53, 44, 49, 48, 48, 44, 45, 49, 50, 49, 44, 45, 49, 49, + 49, 44, 51, 44, 45, 57, 44, 52, 54, 44, 45, 55, 48, 44, 45, 49, 57, 44, + 52, 56, 44, 45, 49, 50, 44, 45, 57, 49, 44, 45, 50, 48, 44, 49, 51, 44, + 54, 53, 44, 45, 55, 48, 44, 52, 49, 44, 45, 57, 53, 44, 49, 48, 52, 44, + 45, 55, 53, 44, 45, 49, 49, 53, 44, 49, 48, 49, 44, 45, 57, 52, 44, 45, + 49, 50, 51, 44, 45, 51, 53, 44, 45, 50, 49, 44, 45, 52, 50, 44, 45, 51, + 48, 44, 45, 55, 49, 44, 45, 49, 49, 57, 44, 52, 52, 44, 49, 49, 49, 44, + 49, 48, 53, 44, 54, 54, 44, 45, 49, 50, 54, 44, 55, 50, 44, 45, 52, 48, + 44, 49, 50, 49, 44, 45, 50, 49, 44, 52, 50, 44, 45, 55, 56, 44, 49, 50, + 54, 44, 56, 49, 44, 45, 57, 52, 44, 55, 52, 44, 49, 49, 50, 44, 45, 56, + 54, 44, 51, 50, 44, 55, 54, 44, 49, 49, 55, 44, 45, 56, 44, 56, 54, 44, + 49, 48, 51, 44, 54, 50, 44, 49, 49, 55, 44, 54, 55, 44, 45, 56, 54, 44, + 45, 49, 48, 48, 44, 45, 49, 48, 57, 44, 45, 53, 52, 44, 45, 51, 49, 44, + 45, 56, 57, 44, 48, 93,125,125, + ] + + XCTAssertThrowsError(try JSONDecoder().decode( + FilePath.self, + from: Data(input) + )) + } + + func testInvalidSystemString() { + // _storage is a SystemString whose invariants are violated; it contains + // a non-terminating null byte. + let input: [UInt8] = [ + 123, 34, 95,115,116,111,114, 97,103,101, 34, 58,123, 34,110,117,108,108, + 84,101,114,109,105,110, 97,116,101,100, 83,116,111,114, 97,103,101, 34, + 58, 91, 49, 49, 49, 44, 48, 44, 45, 49, 54, 44, 57, 49, 44, 52, 54, 44, + 45, 49, 48, 50, 44, 49, 49, 53, 44, 45, 50, 49, 44, 45, 49, 49, 56, 44, + 52, 57, 44, 57, 50, 44, 45, 49, 48, 44, 53, 56, 44, 45, 55, 48, 44, 57, + 55, 44, 56, 44, 57, 57, 44, 48, 93,125, 125 + ] + + XCTAssertThrowsError(try JSONDecoder().decode( + FilePath.self, + from: Data(input) + )) + } + + func testInvalidExample() { + // Another misformed example from Johannes that violates FilePath's + // invariants by virtue of not being normalized. + let input: [UInt8] = [ + 123, 34, 95,115,116,111,114, 97,103,101, 34, 58,123, 34,110,117,108,108, + 84,101,114,109,105,110, 97,116,101,100, 83,116,111,114, 97,103,101, 34, + 58, 91, 56, 55, 44, 50, 52, 44, 45, 49, 49, 53, 44, 45, 49, 57, 44, 49, + 50, 50, 44, 45, 54, 56, 44, 57, 49, 44, 45, 49, 48, 54, 44, 45, 49, 48, + 48, 44, 45, 49, 49, 52, 44, 53, 54, 44, 45, 54, 53, 44, 49, 49, 56, 44, + 45, 54, 48, 44, 54, 54, 44, 45, 52, 50, 44, 55, 55, 44, 45, 54, 44, 45, + 52, 50, 44, 45, 56, 56, 44, 52, 55, 44, 48, 93,125, 125 + ] + + XCTAssertThrowsError(try JSONDecoder().decode( + FilePath.self, + from: Data(input) + )) + } + + func testEmptyString() { + // FilePath with an empty (and hence not null-terminated) SystemString. + let input: [UInt8] = [ + 123, 34, 95,115,116,111,114, 97,103,101, 34, 58,123, 34,110,117,108,108, + 84,101,114,109,105,110, 97,116,101,100, 83,116,111,114, 97,103,101, 34, + 58, 91, 93,125,125 + ] + + XCTAssertThrowsError(try JSONDecoder().decode( + FilePath.self, + from: Data(input) + )) + } +}