CVE-2023-31136: Insufficiently Protected Credentials (CWE-522)
value: PostgresNIO is a Swift client for PostgreSQL. Any user of PostgresNIO prior to version 1.14.2 connecting to servers with TLS enabled is vulnerable to a man-in-the-middle attacker injecting false responses to the client's first few queries, despite the use of TLS certificate verification and encryption. The vulnerability is addressed in PostgresNIO versions starting from 1.14.2. There are no known workarounds for unpatched users.
CWE description: The product transmits or stores authentication credentials, but it uses an insecure method that is susceptible to unauthorized interception and/or retrieval.
CVSS v3 base score: 5.9
Published: 2023-05-09T14:15Z
Repository: https://github.com/vapor/postgres-nio
Language: Swift
Files changed: Package.swift, Sources/PostgresNIO/New/Connection State Machine/ConnectionStateMachine.swift	, Sources/PostgresNIO/New/PSQLError.swift, Sources/PostgresNIO/New/PostgresChannelHandler.swift, Sources/PostgresNIO/Postgres+PSQLCompat.swift, Tests/PostgresNIOTests/New/Connection State Machine/ConnectionStateMachineTests.swift	, Tests/PostgresNIOTests/New/PostgresChannelHandlerTests.swift
Commit date: 2023-05-04 12:19:06 -0500
Commit message: Merge pull request from GHSA-9cfh-vx93-84vv

* Ensure empty incoming buffer when TLS negotiation starts

* cleanup

