Mitigate out-of-bounds memory read (#4597)

glessard · web-flow · commit e1536b2a3f78 · 2022-07-07T10:10:08.000-07:00
Mitigate an oob memory read issue due to pointer conversion, by
- adding overloads for `String.init?(cString:encoding:)` and `String.init?(utf8String:)`
- enforcing null-termination precondition when possible
- adding advisory deprecations for the obviously fixable cases
- clarifying the null-termination precondition in doc-comments

addresses rdar://93255079
diff --git a/Sources/Foundation/NSStringAPI.swift b/Sources/Foundation/NSStringAPI.swift
@@ -2,7 +2,7 @@
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -20,7 +20,7 @@
 // This file is shared between two projects:
 //
 // 1. https://github.com/apple/swift/tree/master/stdlib/public/Darwin/Foundation
-// 2. https://github.com/apple/swift-corelibs-foundation/tree/master/Foundation
+// 2. https://github.com/apple/swift-corelibs-foundation/tree/main/Foundation
 //
 // If you change this file, you must update it in both places.
 
@@ -176,7 +176,7 @@ extension String {
     // + (instancetype)stringWithUTF8String:(const char *)bytes
 
     /// Creates a string by copying the data from a given
-    /// C array of UTF8-encoded bytes.
+    /// null-terminated C array of UTF8-encoded bytes.
     public init?(utf8String bytes: UnsafePointer<CChar>) {
         if let str = String(validatingUTF8: bytes) {
             self = str
@@ -188,6 +188,54 @@ extension String {
             return nil
         }
     }
+
+    /// Creates a string by copying the data from a given
+    /// null-terminated array of UTF8-encoded bytes.
+    @_alwaysEmitIntoClient
+    public init?(utf8String bytes: [CChar]) {
+        // the stdlib's validatingUTF8 [CChar] overload checks for null termination.
+        if let str = String(validatingUTF8: bytes) {
+            self = str
+            return
+        }
+        guard let nullPosition = bytes.firstIndex(of: 0) else {
+            fatalError(
+                "input of String.init(utf8String:) must be null-terminated"
+            )
+        }
+        let ns = bytes.withUnsafeBytes {
+            NSString(bytes: $0.baseAddress!,
+                     length: nullPosition,
+                     encoding: Encoding.utf8.rawValue)
+        }
+        guard let ns = ns else {
+            return nil
+        }
+        self = String._unconditionallyBridgeFromObjectiveC(ns)
+    }
+
+    @_alwaysEmitIntoClient
+    @available(*, deprecated, message: "Use a copy of the String argument")
+    public init?(utf8String bytes: String) {
+        var decoded = bytes
+        decoded.makeContiguousUTF8()
+        if let null = decoded.firstIndex(of: "\0") {
+            decoded = String(decoded[..<null])
+        }
+        self = decoded
+    }
+
+    @_alwaysEmitIntoClient
+    @available(*, deprecated, message: "Use String(_ scalar: Unicode.Scalar)")
+    public init?(utf8String bytes: inout CChar) {
+        // a byte interpreted as a buffer is valid only if the value is zero.
+        guard bytes == 0 else {
+            fatalError(
+                "input of String.init(utf8String:) must be null-terminated"
+            )
+        }
+        self = ""
+    }
 }
 
 extension String {
@@ -369,15 +417,16 @@ extension String {
     //     initWithCString:(const char *)nullTerminatedCString
     //     encoding:(NSStringEncoding)encoding
 
-    /// Produces a string containing the bytes in a given C array,
-    /// interpreted according to a given encoding.
-    public init?(
-        cString: UnsafePointer<CChar>,
-        encoding enc: Encoding
-        ) {
-        if enc == .utf8, let str = String(validatingUTF8: cString) {
-            self = str
-            return
+    /// Produces a string by copying the null-terminated bytes
+    /// in a given C array, interpreted according to a given encoding.
+    public init?(cString: UnsafePointer<CChar>, encoding enc: Encoding) {
+        if enc == .utf8 || enc == .ascii {
+            if let str = String(validatingUTF8: cString) {
+                if enc == .utf8 || str._guts._isContiguousASCII {
+                    self = str
+                    return
+                }
+            }
         }
         if let ns = NSString(cString: cString, encoding: enc.rawValue) {
             self = String._unconditionallyBridgeFromObjectiveC(ns)
@@ -386,6 +435,64 @@ extension String {
         }
     }
 
+    /// Produces a string by copying the null-terminated bytes
+    /// in a given array, interpreted according to a given encoding.
+    @_alwaysEmitIntoClient
+    public init?(cString: [CChar], encoding enc: Encoding) {
+        if enc == .utf8 || enc == .ascii {
+            // the stdlib's validatingUTF8 [CChar] overload checks for null termination.
+            if let str = String(validatingUTF8: cString) {
+                if enc == .utf8 || str._guts._isContiguousASCII {
+                    self = str
+                    return
+                }
+            }
+        }
+        guard let nullPosition = cString.firstIndex(of: 0) else {
+            fatalError(
+                "input of String.init(cString:encoding:) must be null-terminated"
+            )
+        }
+        let ns = cString.withUnsafeBytes {
+            NSString(bytes: $0.baseAddress!,
+                     length: nullPosition,
+                     encoding: enc.rawValue)
+        }
+        guard let ns = ns else {
+            return nil
+        }
+        self = String._unconditionallyBridgeFromObjectiveC(ns)
+    }
+
+    @_alwaysEmitIntoClient
+    @available(*, deprecated, message: "Use a copy of the String argument")
+    public init?(cString: String, encoding enc: Encoding) {
+        if enc == .utf8 || enc == .ascii {
+            var decoded = cString
+            decoded.makeContiguousUTF8()
+            if let null = decoded.firstIndex(of: "\0") {
+                decoded = String(decoded[..<null])
+            }
+            if enc == .utf8 || decoded.utf8.allSatisfy({ $0 < 128 }) {
+                self = decoded
+                return
+            }
+        }
+        return nil
+    }
+
+    @_alwaysEmitIntoClient
+    @available(*, deprecated, message: "Use String(_ scalar: Unicode.Scalar)")
+    public init?(cString: inout CChar, encoding enc: Encoding) {
+        // a byte interpreted as a buffer is valid only if the value is zero.
+        guard cString == 0 else {
+            fatalError(
+                "input of String.init(cString:encoding:) must be null-terminated"
+            )
+        }
+        self = ""
+    }
+
     // FIXME: handle optional locale with default arguments
 
     // - (instancetype)
@@ -463,7 +570,7 @@ extension String {
     }
 }
 
-extension StringProtocol where Index == String.Index {
+extension StringProtocol {
     //===--- Bridging Helpers -----------------------------------------------===//
     //===--------------------------------------------------------------------===//
 
diff --git a/Tests/Foundation/Tests/TestNSString.swift b/Tests/Foundation/Tests/TestNSString.swift
@@ -1,6 +1,6 @@
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2016 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See http://swift.org/LICENSE.txt for license information
@@ -1644,6 +1644,112 @@ class TestNSString: LoopbackServerTest {
         XCTAssertEqual(String(ns), "Test")
     }
 
+    func test_initString_utf8StringWithArrayInput() {
+        var source: [CChar] = [0x61, 0x62, 0, 0x63]
+        var str: String?
+        str = String(utf8String: source)
+        XCTAssertNotNil(str)
+        source.withUnsafeBufferPointer {
+            XCTAssertEqual(str, String(utf8String: $0.baseAddress!))
+        }
+        // substitute a value not valid in UTF-8
+        source[1] = CChar(bitPattern: 0xff)
+        str = String(utf8String: source)
+        XCTAssertNil(str)
+    }
+
+    @available(*, deprecated) // silence the deprecation warning within
+    func test_initString_utf8StringWithStringInput() {
+        let source = "ab\0c"
+        var str: String?
+        str = String(utf8String: source)
+        XCTAssertNotNil(str)
+        source.withCString {
+            XCTAssertEqual(str, String(utf8String: $0))
+        }
+        str = String(utf8String: "")
+        XCTAssertNotNil(str)
+        XCTAssertEqual(str?.isEmpty, true)
+    }
+
+    @available(*, deprecated) // silence the deprecation warning within
+    func test_initString_utf8StringWithInoutConversion() {
+        var c = CChar.zero
+        var str: String?
+        str = String(utf8String: &c)
+        // Any other value of `c` would violate the null-terminated precondition
+        XCTAssertNotNil(str)
+        XCTAssertEqual(str?.isEmpty, true)
+    }
+
+    func test_initString_cStringWithArrayInput() {
+        var source: [CChar] = [0x61, 0x62, 0, 0x63]
+        var str: String?
+        str = String(cString: source, encoding: .utf8)
+        XCTAssertNotNil(str)
+        source.withUnsafeBufferPointer {
+            XCTAssertEqual(
+                str, String(cString: $0.baseAddress!, encoding: .utf8)
+            )
+        }
+        str = String(cString: source, encoding: .ascii)
+        XCTAssertNotNil(str)
+        source.withUnsafeBufferPointer {
+            XCTAssertEqual(
+                str, String(cString: $0.baseAddress!, encoding: .ascii)
+            )
+        }
+        str = String(cString: source, encoding: .macOSRoman)
+        XCTAssertNotNil(str)
+        source.withUnsafeBufferPointer {
+            XCTAssertEqual(
+                str, String(cString: $0.baseAddress!, encoding: .macOSRoman)
+            )
+        }
+        // substitute a value not valid in UTF-8
+        source[1] = CChar(bitPattern: 0xff)
+        str = String(cString: source, encoding: .utf8)
+        XCTAssertNil(str)
+        str = String(cString: source, encoding: .macOSRoman)
+        XCTAssertNotNil(str)
+        source.withUnsafeBufferPointer {
+            XCTAssertEqual(
+                str, String(cString: $0.baseAddress!, encoding: .macOSRoman)
+            )
+        }
+    }
+
+    @available(*, deprecated) // silence the deprecation warning within
+    func test_initString_cStringWithStringInput() {
+        let source = "ab\0c"
+        var str: String?
+        str = String(cString: source, encoding: .utf8)
+        XCTAssertNotNil(str)
+        source.withCString {
+            XCTAssertEqual(str, String(cString: $0, encoding: .utf8))
+        }
+        str = String(cString: source, encoding: .ascii)
+        XCTAssertNotNil(str)
+        source.withCString {
+            XCTAssertEqual(str, String(cString: $0, encoding: .ascii))
+        }
+        str = String(cString: "", encoding: .utf8)
+        XCTAssertNotNil(str)
+        XCTAssertEqual(str?.isEmpty, true)
+        str = String(cString: "Caractères", encoding: .ascii)
+        XCTAssertNil(str)
+    }
+
+    @available(*, deprecated) // silence the deprecation warning within
+    func test_initString_cStringWithInoutConversion() {
+        var c = CChar.zero
+        var str: String?
+        str = String(cString: &c, encoding: .ascii)
+        // Any other value of `c` would violate the null-terminated precondition
+        XCTAssertNotNil(str)
+        XCTAssertEqual(str?.isEmpty, true)
+    }
+
     static var allTests: [(String, (TestNSString) -> () throws -> Void)] {
         var tests = [
             ("test_initData", test_initData),
@@ -1718,6 +1824,12 @@ class TestNSString: LoopbackServerTest {
             ("test_enumerateSubstrings", test_enumerateSubstrings),
             ("test_paragraphRange", test_paragraphRange),
             ("test_initStringWithNSString", test_initStringWithNSString),
+            ("test_initString_utf8StringWithArrayInput", test_initString_utf8StringWithArrayInput),
+            ("test_initString_utf8StringWithStringInput", test_initString_utf8StringWithStringInput),
+            ("test_initString_utf8StringWithInoutConversion", test_initString_utf8StringWithInoutConversion),
+            ("test_initString_cStringWithArrayInput", test_initString_cStringWithArrayInput),
+            ("test_initString_cStringWithStringInput", test_initString_cStringWithStringInput),
+            ("test_initString_cStringWithInoutConversion", test_initString_cStringWithInoutConversion),
         ]
 
 #if !os(Windows)
