From 568dd536d30abf95bb2a843b28c7bf2c355b00b4 Mon Sep 17 00:00:00 2001 From: Rui Paulo Date: Thu, 17 Sep 2026 11:06:09 -0700 Subject: [PATCH] QUIC: fix safety issues found by fuzzing - Stop processing coalesced packets and frames once the connection has reached a terminal state, avoiding use of torn-down per-connection state (e.g. crypto) after a graceful or error close - Finalize embedded frames via new `QUICFrame.discard` when rejecting a frame before `processFrame`, preventing a fatal precondition - Clear crypto reassembly queues on teardown and guard TLS restart on missing TLS options during INITIAL retransmit - Gate transport parameter and stateless reset token error logging behind `DisableErrorLogging` for fuzzing builds --- Sources/SwiftNetwork/QUIC/Crypto.swift | 3 ++ .../SwiftNetwork/QUIC/QUICConnection.swift | 35 +++++++++++++++++-- Sources/SwiftNetwork/QUIC/QUICFrame.swift | 20 +++++++++++ .../QUIC/StatelessResetToken.swift | 4 +++ .../QUIC/TransportParameters.swift | 16 +++++++++ 5 files changed, 75 insertions(+), 3 deletions(-) diff --git a/Sources/SwiftNetwork/QUIC/Crypto.swift b/Sources/SwiftNetwork/QUIC/Crypto.swift index 65619aaa..64958042 100644 --- a/Sources/SwiftNetwork/QUIC/Crypto.swift +++ b/Sources/SwiftNetwork/QUIC/Crypto.swift @@ -173,6 +173,9 @@ final class QUICCrypto { initialOutboundData.empty() handshakeOutboundData.empty() applicationOutboundData.empty() + initialReassemblyQueue.dequeueAll() + handshakeReassemblyQueue.dequeueAll() + applicationReassemblyQueue.dequeueAll() self.parentConnection = nil } diff --git a/Sources/SwiftNetwork/QUIC/QUICConnection.swift b/Sources/SwiftNetwork/QUIC/QUICConnection.swift index 55ecde96..885cbe0d 100644 --- a/Sources/SwiftNetwork/QUIC/QUICConnection.swift +++ b/Sources/SwiftNetwork/QUIC/QUICConnection.swift @@ -1712,7 +1712,15 @@ public final class QUICConnection: ManyToManyApplicationStreamProtocol, break } - if closeError != nil { + if closeError != nil || state.isTerminal { + // Besides a locally-detected error (closeError), the peer may + // have gracefully closed the connection (CONNECTION_CLOSE + // with NO_ERROR, or an APPLICATION_CLOSE frame, neither of + // which set closeError) while processing this packet's + // frames. Either way, `close()` has already torn down crypto + // and other per-connection state (see closeTLSFlow()), so we + // must not hand any further coalesced packets in this + // datagram to that torn-down state. frame.finalize(success: false) close() return @@ -1809,6 +1817,7 @@ public final class QUICConnection: ManyToManyApplicationStreamProtocol, .protocolViolation, "Client sent initial packet with invalid QUIC frames" ) + QUICFrame.discard(quicFrame) return false } } @@ -1831,10 +1840,22 @@ public final class QUICConnection: ManyToManyApplicationStreamProtocol, log.error("Invalid frame type during the handshake: \(quicFrame.frameType)") closeFrameType = quicFrame.frameType close(with: .protocolViolation, "invalid frame type during the handshake") + QUICFrame.discard(quicFrame) + return false } if !processFrame(quicFrame, packetNumberSpace: packet.numberSpace, path: path) { break } + if state.isTerminal { + // Some frame handlers (e.g. CONNECTION_CLOSE, APPLICATION_CLOSE) + // call close() - which tears down crypto and other per-connection + // state - but still report success (return true) for the frame + // itself. Stop processing any further frames from this packet + // once that happens, rather than continuing to hand already + // torn-down state to later frames (e.g. a coalesced CRYPTO + // frame after a CONNECTION_CLOSE). + break + } } if unvalidatedPath { @@ -1932,10 +1953,14 @@ public final class QUICConnection: ManyToManyApplicationStreamProtocol, // Resetting congestion control will reset the pacer too currentPath.resetCongestionControl() log.info("Retransmitting INITIAL with version \(version.rawValue)") + guard let tlsOptions else { + log.error("Failed to start TLS: missing TLS options") + return + } // Resetting crypto here will guarantee the initial is sent again crypto.stop() crypto = QUICCrypto(context: context) - guard let tlsOptions, crypto.start(with: self, tlsOptions: tlsOptions) else { + guard crypto.start(with: self, tlsOptions: tlsOptions) else { log.error("Failed to start TLS") return } @@ -2026,10 +2051,14 @@ public final class QUICConnection: ManyToManyApplicationStreamProtocol, protector.deriveInitialSecrets(destinationCID: scid) log.info("Retransmitting INITIAL with token len: \(packet.tokenLength)") + guard let tlsOptions else { + log.error("Failed to start TLS: missing TLS options") + return + } // Resetting crypto here will guarantee the initial is sent again crypto.stop() crypto = QUICCrypto(context: context) - guard let tlsOptions, crypto.start(with: self, tlsOptions: tlsOptions) else { + guard crypto.start(with: self, tlsOptions: tlsOptions) else { log.error("Failed to start TLS") return } diff --git a/Sources/SwiftNetwork/QUIC/QUICFrame.swift b/Sources/SwiftNetwork/QUIC/QUICFrame.swift index 505c0f01..84073422 100644 --- a/Sources/SwiftNetwork/QUIC/QUICFrame.swift +++ b/Sources/SwiftNetwork/QUIC/QUICFrame.swift @@ -142,6 +142,26 @@ enum QUICFrame: ~Copyable { } } + // Some inbound QUIC frame kinds (CRYPTO, STREAM, DATAGRAM) hold an + // embedded `Frame` that borrows into the packet's buffer, and MUST be + // finalized exactly once before being released - see `Frame.deinit`. + // Callers that reject a `QUICFrame` before handing it to `processFrame` + // (e.g. because it's invalid in an INITIAL packet, or not allowed during + // the handshake) must use this instead of just letting the value go out + // of scope, otherwise the unfinalized `Frame` trips a fatal precondition. + static func discard(_ frame: consuming QUICFrame, success: Bool = false) { + switch consume frame { + case .crypto(var frame): + frame.frame.finalize(success: success) + case .stream(var frame): + frame.frame.finalize(success: success) + case .datagram(var frame): + frame.frame.finalize(success: success) + default: + break + } + } + static func parse( type: FrameType, frame: inout Frame, diff --git a/Sources/SwiftNetwork/QUIC/StatelessResetToken.swift b/Sources/SwiftNetwork/QUIC/StatelessResetToken.swift index f408c4e6..dcc8db32 100644 --- a/Sources/SwiftNetwork/QUIC/StatelessResetToken.swift +++ b/Sources/SwiftNetwork/QUIC/StatelessResetToken.swift @@ -41,7 +41,9 @@ public struct QUICStatelessResetToken: Equatable, Sendable, CustomStringConverti public init?(_ token: [UInt8]) { guard token.count == QUICStatelessResetToken.size else { + #if !DisableErrorLogging Logger.proto.fault("Invalid Stateless Reset Token") + #endif return nil } _token = TokenStorage(tokenSpan: token.span) @@ -49,7 +51,9 @@ public struct QUICStatelessResetToken: Equatable, Sendable, CustomStringConverti public init?(_ token: Span) { guard token.count == QUICStatelessResetToken.size else { + #if !DisableErrorLogging Logger.proto.fault("Invalid Stateless Reset Token") + #endif return nil } _token = TokenStorage(tokenSpan: token) diff --git a/Sources/SwiftNetwork/QUIC/TransportParameters.swift b/Sources/SwiftNetwork/QUIC/TransportParameters.swift index 4424350d..82c026b5 100644 --- a/Sources/SwiftNetwork/QUIC/TransportParameters.swift +++ b/Sources/SwiftNetwork/QUIC/TransportParameters.swift @@ -396,7 +396,9 @@ enum TransportParameter: Equatable { } guard vleSize == buffer.count else { let bufferCount = buffer.count + #if !DisableErrorLogging Logger.proto.error("VLE size \(vleSize) doesn't match TP size \(bufferCount)") + #endif throw QUICError.transportParametersDecode(TransportParameterDecodeErrors.invalidSize) } return value @@ -413,7 +415,9 @@ enum TransportParameter: Equatable { } guard vleSize == buffer.count else { let bufferCount = buffer.count + #if !DisableErrorLogging Logger.proto.error("VLE size \(vleSize) doesn't match TP size \(bufferCount)") + #endif throw QUICError.transportParametersDecode(TransportParameterDecodeErrors.invalidSize) } return value @@ -426,7 +430,9 @@ enum TransportParameter: Equatable { let connectionID = QUICConnectionID(buffer) else { let bufferCount = buffer.count + #if !DisableErrorLogging Logger.proto.error("ConnectionID size \(bufferCount) is invalid") + #endif throw QUICError.transportParametersDecode(TransportParameterDecodeErrors.invalidSize) } return connectionID @@ -439,7 +445,9 @@ enum TransportParameter: Equatable { let connectionID = QUICConnectionID(buffer) else { let bufferCount = buffer.count + #if !DisableErrorLogging Logger.proto.error("ConnectionID size \(bufferCount) is invalid") + #endif throw QUICError.transportParametersDecode(TransportParameterDecodeErrors.invalidSize) } return connectionID @@ -450,7 +458,9 @@ enum TransportParameter: Equatable { ) throws(QUICError) -> QUICStatelessResetToken { guard let statelessResetToken = QUICStatelessResetToken(buffer) else { let bufferCount = buffer.count + #if !DisableErrorLogging Logger.proto.error("StatelessResetToken size \(bufferCount) is invalid") + #endif throw QUICError.transportParametersDecode(TransportParameterDecodeErrors.invalidSize) } return statelessResetToken @@ -461,7 +471,9 @@ enum TransportParameter: Equatable { ) throws(QUICError) -> QUICStatelessResetToken { guard let statelessResetToken = QUICStatelessResetToken(buffer) else { let bufferCount = buffer.count + #if !DisableErrorLogging Logger.proto.error("StatelessResetToken size \(bufferCount) is invalid") + #endif throw QUICError.transportParametersDecode(TransportParameterDecodeErrors.invalidSize) } return statelessResetToken @@ -474,7 +486,9 @@ enum TransportParameter: Equatable { || buffer.count > PreferredAddress.maximumSize { let bufferCount = buffer.count + #if !DisableErrorLogging Logger.proto.error("PreferredAddress size \(bufferCount) is invalid") + #endif throw QUICError.transportParametersDecode(TransportParameterDecodeErrors.invalidSize) } var ipv4Address: UInt32 = 0 @@ -517,7 +531,9 @@ enum TransportParameter: Equatable { || buffer.count > PreferredAddress.maximumSize { let bufferCount = buffer.count + #if !DisableErrorLogging Logger.proto.error("PreferredAddress size \(bufferCount) is invalid") + #endif throw QUICError.transportParametersDecode(TransportParameterDecodeErrors.invalidSize) } var ipv4Address: UInt32 = 0