* Use real nio version
Diff: diff --git a/Package.swift b/Package.swift
index 52857cd..c1cb4bd 100644
--- a/Package.swift
+++ b/Package.swift
@@ -12,11 +12,11 @@ let package = Package(
     products: [
         .library(name: "PostgresNIO", targets: ["PostgresNIO"]),
     ],
     dependencies: [
         .package(url: "https://github.com/apple/swift-atomics.git", from: "1.1.0"),
-        .package(url: "https://github.com/apple/swift-nio.git", from: "2.51.1"),
+        .package(url: "https://github.com/apple/swift-nio.git", from: "2.52.0"),
         .package(url: "https://github.com/apple/swift-nio-transport-services.git", from: "1.16.0"),
         .package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.23.1"),
         .package(url: "https://github.com/apple/swift-crypto.git", "1.0.0" ..< "3.0.0"),
         .package(url: "https://github.com/apple/swift-metrics.git", from: "2.0.0"),
         .package(url: "https://github.com/apple/swift-log.git", from: "1.5.2"),
diff --git a/Sources/PostgresNIO/New/Connection State Machine/ConnectionStateMachine.swift b/Sources/PostgresNIO/New/Connection State Machine/ConnectionStateMachine.swift
index 65b43cd..563bb02 100644
--- a/Sources/PostgresNIO/New/Connection State Machine/ConnectionStateMachine.swift	
+++ b/Sources/PostgresNIO/New/Connection State Machine/ConnectionStateMachine.swift	
@@ -226,13 +226,16 @@ struct ConnectionStateMachine {
         case .modifying:
             preconditionFailure("Invalid state")
         }
     }
     
-    mutating func sslSupportedReceived() -> ConnectionAction {
+    mutating func sslSupportedReceived(unprocessedBytes: Int) -> ConnectionAction {
         switch self.state {
         case .sslRequestSent:
+            if unprocessedBytes > 0 {
+                return self.closeConnectionAndCleanup(.receivedUnencryptedDataAfterSSLRequest)
+            }
             self.state = .sslNegotiated
             return .establishSSLConnection
             
         case .initialized,
              .sslNegotiated,
@@ -1077,13 +1080,22 @@ extension ConnectionStateMachine {
 }
 
 extension ConnectionStateMachine {
     func shouldCloseConnection(reason error: PSQLError) -> Bool {
         switch error.code.base {
-        case .sslUnsupported:
-            return true
-        case .failedToAddSSLHandler:
+        case .failedToAddSSLHandler,
+             .receivedUnencryptedDataAfterSSLRequest,
+             .sslUnsupported,
+             .messageDecodingFailure,
+             .unexpectedBackendMessage,
+             .unsupportedAuthMechanism,
+             .authMechanismRequiresPassword,
+             .saslError,
+             .tooManyParameters,
+             .invalidCommandTag,
+             .connectionError,
+             .uncleanShutdown:
             return true
         case .queryCancelled:
             return false
         case .server:
             guard let sqlState = error.serverInfo?[.sqlState] else {
@@ -1095,32 +1107,14 @@ extension ConnectionStateMachine {
                 // these are authentication errors
                 return true
             }
             
             return false
-        case .messageDecodingFailure:
-            return true
-        case .unexpectedBackendMessage:
-            return true
-        case .unsupportedAuthMechanism:
-            return true
-        case .authMechanismRequiresPassword:
-            return true
-        case .saslError:
-            return true
-        case .tooManyParameters:
-            return true
-        case .invalidCommandTag:
-            return true
         case .connectionQuiescing:
             preconditionFailure("Pure client error, that is thrown directly in PostgresConnection")
         case .connectionClosed:
             preconditionFailure("Pure client error, that is thrown directly and should never ")
-        case .connectionError:
-            return true
-        case .uncleanShutdown:
-            return true
         }
     }
 
     mutating func setErrorAndCreateCleanupContextIfNeeded(_ error: PSQLError) -> ConnectionAction.CleanUpContext? {
         guard self.shouldCloseConnection(reason: error) else {
diff --git a/Sources/PostgresNIO/New/PSQLError.swift b/Sources/PostgresNIO/New/PSQLError.swift
index 2c2bac2..08b6a01 100644
--- a/Sources/PostgresNIO/New/PSQLError.swift
+++ b/Sources/PostgresNIO/New/PSQLError.swift
@@ -5,10 +5,11 @@ public struct PSQLError: Error {
 
     public struct Code: Sendable, Hashable, CustomStringConvertible {
         enum Base: Sendable, Hashable {
             case sslUnsupported
             case failedToAddSSLHandler
+            case receivedUnencryptedDataAfterSSLRequest
             case server
             case messageDecodingFailure
             case unexpectedBackendMessage
             case unsupportedAuthMechanism
             case authMechanismRequiresPassword
@@ -29,10 +30,11 @@ public struct PSQLError: Error {
             self.base = base
         }
 
         public static let sslUnsupported = Self.init(.sslUnsupported)
         public static let failedToAddSSLHandler = Self(.failedToAddSSLHandler)
+        public static let receivedUnencryptedDataAfterSSLRequest = Self(.receivedUnencryptedDataAfterSSLRequest)
         public static let server = Self(.server)
         public static let messageDecodingFailure = Self(.messageDecodingFailure)
         public static let unexpectedBackendMessage = Self(.unexpectedBackendMessage)
         public static let unsupportedAuthMechanism = Self(.unsupportedAuthMechanism)
         public static let authMechanismRequiresPassword = Self(.authMechanismRequiresPassword)
@@ -49,10 +51,12 @@ public struct PSQLError: Error {
             switch self.base {
             case .sslUnsupported:
                 return "sslUnsupported"
             case .failedToAddSSLHandler:
                 return "failedToAddSSLHandler"
+            case .receivedUnencryptedDataAfterSSLRequest:
+                return "receivedUnencryptedDataAfterSSLRequest"
             case .server:
                 return "server"
             case .messageDecodingFailure:
                 return "messageDecodingFailure"
             case .unexpectedBackendMessage:
@@ -341,10 +345,12 @@ public struct PSQLError: Error {
 
     static var queryCancelled: PSQLError { PSQLError(code: .queryCancelled) }
 
     static var uncleanShutdown: PSQLError { PSQLError(code: .uncleanShutdown) }
 
+    static var receivedUnencryptedDataAfterSSLRequest: PSQLError { PSQLError(code: .receivedUnencryptedDataAfterSSLRequest) }
+
     static func server(_ response: PostgresBackendMessage.ErrorResponse) -> PSQLError {
         var error = PSQLError(code: .server)
         error.serverInfo = .init(response)
         return error
     }
diff --git a/Sources/PostgresNIO/New/PostgresChannelHandler.swift b/Sources/PostgresNIO/New/PostgresChannelHandler.swift
index a3cd1e4..84f07d4 100644
--- a/Sources/PostgresNIO/New/PostgresChannelHandler.swift
+++ b/Sources/PostgresNIO/New/PostgresChannelHandler.swift
@@ -137,11 +137,11 @@ final class PostgresChannelHandler: ChannelDuplexHandler {
                 case .readyForQuery(let transactionState):
                     action = self.state.readyForQueryReceived(transactionState)
                 case .rowDescription(let rowDescription):
                     action = self.state.rowDescriptionReceived(rowDescription)
                 case .sslSupported:
-                    action = self.state.sslSupportedReceived()
+                    action = self.state.sslSupportedReceived(unprocessedBytes: self.decoder.unprocessedBytes)
                 case .sslUnsupported:
                     action = self.state.sslUnsupportedReceived()
                 }
                 
                 self.run(action, with: context)
diff --git a/Sources/PostgresNIO/Postgres+PSQLCompat.swift b/Sources/PostgresNIO/Postgres+PSQLCompat.swift
index 55870f8..ff9773f 100644
--- a/Sources/PostgresNIO/Postgres+PSQLCompat.swift
+++ b/Sources/PostgresNIO/Postgres+PSQLCompat.swift
@@ -29,10 +29,12 @@ extension PSQLError {
         case .unsupportedAuthMechanism:
             let message = self.unsupportedAuthScheme != nil ? String(describing: self.unsupportedAuthScheme!) : "no scheme"
             return PostgresError.protocol("Unsupported auth scheme: \(message)")
         case .authMechanismRequiresPassword:
             return PostgresError.protocol("Unable to authenticate without password")
+        case .receivedUnencryptedDataAfterSSLRequest:
+            return PostgresError.protocol("Received unencrypted data after SSL request")
         case .saslError:
             return self.underlying ?? self
         case .tooManyParameters, .invalidCommandTag:
             return self
         case .connectionQuiescing:
diff --git a/Tests/PostgresNIOTests/New/Connection State Machine/ConnectionStateMachineTests.swift b/Tests/PostgresNIOTests/New/Connection State Machine/ConnectionStateMachineTests.swift
index eaf427d..289665f 100644
--- a/Tests/PostgresNIOTests/New/Connection State Machine/ConnectionStateMachineTests.swift	
+++ b/Tests/PostgresNIOTests/New/Connection State Machine/ConnectionStateMachineTests.swift	
@@ -17,24 +17,31 @@ class ConnectionStateMachineTests: XCTestCase {
     
     func testSSLStartupSuccess() {
         let authContext = AuthContext(username: "test", password: "abc123", database: "test")
         var state = ConnectionStateMachine(requireBackendKeyData: true)
         XCTAssertEqual(state.connected(tls: .require), .sendSSLRequest)
-        XCTAssertEqual(state.sslSupportedReceived(), .establishSSLConnection)
+        XCTAssertEqual(state.sslSupportedReceived(unprocessedBytes: 0), .establishSSLConnection)
         XCTAssertEqual(state.sslHandlerAdded(), .wait)
         XCTAssertEqual(state.sslEstablished(), .provideAuthenticationContext)
         XCTAssertEqual(state.provideAuthenticationContext(authContext), .sendStartupMessage(authContext))
         let salt: (UInt8, UInt8, UInt8, UInt8) = (0,1,2,3)
         XCTAssertEqual(state.authenticationMessageReceived(.md5(salt: salt)), .sendPasswordMessage(.md5(salt: salt), authContext))
     }
-    
+
+    func testSSLStartupFailureTooManyBytesRemaining() {
+        var state = ConnectionStateMachine(requireBackendKeyData: true)
+        XCTAssertEqual(state.connected(tls: .require), .sendSSLRequest)
+        let failError = PSQLError.receivedUnencryptedDataAfterSSLRequest
+        XCTAssertEqual(state.sslSupportedReceived(unprocessedBytes: 1), .closeConnectionAndCleanup(.init(action: .close, tasks: [], error: failError, closePromise: nil)))
+    }
+
     func testSSLStartupFailHandler() {
         struct SSLHandlerAddError: Error, Equatable {}
         
         var state = ConnectionStateMachine(requireBackendKeyData: true)
         XCTAssertEqual(state.connected(tls: .require), .sendSSLRequest)
-        XCTAssertEqual(state.sslSupportedReceived(), .establishSSLConnection)
+        XCTAssertEqual(state.sslSupportedReceived(unprocessedBytes: 0), .establishSSLConnection)
         let failError = PSQLError.failedToAddSSLHandler(underlying: SSLHandlerAddError())
         XCTAssertEqual(state.errorHappened(failError), .closeConnectionAndCleanup(.init(action: .close, tasks: [], error: failError, closePromise: nil)))
     }
     
     func testTLSRequiredStartupSSLUnsupported() {
diff --git a/Tests/PostgresNIOTests/New/PostgresChannelHandlerTests.swift b/Tests/PostgresNIOTests/New/PostgresChannelHandlerTests.swift
index 9e3bbef..7ab0ce3 100644
--- a/Tests/PostgresNIOTests/New/PostgresChannelHandlerTests.swift
+++ b/Tests/PostgresNIOTests/New/PostgresChannelHandlerTests.swift
@@ -75,11 +75,48 @@ class PostgresChannelHandlerTests: XCTestCase {
         
         XCTAssertEqual(startupMessage.parameters.user, config.username)
         XCTAssertEqual(startupMessage.parameters.database, config.database)
         XCTAssertEqual(startupMessage.parameters.replication, .false)
     }
-    
+
+    func testEstablishSSLCallbackIsNotCalledIfSSLIsSupportedButAnotherMEssageIsSentAsWell() {
+        var config = self.testConnectionConfiguration()
+        XCTAssertNoThrow(config.tls = .require(try NIOSSLContext(configuration: .makeClientConfiguration())))
+        var addSSLCallbackIsHit = false
+        let handler = PostgresChannelHandler(configuration: config) { channel in
+            addSSLCallbackIsHit = true
+        }
+        let eventHandler = TestEventHandler()
+        let embedded = EmbeddedChannel(handlers: [
+            ReverseByteToMessageHandler(PSQLFrontendMessageDecoder()),
+            handler,
+            eventHandler
+        ])
+
+        var maybeMessage: PostgresFrontendMessage?
+        XCTAssertNoThrow(embedded.connect(to: try .init(ipAddress: "0.0.0.0", port: 5432), promise: nil))
+        XCTAssertNoThrow(maybeMessage = try embedded.readOutbound(as: PostgresFrontendMessage.self))
+        guard case .sslRequest(let request) = maybeMessage else {
+            return XCTFail("Unexpected message")
+        }
+
+        XCTAssertEqual(request.code, 80877103)
+
+        var responseBuffer = ByteBuffer()
+        responseBuffer.writeInteger(UInt8(ascii: "S"))
+        responseBuffer.writeInteger(UInt8(ascii: "1"))
+        XCTAssertNoThrow(try embedded.writeInbound(responseBuffer))
+
+        XCTAssertFalse(addSSLCallbackIsHit)
+
+        // the event handler should have seen an error
+        XCTAssertEqual(eventHandler.errors.count, 1)
+
+        // the connections should be closed
+        XCTAssertFalse(embedded.isActive)
+    }
+
     func testSSLUnsupportedClosesConnection() throws {
         let config = self.testConnectionConfiguration(tls: .require(try NIOSSLContext(configuration: .makeClientConfiguration())))
         
         let handler = PostgresChannelHandler(configuration: config) { channel in
             XCTFail("This callback should never be exectuded")

Vulnerable code:         .package(url: "https://github.com/apple/swift-nio.git", from: "2.51.1"),
    mutating func sslSupportedReceived() -> ConnectionAction {
        case .sslUnsupported:
            return true
        case .failedToAddSSLHandler:
        case .messageDecodingFailure:
            return true
        case .unexpectedBackendMessage:
            return true
        case .unsupportedAuthMechanism:
            return true
        case .authMechanismRequiresPassword:
            return true
        case .saslError:
            return true
        case .tooManyParameters:
            return true
        case .invalidCommandTag:
            return true
        case .connectionError:
            return true
        case .uncleanShutdown:
            return true
                    action = self.state.sslSupportedReceived()
        XCTAssertEqual(state.sslSupportedReceived(), .establishSSLConnection)
    
        XCTAssertEqual(state.sslSupportedReceived(), .establishSSLConnection)
    
Fixed code:         .package(url: "https://github.com/apple/swift-nio.git", from: "2.52.0"),
    mutating func sslSupportedReceived(unprocessedBytes: Int) -> ConnectionAction {
            if unprocessedBytes > 0 {
                return self.closeConnectionAndCleanup(.receivedUnencryptedDataAfterSSLRequest)
            }
        case .failedToAddSSLHandler,
             .receivedUnencryptedDataAfterSSLRequest,
             .sslUnsupported,
             .messageDecodingFailure,
             .unexpectedBackendMessage,
             .unsupportedAuthMechanism,
             .authMechanismRequiresPassword,
             .saslError,
             .tooManyParameters,
             .invalidCommandTag,
             .connectionError,
             .uncleanShutdown:
            case receivedUnencryptedDataAfterSSLRequest
        public static let receivedUnencryptedDataAfterSSLRequest = Self(.receivedUnencryptedDataAfterSSLRequest)
            case .receivedUnencryptedDataAfterSSLRequest:
                return "receivedUnencryptedDataAfterSSLRequest"
    static var receivedUnencryptedDataAfterSSLRequest: PSQLError { PSQLError(code: .receivedUnencryptedDataAfterSSLRequest) }

                    action = self.state.sslSupportedReceived(unprocessedBytes: self.decoder.unprocessedBytes)
        case .receivedUnencryptedDataAfterSSLRequest:
            return PostgresError.protocol("Received unencrypted data after SSL request")
        XCTAssertEqual(state.sslSupportedReceived(unprocessedBytes: 0), .establishSSLConnection)

    func testSSLStartupFailureTooManyBytesRemaining() {
        var state = ConnectionStateMachine(requireBackendKeyData: true)
        XCTAssertEqual(state.connected(tls: .require), .sendSSLRequest)
        let failError = PSQLError.receivedUnencryptedDataAfterSSLRequest
        XCTAssertEqual(state.sslSupportedReceived(unprocessedBytes: 1), .closeConnectionAndCleanup(.init(action: .close, tasks: [], error: failError, closePromise: nil)))
    }

        XCTAssertEqual(state.sslSupportedReceived(unprocessedBytes: 0), .establishSSLConnection)

    func testEstablishSSLCallbackIsNotCalledIfSSLIsSupportedButAnotherMEssageIsSentAsWell() {
        var config = self.testConnectionConfiguration()
        XCTAssertNoThrow(config.tls = .require(try NIOSSLContext(configuration: .makeClientConfiguration())))
        var addSSLCallbackIsHit = false
        let handler = PostgresChannelHandler(configuration: config) { channel in
            addSSLCallbackIsHit = true
        }
        let eventHandler = TestEventHandler()
        let embedded = EmbeddedChannel(handlers: [
            ReverseByteToMessageHandler(PSQLFrontendMessageDecoder()),
            handler,
            eventHandler
        ])

        var maybeMessage: PostgresFrontendMessage?
        XCTAssertNoThrow(embedded.connect(to: try .init(ipAddress: "0.0.0.0", port: 5432), promise: nil))
        XCTAssertNoThrow(maybeMessage = try embedded.readOutbound(as: PostgresFrontendMessage.self))
        guard case .sslRequest(let request) = maybeMessage else {
            return XCTFail("Unexpected message")
        }

        XCTAssertEqual(request.code, 80877103)

        var responseBuffer = ByteBuffer()
        responseBuffer.writeInteger(UInt8(ascii: "S"))
        responseBuffer.writeInteger(UInt8(ascii: "1"))
        XCTAssertNoThrow(try embedded.writeInbound(responseBuffer))

        XCTAssertFalse(addSSLCallbackIsHit)

        // the event handler should have seen an error
        XCTAssertEqual(eventHandler.errors.count, 1)

        // the connections should be closed
        XCTAssertFalse(embedded.isActive)
    }