CVE-2023-44386 (Swift, published 2023-10-05T18:15Z)
Description: value: Vapor is an HTTP web framework for Swift. There is a denial of service vulnerability impacting all users of affected versions of Vapor. The HTTP1 error handler closed connections when HTTP parse errors occur instead of passing them on. The issue is fixed as of Vapor release 4.84.2.
Severity: nan (CVSS v3 base score: 5.3)
Weakness: CWE-696 \u2014 Incorrect Behavior Order
The product performs multiple related behaviors, but the behaviors are performed in the wrong order in ways which may produce resultant weaknesses.
Repository: https://github.com/vapor/vapor
Files changed:
Sources/Vapor/HTTP/Server/HTTPServer.swift
Sources/Vapor/HTTP/Server/HTTPServerHandler.swift
Sources/Vapor/HTTP/Server/HTTPServerRequestDecoder.swift
Tests/VaporTests/PipelineTests.swift
Commit message: Merge pull request from GHSA-3mwq-h3g6-ffhm

* Fix the HTTP1 error handler to close connections when HTTP parse errors occur instead of passing them on.

* Move the error handling for parser errors into HTTPServerRequestDecoder, where it arguably belongs. Don't send 400 bad request for those errors (not required by RFC, difficult to do safely for all cases), fix inbound user event handling in multiple handlers.

Vulnerable code:
/// A simple channel handler that catches errors emitted by parsing HTTP requests
/// and sends 400 Bad Request responses.
///
/// This channel handler provides the basic behaviour that the majority of simple HTTP
/// servers want. This handler does not suppress the parser errors: it allows them to
/// continue to pass through the pipeline so that other handlers (e.g. logging ones) can
/// deal with the error.
/// 
/// adapted from: https://github.com/apple/swift-nio/blob/00341c92770e0a7bebdc5fda783f08765eb3ff56/Sources/NIOHTTP1/HTTPServerProtocolErrorHandler.swift
final class HTTP1ServerErrorHandler: ChannelDuplexHandler, RemovableChannelHandler {
    typealias InboundIn = Never
    typealias InboundOut = Never
    typealias OutboundIn = HTTPServerResponsePart
    typealias OutboundOut = HTTPServerResponsePart
    let logger: Logger
    private var hasUnterminatedResponse: Bool = false
    
    init(logger: Logger) {
        self.logger = logger
    }
    
    func errorCaught(context: ChannelHandlerContext, error: Error) {
        if let error = error as? HTTPParserError {
            self.makeHTTPParserErrorResponse(context: context, error: error)
        }

        // Now pass the error on in case someone else wants to see it.
        // In the Vapor ChannelPipeline the connection will eventually 
        // be closed by the NIOCloseOnErrorHandler
        context.fireErrorCaught(error)
    }

    private func makeHTTPParserErrorResponse(context: ChannelHandlerContext, error: HTTPParserError) {
        // Any HTTPParserError is automatically fatal, and we don't actually need (or want) to
        // provide that error to the client: we just want to inform them something went wrong
        // and then close off the pipeline. However, we can only send an
        // HTTP error response if another response hasn't started yet.
        //
        // A side note here: we cannot block or do any delayed work. 
        // The channel might be closed right after we return from this function.
        if !self.hasUnterminatedResponse {
            self.logger.debug("Bad Request - Invalid HTTP: \(error)")
            let headers = HTTPHeaders([("Connection", "close"), ("Content-Length", "0")])
            let head = HTTPResponseHead(version: .http1_1, status: .badRequest, headers: headers)
            context.write(self.wrapOutboundOut(.head(head)), promise: nil)
            context.writeAndFlush(self.wrapOutboundOut(.end(nil)), promise: nil)
        }
    }

    public func write(context: ChannelHandlerContext, data: NIOAny, promise: EventLoopPromise<Void>?) {
        let res = self.unwrapOutboundIn(data)
        switch res {
        case .head(let head) where head.isInformational:
            precondition(!self.hasUnterminatedResponse)
        case .head:
            precondition(!self.hasUnterminatedResponse)
            self.hasUnterminatedResponse = true
        case .body:
            precondition(self.hasUnterminatedResponse)
        case .end:
            precondition(self.hasUnterminatedResponse)
            self.hasUnterminatedResponse = false
        }
        context.write(data, promise: promise)
    }
}

        let errorHandler = HTTP1ServerErrorHandler(logger: configuration.logger)
        handlers.append(errorHandler)

            self.logger.trace("Unhandled user event: \(event)")
            self.logger.trace("Unhandled user event: \(event)")
        try XCTAssertContains(channel.readOutbound(as: ByteBuffer.self)?.string, "HTTP/1.1 400 Bad Request")

Fixed code:
            context.fireUserInboundEventTriggered(event)

        if error is HTTPParserError {
            self.logger.debug("Invalid HTTP request, will close connection: \(String(reflecting: error))")
        }
            context.fireUserInboundEventTriggered(event)