Cleanup lifetime management around NSData.bytes.

Lukasa · Lukasa · commit 9487a4975ac7 · 2021-03-29T09:32:37.000+01:00
Several blocks of code in Foundation use the .bytes member on NSData to
construct an interior pointer. However, NSData can potentially be freed
very early, and in newer Swift releases is more likely to do so, leading
to this pointer dangling.

This patch attempts a minimal change by forcing the NSData object to
live longer using withExtendedLifetime. This should resolve the misuse
fairly effectively. It does not address cases where the code could
potentially be entirely rewritten to be more efficient or more sensible,
because I wasn't here to engage in a complete refactor.
diff --git a/Sources/Foundation/Data.swift b/Sources/Foundation/Data.swift
@@ -2020,7 +2020,9 @@ public struct Data : ReferenceConvertible, Equatable, Hashable, RandomAccessColl
     @inlinable // This is @inlinable as a convenience initializer.
     public init(contentsOf url: __shared URL, options: Data.ReadingOptions = []) throws {
         let d = try NSData(contentsOf: url, options: ReadingOptions(rawValue: options.rawValue))
-        self.init(bytes: d.bytes, count: d.length)
+        self = withExtendedLifetime(d) {
+            return Data(bytes: d.bytes, count: d.length)
+        }
     }
     
     /// Initialize a `Data` from a Base-64 encoded String using the given options.
