Skip to content

QUIC: fix safety issues found by fuzzing #157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rpaulo merged 1 commit into from
Sep 17, 2026
Merged

QUIC: fix safety issues found by fuzzing #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Sources/SwiftNetwork/QUIC/Crypto.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ final class QUICCrypto {
initialOutboundData.empty()
handshakeOutboundData.empty()
applicationOutboundData.empty()
initialReassemblyQueue.dequeueAll()
handshakeReassemblyQueue.dequeueAll()
applicationReassemblyQueue.dequeueAll()

self.parentConnection = nil
}
Expand Down
35 changes: 32 additions & 3 deletions Sources/SwiftNetwork/QUIC/QUICConnection.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1809,6 +1817,7 @@ public final class QUICConnection: ManyToManyApplicationStreamProtocol,
.protocolViolation,
"Client sent initial packet with invalid QUIC frames"
)
QUICFrame.discard(quicFrame)
return false
}
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
20 changes: 20 additions & 0 deletions Sources/SwiftNetwork/QUIC/QUICFrame.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions Sources/SwiftNetwork/QUIC/StatelessResetToken.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,19 @@ 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)
}

public init?(_ token: Span<UInt8>) {
guard token.count == QUICStatelessResetToken.size else {
#if !DisableErrorLogging
Logger.proto.fault("Invalid Stateless Reset Token")
#endif
return nil
}
_token = TokenStorage(tokenSpan: token)
Expand Down
16 changes: 16 additions & 0 deletions Sources/SwiftNetwork/QUIC/TransportParameters.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down