@@ -2031,7 +2033,9 @@ public struct Data : ReferenceConvertible, Equatable, Hashable, RandomAccessColl
     @inlinable // This is @inlinable as a convenience initializer.
     public init?(base64Encoded base64String: __shared String, options: Data.Base64DecodingOptions = []) {
         if let d = NSData(base64Encoded: base64String, options: Base64DecodingOptions(rawValue: options.rawValue)) {
-            self.init(bytes: d.bytes, count: d.length)
+            self = withExtendedLifetime(d) {
+                Data(bytes: d.bytes, count: d.length)
+            }
         } else {
             return nil
         }
@@ -2046,7 +2050,9 @@ public struct Data : ReferenceConvertible, Equatable, Hashable, RandomAccessColl
     @inlinable // This is @inlinable as a convenience initializer.
     public init?(base64Encoded base64Data: __shared Data, options: Data.Base64DecodingOptions = []) {
         if let d = NSData(base64Encoded: base64Data, options: Base64DecodingOptions(rawValue: options.rawValue)) {
-            self.init(bytes: d.bytes, count: d.length)
+            self = withExtendedLifetime(d) {
+                Data(bytes: d.bytes, count: d.length)
+            }
         } else {
             return nil
         }
diff --git a/Sources/Foundation/NSAttributedString.swift b/Sources/Foundation/NSAttributedString.swift
@@ -558,27 +558,29 @@ internal func _NSReadIntFromMutableAttributedStringCoding(_ data: NSData, _ star
     let length = data.length
     
     var value = 0
-    
-    while offset < length {
-        let i = Int(data.bytes.load(fromByteOffset: offset, as: UInt8.self))
-        
-        offset += 1
-        
-        let isLast = i < 128
-        
-        let intermediateValue = multiplier.multipliedReportingOverflow(by: isLast ? i : (i - 128))
-        guard !intermediateValue.overflow else { return nil }
-        
-        let newValue = value.addingReportingOverflow(intermediateValue.partialValue)
-        guard !newValue.overflow else { return nil }
-        
-        value = newValue.partialValue
 
-        if isLast {
-            return (value: value, newOffset: offset)
+    withExtendedLifetime(data) {
+        while offset < length {
+            let i = Int(data.bytes.load(fromByteOffset: offset, as: UInt8.self))
+
+            offset += 1
+
+            let isLast = i < 128
+
+            let intermediateValue = multiplier.multipliedReportingOverflow(by: isLast ? i : (i - 128))
+            guard !intermediateValue.overflow else { return nil }
+
+            let newValue = value.addingReportingOverflow(intermediateValue.partialValue)
+            guard !newValue.overflow else { return nil }
+
+            value = newValue.partialValue
+
+            if isLast {
+                return (value: value, newOffset: offset)
+            }
+
+            multiplier *= 128
         }
-        
-        multiplier *= 128
     }
     
     return nil // Getting to the end of the stream indicates error, since we were still expecting more bytes
diff --git a/Sources/Foundation/NSData.swift b/Sources/Foundation/NSData.swift
@@ -153,15 +153,20 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding {
     public init(contentsOfFile path: String, options readOptionsMask: ReadingOptions = []) throws {
         super.init()
         let readResult = try NSData.readBytesFromFileWithExtendedAttributes(path, options: readOptionsMask)
-        _init(bytes: readResult.bytes, length: readResult.length, copy: false, deallocator: readResult.deallocator)
+
+        withExtendedLifetime(readResult) {
+            _init(bytes: readResult.bytes, length: readResult.length, copy: false, deallocator: readResult.deallocator)
+        }
     }
 
     /// Initializes a data object with the contents of the file at a given path.
     public init?(contentsOfFile path: String) {
         do {
             super.init()
             let readResult = try NSData.readBytesFromFileWithExtendedAttributes(path, options: [])
-            _init(bytes: readResult.bytes, length: readResult.length, copy: false, deallocator: readResult.deallocator)
+            withExtendedLifetime(readResult) {
+                _init(bytes: readResult.bytes, length: readResult.length, copy: false, deallocator: readResult.deallocator)
+            }
         } catch {
             return nil
         }
@@ -179,15 +184,19 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding {
     public init(contentsOf url: URL, options readOptionsMask: ReadingOptions = []) throws {
         super.init()
         let (data, _) = try NSData.contentsOf(url: url, options: readOptionsMask)
-        _init(bytes: UnsafeMutableRawPointer(mutating: data.bytes), length: data.length, copy: true)
+        withExtendedLifetime(data) {
+            _init(bytes: UnsafeMutableRawPointer(mutating: data.bytes), length: data.length, copy: true)
+        }
     }
 
     /// Initializes a data object with the data from the location specified by a given URL.
     public init?(contentsOf url: URL) {
         super.init()
         do {
             let (data, _) = try NSData.contentsOf(url: url)
-            _init(bytes: UnsafeMutableRawPointer(mutating: data.bytes), length: data.length, copy: true)
+            withExtendedLifetime(data) {
+                _init(bytes: UnsafeMutableRawPointer(mutating: data.bytes), length: data.length, copy: true)
+            }
         } catch {
             return nil
         }
@@ -375,15 +384,19 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding {
             guard let data = aDecoder._decodePropertyListForKey("NS.data") as? NSData else {
                 return nil
             }
-            _init(bytes: UnsafeMutableRawPointer(mutating: data.bytes), length: data.length, copy: true)
+            withExtendedLifetime(data) {
+                _init(bytes: UnsafeMutableRawPointer(mutating: data.bytes), length: data.length, copy: true)
+            }
         } else {
             let result : Data? = aDecoder.withDecodedUnsafeBufferPointer(forKey: "NS.bytes") {
                 guard let buffer = $0 else { return nil }
                 return Data(buffer: buffer)
             }
             
-            guard let r = result else { return nil }
-            _init(bytes: UnsafeMutableRawPointer(mutating: r._nsObject.bytes), length: r.count, copy: true)
+            guard var r = result else { return nil }
+            result.withUnsafeMutableBytes {
+                _init(bytes: $0.baseAddress, length: $0.count, copy: true)
+            }
         }
     }
     
@@ -559,24 +572,27 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding {
     /// Finds and returns the range of the first occurrence of the given data, within the given range, subject to given options.
     open func range(of dataToFind: Data, options mask: SearchOptions = [], in searchRange: NSRange) -> NSRange {
         let dataToFind = dataToFind._nsObject
-        guard dataToFind.length > 0 else {return NSRange(location: NSNotFound, length: 0)}
-        guard let searchRange = Range(searchRange) else {fatalError("invalid range")}
-        
-        precondition(searchRange.upperBound <= self.length, "range outside the bounds of data")
 
-        let basePtr = self.bytes.bindMemory(to: UInt8.self, capacity: self.length)
-        let baseData = UnsafeBufferPointer<UInt8>(start: basePtr, count: self.length)[searchRange]
-        let searchPtr = dataToFind.bytes.bindMemory(to: UInt8.self, capacity: dataToFind.length)
-        let search = UnsafeBufferPointer<UInt8>(start: searchPtr, count: dataToFind.length)
-        
-        let location : Int?
-        let anchored = mask.contains(.anchored)
-        if mask.contains(.backwards) {
-            location = NSData.searchSubSequence(search.reversed(), inSequence: baseData.reversed(),anchored : anchored).map {$0.base-search.count}
-        } else {
-            location = NSData.searchSubSequence(search, inSequence: baseData,anchored : anchored)
+        return withExtendedLifetime(dataToFind) {
+            guard dataToFind.length > 0 else {return NSRange(location: NSNotFound, length: 0)}
+            guard let searchRange = Range(searchRange) else {fatalError("invalid range")}
+
+            precondition(searchRange.upperBound <= self.length, "range outside the bounds of data")
+
+            let basePtr = self.bytes.bindMemory(to: UInt8.self, capacity: self.length)
+            let baseData = UnsafeBufferPointer<UInt8>(start: basePtr, count: self.length)[searchRange]
+            let searchPtr = dataToFind.bytes.bindMemory(to: UInt8.self, capacity: dataToFind.length)
+            let search = UnsafeBufferPointer<UInt8>(start: searchPtr, count: dataToFind.length)
+
+            let location : Int?
+            let anchored = mask.contains(.anchored)
+            if mask.contains(.backwards) {
+                location = NSData.searchSubSequence(search.reversed(), inSequence: baseData.reversed(),anchored : anchored).map {$0.base-search.count}
+            } else {
+                location = NSData.searchSubSequence(search, inSequence: baseData,anchored : anchored)
+            }
+            return location.map {NSRange(location: $0, length: search.count)} ?? NSRange(location: NSNotFound, length: 0)
         }
-        return location.map {NSRange(location: $0, length: search.count)} ?? NSRange(location: NSNotFound, length: 0)
     }
 
     private static func searchSubSequence<T : Collection, T2 : Sequence>(_ subSequence : T2, inSequence seq: T,anchored : Bool) -> T.Index? where T.Iterator.Element : Equatable, T.Iterator.Element == T2.Iterator.Element {
diff --git a/Sources/Foundation/NSString.swift b/Sources/Foundation/NSString.swift
@@ -1368,12 +1368,16 @@ extension NSString {
     public convenience init(contentsOf url: URL, encoding enc: UInt) throws {
         let readResult = try NSData(contentsOf: url, options: [])
 
-        let bytePtr = readResult.bytes.bindMemory(to: UInt8.self, capacity: readResult.length)
-        guard let cf = CFStringCreateWithBytes(kCFAllocatorDefault, bytePtr, readResult.length, CFStringConvertNSStringEncodingToEncoding(numericCast(enc)), true) else {
-            throw NSError(domain: NSCocoaErrorDomain, code: CocoaError.fileReadInapplicableStringEncoding.rawValue, userInfo: [
-                NSDebugDescriptionErrorKey : "Unable to create a string using the specified encoding."
-                ])
+        let cf = try withExtendedLifetime(readResult) {
+            let bytePtr = readResult.bytes.bindMemory(to: UInt8.self, capacity: readResult.length)
+            guard let cf = CFStringCreateWithBytes(kCFAllocatorDefault, bytePtr, readResult.length, CFStringConvertNSStringEncodingToEncoding(numericCast(enc)), true) else {
+                throw NSError(domain: NSCocoaErrorDomain, code: CocoaError.fileReadInapplicableStringEncoding.rawValue, userInfo: [
+                    NSDebugDescriptionErrorKey : "Unable to create a string using the specified encoding."
+                    ])
+            }
+            return cf
         }
+
         var str: String?
         if String._conditionallyBridgeFromObjectiveC(cf._nsObject, result: &str) {
             self.init(str!)
@@ -1393,42 +1397,47 @@ extension NSString {
 
         let encoding: UInt
         let offset: Int
-        // Look for a BOM (Byte Order Marker) to try and determine the text Encoding, this also skips
-        // over the bytes. This takes precedence over the textEncoding in the http header
-        let bytePtr = readResult.bytes.bindMemory(to: UInt8.self, capacity:readResult.length)
-        if readResult.length >= 4 && bytePtr[0] == 0xFF && bytePtr[1] == 0xFE && bytePtr[2] == 0x00 && bytePtr[3] == 0x00 {
-            encoding = String.Encoding.utf32LittleEndian.rawValue
-            offset = 4
-        }
-        else if readResult.length >= 2 && bytePtr[0] == 0xFE && bytePtr[1] == 0xFF {
-            encoding = String.Encoding.utf16BigEndian.rawValue
-            offset = 2
-        }
-        else if readResult.length >= 2 && bytePtr[0] == 0xFF && bytePtr[1] == 0xFE {
-            encoding = String.Encoding.utf16LittleEndian.rawValue
-            offset = 2
-        }
-        else if readResult.length >= 4 && bytePtr[0] == 0x00 && bytePtr[1] == 0x00 && bytePtr[2] == 0xFE && bytePtr[3] == 0xFF {
-            encoding = String.Encoding.utf32BigEndian.rawValue
-            offset = 4
-        }
-        else if let charSet = textEncodingNameMaybe, let textEncoding = String.Encoding(charSet: charSet) {
-            encoding = textEncoding.rawValue
-            offset = 0
-        } else {
-            //Need to work on more conditions. This should be the default
-            encoding = String.Encoding.utf8.rawValue
-            offset = 0
-        }
 
-        // Since the encoding being passed includes the byte order the BOM wont be checked or skipped, so pass offset to
-        // manually skip the BOM header.
-        guard let cf = CFStringCreateWithBytes(kCFAllocatorDefault, bytePtr + offset, readResult.length - offset,
-                                               CFStringConvertNSStringEncodingToEncoding(numericCast(encoding)), true) else {
-            throw NSError(domain: NSCocoaErrorDomain, code: CocoaError.fileReadInapplicableStringEncoding.rawValue, userInfo: [
-                NSDebugDescriptionErrorKey : "Unable to create a string using the specified encoding."
-                ])
+        let cf = try withExtendedLifetime(readResult) {
+            // Look for a BOM (Byte Order Marker) to try and determine the text Encoding, this also skips
+            // over the bytes. This takes precedence over the textEncoding in the http header
+            let bytePtr = readResult.bytes.bindMemory(to: UInt8.self, capacity:readResult.length)
+            if readResult.length >= 4 && bytePtr[0] == 0xFF && bytePtr[1] == 0xFE && bytePtr[2] == 0x00 && bytePtr[3] == 0x00 {
+                encoding = String.Encoding.utf32LittleEndian.rawValue
+                offset = 4
+            }
+            else if readResult.length >= 2 && bytePtr[0] == 0xFE && bytePtr[1] == 0xFF {
+                encoding = String.Encoding.utf16BigEndian.rawValue
+                offset = 2
+            }
+            else if readResult.length >= 2 && bytePtr[0] == 0xFF && bytePtr[1] == 0xFE {
+                encoding = String.Encoding.utf16LittleEndian.rawValue
+                offset = 2
+            }
+            else if readResult.length >= 4 && bytePtr[0] == 0x00 && bytePtr[1] == 0x00 && bytePtr[2] == 0xFE && bytePtr[3] == 0xFF {
+                encoding = String.Encoding.utf32BigEndian.rawValue
+                offset = 4
+            }
+            else if let charSet = textEncodingNameMaybe, let textEncoding = String.Encoding(charSet: charSet) {
+                encoding = textEncoding.rawValue
+                offset = 0
+            } else {
+                //Need to work on more conditions. This should be the default
+                encoding = String.Encoding.utf8.rawValue
+                offset = 0
+            }
+
+            // Since the encoding being passed includes the byte order the BOM wont be checked or skipped, so pass offset to
+            // manually skip the BOM header.
+            guard let cf = CFStringCreateWithBytes(kCFAllocatorDefault, bytePtr + offset, readResult.length - offset,
+                                                   CFStringConvertNSStringEncodingToEncoding(numericCast(encoding)), true) else {
+                throw NSError(domain: NSCocoaErrorDomain, code: CocoaError.fileReadInapplicableStringEncoding.rawValue, userInfo: [
+                    NSDebugDescriptionErrorKey : "Unable to create a string using the specified encoding."
+                    ])
+            }
+            return cf
         }
+
         var str: String?
         if String._conditionallyBridgeFromObjectiveC(cf._nsObject, result: &str) {
             self.init(str!)
diff --git a/Sources/FoundationNetworking/NSURLRequest.swift b/Sources/FoundationNetworking/NSURLRequest.swift
@@ -237,8 +237,10 @@ open class NSURLRequest : NSObject, NSSecureCoding, NSCopying, NSMutableCopying
         aCoder.encode(self.cachePolicy.rawValue._bridgeToObjectiveC(), forKey: "NS._cachePolicy")
         aCoder.encode(self.timeoutInterval._bridgeToObjectiveC(), forKey: "NS._timeoutInterval")
         if let httpBody = self.httpBody?._bridgeToObjectiveC() {
-            let bytePtr = httpBody.bytes.bindMemory(to: UInt8.self, capacity: httpBody.length)
-            aCoder.encodeBytes(bytePtr, length: httpBody.length, forKey: "NS.httpBody")
+            withExtendedLifetime(httpBody) {
+                let bytePtr = httpBody.bytes.bindMemory(to: UInt8.self, capacity: httpBody.length)
+                aCoder.encodeBytes(bytePtr, length: httpBody.length, forKey: "NS.httpBody")
+            }
         }
         //On macOS input stream is not encoded.
         aCoder.encode(self.networkServiceType.rawValue._bridgeToObjectiveC(), forKey: "NS._networkServiceType")
diff --git a/Sources/FoundationNetworking/URLSession/NativeProtocol.swift b/Sources/FoundationNetworking/URLSession/NativeProtocol.swift
@@ -256,7 +256,9 @@ internal class _NativeProtocol: URLProtocol, _EasyHandleDelegate {
         if case .inMemory(let bodyData) = bodyDataDrain {
             var data = Data()
             if let body = bodyData {
-                data = Data(bytes: body.bytes, count: body.length)
+                withExtendedLifetime(body) {
+                    data = Data(bytes: body.bytes, count: body.length)
+                }
             }
             self.client?.urlProtocol(self, didLoad: data)
             self.internalState = .taskCompleted
diff --git a/Sources/FoundationNetworking/URLSession/NetworkingSpecific.swift b/Sources/FoundationNetworking/URLSession/NetworkingSpecific.swift
@@ -85,7 +85,7 @@ class _NSNonfileURLContentLoader: _NSNonfileURLContentLoading {
             switch statusCode {
                 // These are the only valid response codes that data will be returned for, all other codes will be treated as error.
                 case 101, 200...399, 401, 407:
-                    return (NSData(bytes: UnsafeMutableRawPointer(mutating: (data as NSData).bytes), length: data.count), urlResponse?.textEncodingName)
+                    return (data as NSData, urlResponse?.textEncodingName)
 
                 default:
                     break
diff --git a/Tests/Foundation/Tests/TestNSData.swift b/Tests/Foundation/Tests/TestNSData.swift
