Harden HTTP.jl security defaults for v2.4.0

Project.toml

@@ -1,6 +1,6 @@
 name = "HTTP"
 uuid = "cd3eb016-35fb-5094-929b-558a96fad6f3"
-version = "2.3.1"
+version = "2.4.0"
 authors = ["Jacob Quinn", "contributors: https://github.com/JuliaWeb/HTTP.jl/graphs/contributors"]
 
 [deps]

docs/src/guides/protocols.md

@@ -95,7 +95,9 @@ end
 HTTP and WebSocket routes. Compression is most beneficial for larger, repetitive
 text/JSON payloads; tiny or already-compressed (binary/media) messages gain
 little. Decompressed message size is bounded by `maxframesize`, guarding against
-decompression bombs.
+decompression bombs. `maxframesize` defaults to 16 MiB for high-level
+WebSocket clients and servers; pass a larger value explicitly if your protocol
+requires larger messages.
 
 ## HTTP/2 Support
 

docs/src/guides/server.md

@@ -112,6 +112,12 @@ server = HTTP.serve!(
 The older `readtimeout` keyword is accepted as a seconds-valued migration alias
 for `read_timeout`.
 
+Ordinary `serve!` request handlers receive a buffered `HTTP.Request` body. That
+buffering is capped by `max_body_bytes`, which defaults to 64 MiB. Raise the
+limit for larger in-memory uploads, pass `max_body_bytes = 0` to restore legacy
+unbounded buffering, or use `listen!`/stream handlers when the application
+should manage large request bodies incrementally.
+
 ## Routing and Middleware
 
 Use `HTTP.Router` when you want route matching without bringing in a

src/hpack.jl

@@ -584,6 +584,8 @@ end
     return 32 + ncodeunits(name) + ncodeunits(value)
 end
 
+const _MAX_ENCODER_DYNAMIC_TABLE_SIZE = 64 * 1024
+
 function DynamicTable(max_size::Integer=4096)
     max_size >= 0 || throw(ArgumentError("dynamic table max_size must be >= 0"))
     return DynamicTable(DynamicTableEntry[], Int(max_size), 0)

src/http1.jl

@@ -248,8 +248,16 @@ function _parse_transfer_encoding!(hdrs::Headers, proto_major::UInt8, proto_mino
     values = headers(hdrs, "Transfer-Encoding")
     isempty(values) && return false
     if proto_major < UInt8(1) || (proto_major == UInt8(1) && proto_minor < UInt8(1))
-        removeheader(hdrs, "Transfer-Encoding")
-        return false
+        # RFC 9112 §6.1: an HTTP/1.0 message must not use Transfer-Encoding; a
+        # recipient that receives one has faulty framing. The old behavior
+        # silently dropped the header and fell back to Content-Length while
+        # leaving a keep-alive connection open, so a reverse proxy that framed
+        # the body as chunked could leave the trailing chunked bytes on the
+        # reused connection for HTTP.jl to parse as a smuggled pipelined request
+        # (ANT-2026-YD5QTQDZ). Reject instead: this surfaces as a 400 and the
+        # server closes the connection (Response close=true), so no ambiguous
+        # bytes remain on the transport.
+        throw(ProtocolError("Transfer-Encoding is not allowed in HTTP/1.0 messages"))
     end
     length(values) == 1 || throw(ProtocolError("too many Transfer-Encoding header values"))
     raw = _trim_http_ows(values[1])
@@ -281,6 +289,27 @@ function _validate_request_target!(method::String, target::String)::Nothing
     end
 end
 
+# Client-side guard for the HTTP/1 request start line. The method and target
+# flow from caller-constructed `Request` objects (often built by interpolating
+# user-controlled URL components) straight onto the wire via `print`, so we must
+# reject CR/LF and other control bytes here before serialization. Without this an
+# attacker who influences the URL path/query/method could embed `\r\n` to inject
+# headers or smuggle a pipelined request on a pooled keep-alive connection. The
+# server read path already enforces this via `_validate_request_target!`; this is
+# the symmetric check for the client write path (the HTTP/2 client validates
+# `:path`/`:method` analogously). `host`, when present, is the authority used in
+# absolute-form/proxy-forward start lines and must be control-byte-free as well.
+function _validate_request_start_line!(method::String, target::String, host::Union{Nothing,AbstractString}=nothing)::Nothing
+    # An HTTP method is an RFC 7230 token; `_valid_header_field_name` enforces
+    # exactly that grammar (and so excludes spaces, CR, LF and other CTLs).
+    _valid_header_field_name(method) || throw(ParseError("invalid HTTP method: $(repr(method))"))
+    _validate_request_target!(method, target)
+    if host !== nothing
+        _string_contains_ctl_byte(host) && throw(ParseError("invalid HTTP request host: $(repr(host))"))
+    end
+    return nothing
+end
+
 function _validate_request_host!(hdrs::Headers, method::String, proto_major::UInt8, proto_minor::UInt8)::Union{Nothing,String}
     host_values = headers(hdrs, "Host")
     if (proto_major > UInt8(1) || (proto_major == UInt8(1) && proto_minor >= UInt8(1))) &&
@@ -331,19 +360,37 @@ function _consume_crlf(io::IO)
 end
 
 function _parse_chunk_size(line::AbstractString)::Int64
-    trimmed = _trim_http_ows(line)
-    isempty(trimmed) && throw(ParseError("empty chunk size line"))
-    semi = findfirst(';', trimmed)
-    token = semi === nothing ? trimmed : String(SubString(trimmed, firstindex(trimmed), prevind(trimmed, semi)))
-    token = _trim_http_ows(token)
-    isempty(token) && throw(ParseError("empty chunk size"))
-    size = try
-        parse(Int64, token; base=16)
-    catch
-        throw(ParseError("invalid chunk size: $(repr(line))"))
+    # RFC 9112 §7.1: chunk = chunk-size [ chunk-ext ] CRLF, where
+    #   chunk-size = 1*HEXDIG. The chunk-size token must be one or more ASCII
+    # hex digits with no sign, no "0x" prefix, and no surrounding whitespace.
+    # We split off any chunk extension at the first ';' and then validate the
+    # remaining token byte-by-byte instead of using Base.parse(Int64,...;base=16),
+    # which silently tolerates '+'/'-', a "0x" prefix, and isspace padding
+    # (including a trailing bare CR). Accepting those forms lets HTTP.jl disagree
+    # with an RFC-strict front-end proxy about where the chunked body ends, which
+    # enables HTTP request smuggling (ANT-2026-SRPX7DN1). This mirrors the strict
+    # decimal parser used for Content-Length (_parse_int64_decimal_header_value).
+    semi = findfirst(';', line)
+    token = semi === nothing ? line : SubString(line, firstindex(line), prevind(line, semi))
+    units = codeunits(token)
+    isempty(units) && throw(ParseError("empty chunk size"))
+    limit = UInt64(typemax(Int64))
+    total = UInt64(0)
+    @inbounds for b in units
+        if 0x30 <= b <= 0x39          # '0'-'9'
+            digit = UInt64(b - 0x30)
+        elseif 0x41 <= b <= 0x46      # 'A'-'F'
+            digit = UInt64(b - 0x41 + 0x0a)
+        elseif 0x61 <= b <= 0x66      # 'a'-'f'
+            digit = UInt64(b - 0x61 + 0x0a)
+        else
+            throw(ParseError("invalid chunk size: $(repr(line))"))
+        end
+        # Guard against overflow past Int64 range (also keeps the result >= 0).
+        total > ((limit - digit) >> 4) && throw(ParseError("chunk size too large: $(repr(line))"))
+        total = (total << 4) + digit
     end
-    size < 0 && throw(ParseError("negative chunk size"))
-    return size
+    return Int64(total)
 end
 
 function _read_next_chunk!(body::ChunkedBody)
@@ -409,6 +456,8 @@ end
 
 function _write_start_line!(io::IO, request::Request, wire_target::Union{Nothing,AbstractString}=nothing)
     target = wire_target === nothing ? request.target : String(wire_target)
+    # Reject CR/LF/CTL in the method or target before they reach the socket.
+    _validate_request_start_line!(request.method, target)
     print(io, request.method, ' ', target, " HTTP/", Int(request.proto_major), '.', Int(request.proto_minor), "\r\n")
     return nothing
 end
@@ -479,14 +528,20 @@ function _status_text(status::Integer)::String
     return ""
 end
 
-function _write_status_line!(io::IO, response::Response)
+@inline function _response_reason_phrase(response::Response)::String
     reason = isempty(response.reason) ? _status_text(response.status) : response.reason
+    _string_contains_ctl_byte(reason) && throw(ProtocolError("invalid HTTP response reason phrase: $(repr(reason))"))
+    return reason
+end
+
+@inline function _write_status_line!(io::IO, response::Response)::Nothing
+    reason = _response_reason_phrase(response)
     print(io, "HTTP/", Int(response.proto_major), '.', Int(response.proto_minor), ' ', response.status, ' ', reason, "\r\n")
     return nothing
 end
 
-function _append_status_line!(buf::IOBuffer, response::Response)
-    reason = isempty(response.reason) ? _status_text(response.status) : response.reason
+@inline function _append_status_line!(buf::IOBuffer, response::Response)::Nothing
+    reason = _response_reason_phrase(response)
     print(buf, "HTTP/", Int(response.proto_major), '.', Int(response.proto_minor), ' ', response.status, ' ', reason, "\r\n")
     return nothing
 end
@@ -698,6 +753,8 @@ end
 
 function _append_start_line!(buf::IOBuffer, request::Request, wire_target::Union{Nothing,AbstractString}=nothing)
     target = wire_target === nothing ? request.target : String(wire_target)
+    # Reject CR/LF/CTL in the method or target before they reach the socket.
+    _validate_request_start_line!(request.method, target)
     print(buf, request.method, ' ', target, " HTTP/", Int(request.proto_major), '.', Int(request.proto_minor), "\r\n")
     return nothing
 end
@@ -897,6 +954,15 @@ function read_request(io::IO; max_line_bytes::Integer=_HTTP1_DEFAULT_MAX_LINE_BY
     method, target, proto_major, proto_minor = _parse_request_line(line)
     headers = _read_headers(io, max_line_bytes, max_header_bytes)
     chunked = _parse_transfer_encoding!(headers, proto_major, proto_minor)
+    # A request that carries BOTH Transfer-Encoding and Content-Length has
+    # ambiguous framing (RFC 9112 §6.1/§6.3): a front-end proxy may frame by one
+    # header while HTTP.jl frames by the other, leaving trailing bytes on a
+    # reused keep-alive connection that get parsed as a smuggled request
+    # (ANT-2026-YD5QTQDZ). Reject such requests outright (surfaced as 400 with
+    # the connection closed) instead of silently preferring Transfer-Encoding.
+    if chunked && hasheader(headers, "Content-Length")
+        throw(ProtocolError("request must not include both Transfer-Encoding and Content-Length"))
+    end
     content_length = _parse_content_length(headers)
     chunked && removeheader(headers, "Content-Length")
     content_length = chunked ? Int64(-1) : content_length

src/http2_client.jl

@@ -156,6 +156,25 @@ mutable struct H2Connection
     # Per-stream receive buffer cap applied to every `H2StreamState` opened on
     # this connection. Configured at connect time; defaults to 256 KiB.
     max_buffered_bytes::Int
+    # CONTINUATION sequencing state, mirroring the server read loop. When a
+    # HEADERS/CONTINUATION frame arrives without END_HEADERS, this records the
+    # stream id whose header block is still open; the next frame on the
+    # connection MUST be a CONTINUATION for the same stream (RFC 7540 6.10).
+    # Touched only from the single-threaded read loop, so it needs no lock.
+    continuation_stream::UInt32
+    # While a header block is open on a live stream, holds that stream's state so
+    # the trailing CONTINUATION frames are still delivered to it even if the
+    # application thread unregisters the stream mid-block (the read loop and
+    # `_unregister_stream!` run concurrently). `nothing` when no live block is
+    # open. Touched only from the single-threaded read loop.
+    open_header_stream::Union{Nothing,H2StreamState}
+    # Scratch buffer that accumulates header block fragments for a header block
+    # belonging to an unknown/closed stream. HPACK's dynamic table is
+    # connection-scoped (RFC 7541 2.3.2 / 4), so even discarded header blocks
+    # must be decoded as a single block to keep the decoder in sync; we buffer
+    # the fragments here until END_HEADERS, decode once, then discard.
+    # Touched only from the single-threaded read loop.
+    orphan_header_block::Vector{UInt8}
     # Concurrent acquirers that have committed to opening a stream on this
     # connection but haven't reached `_register_stream!` yet. Used by
     # `_h2_conn_available` so a second caller doesn't decide a peer-capped
@@ -343,7 +362,7 @@ function _apply_peer_settings!(conn::H2Connection, settings::Vector{Pair{UInt16,
         unlock(conn.state_lock)
     end
     if header_table_size !== nothing
-        size = header_table_size::Int
+        size = min(header_table_size::Int, _MAX_ENCODER_DYNAMIC_TABLE_SIZE)
         lock(conn.write_lock)
         try
             set_max_dynamic_table_size_limit!(conn.encoder, size)
@@ -752,9 +771,65 @@ function _send_window_updates!(conn::H2Connection, stream_id::UInt32, nbytes::In
     return nothing
 end
 
+"""
+    _consume_orphan_header_fragment!(conn, stream_id, fragment, end_headers)
+
+Buffer and (on END_HEADERS) HPACK-decode a header block that belongs to an
+unknown or already-closed stream, discarding the decoded result.
+
+HPACK's dynamic table is connection-scoped and mutated as a side effect of
+decoding each header block (RFC 7541 2.3.2, 4). A header block that is silently
+dropped instead of decoded leaves the client's dynamic table permanently out of
+step with the server's encoder, so every later indexed reference on *any*
+stream of the connection resolves against a stale table. We therefore must still
+feed these bytes through `conn.decoder`. Because HPACK instructions may straddle
+fragment boundaries, the block is accumulated across HEADERS/CONTINUATION frames
+and decoded once when END_HEADERS arrives, exactly as for live streams.
+"""
+function _consume_orphan_header_fragment!(
+    conn::H2Connection,
+    stream_id::UInt32,
+    fragment::Vector{UInt8},
+    end_headers::Bool,
+)
+    remaining = conn.max_header_block_bytes - length(conn.orphan_header_block)
+    if remaining < 0 || length(fragment) > remaining
+        # The peer cannot be allowed to grow this scratch buffer without bound.
+        # An oversized block on an unknown stream is a connection-level fault.
+        empty!(conn.orphan_header_block)
+        conn.continuation_stream = UInt32(0)
+        throw(ProtocolError("HTTP/2 response header block exceeded maximum size"))
+    end
+    append!(conn.orphan_header_block, fragment)
+    if end_headers
+        # Decode for the dynamic-table side effect only, then discard the result.
+        try
+            decode_header_block(conn.decoder, conn.orphan_header_block)
+        finally
+            empty!(conn.orphan_header_block)
+            conn.continuation_stream = UInt32(0)
+        end
+    else
+        conn.continuation_stream = stream_id
+    end
+    return nothing
+end
+
 function _process_incoming_frame!(conn::H2Connection, frame::AbstractFrame)
     # Frame-local validation already happened in `read_frame!`; this function is
     # responsible for connection- and stream-level state transitions.
+    #
+    # CONTINUATION sequencing (RFC 7540 6.10): a HEADERS/CONTINUATION frame that
+    # does not set END_HEADERS must be followed only by a CONTINUATION frame for
+    # the same stream. Any other frame in that window is a connection error. This
+    # mirrors the enforcement already performed in the server read loop.
+    if conn.continuation_stream != UInt32(0)
+        if !(frame isa ContinuationFrame && (frame::ContinuationFrame).stream_id == conn.continuation_stream)
+            throw(ProtocolError("expected CONTINUATION for stream $(conn.continuation_stream)"))
+        end
+    elseif frame isa ContinuationFrame
+        throw(ProtocolError("unexpected CONTINUATION frame"))
+    end
     if frame isa SettingsFrame
         settings = frame::SettingsFrame
         if !settings.ack
@@ -805,9 +880,8 @@ function _process_incoming_frame!(conn::H2Connection, frame::AbstractFrame)
         try
             if update.stream_id == UInt32(0)
                 conn.conn_send_window += increment
-            else
-                current = get(() -> conn.initial_stream_send_window, conn.stream_send_window, update.stream_id)
-                conn.stream_send_window[update.stream_id] = current + increment
+            elseif haskey(conn.stream_send_window, update.stream_id)
+                conn.stream_send_window[update.stream_id] += increment
             end
             notify(conn.window_condition; all=true)
         finally
@@ -817,16 +891,48 @@ function _process_incoming_frame!(conn::H2Connection, frame::AbstractFrame)
     elseif frame isa HeadersFrame
         headers = frame::HeadersFrame
         state = _stream_state(conn, headers.stream_id)
-        state === nothing && return nothing
+        if state === nothing
+            # Unknown or already-closed stream (e.g. trailers racing
+            # `_unregister_stream!`, or a malicious server). The header block is
+            # not delivered to any application stream, but it MUST still pass
+            # through the HPACK decoder to keep the connection's dynamic table
+            # synchronized (RFC 7541 4); otherwise all later streams desync.
+            _consume_orphan_header_fragment!(conn, headers.stream_id, headers.header_block_fragment, headers.end_headers)
+            return nothing
+        end
         _handle_stream_header_fragment!(conn, state::H2StreamState, headers.header_block_fragment, headers.end_headers, headers.end_stream)
+        # Track an open header block so a missing END_HEADERS forces CONTINUATION
+        # (RFC 7540 6.10) and the trailing frames still reach this stream even if
+        # the application thread unregisters it before the block completes.
+        if headers.end_headers
+            conn.continuation_stream = UInt32(0)
+            conn.open_header_stream = nothing
+        else
+            conn.continuation_stream = headers.stream_id
+            conn.open_header_stream = state::H2StreamState
+        end
         return nothing
     elseif frame isa PushPromiseFrame
         throw(ProtocolError("HTTP/2 server push is unsupported"))
     elseif frame isa ContinuationFrame
         cont = frame::ContinuationFrame
-        state = _stream_state(conn, cont.stream_id)
-        state === nothing && return nothing
-        _handle_stream_header_fragment!(conn, state::H2StreamState, cont.header_block_fragment, cont.end_headers, false)
+        open_state = conn.open_header_stream
+        if open_state !== nothing
+            # Continuation of a header block that began on a live stream. Route
+            # it to the captured state even if the stream was unregistered
+            # mid-block so the block is decoded as a single unit.
+            _handle_stream_header_fragment!(conn, open_state::H2StreamState, cont.header_block_fragment, cont.end_headers, false)
+            if cont.end_headers
+                conn.continuation_stream = UInt32(0)
+                conn.open_header_stream = nothing
+            else
+                conn.continuation_stream = cont.stream_id
+            end
+            return nothing
+        end
+        # Otherwise this continues a header block for an unknown/closed stream:
+        # decode for the HPACK side effect and discard, just like HEADERS above.
+        _consume_orphan_header_fragment!(conn, cont.stream_id, cont.header_block_fragment, cont.end_headers)
         return nothing
     elseif frame isa DataFrame
         data = frame::DataFrame
@@ -1014,6 +1120,9 @@ function _connect_h2_from_tcp!(
             true,
             _H2_DEFAULT_MAX_HEADER_BLOCK_BYTES,
             _h2_buffered_bytes(http2_settings),
+            UInt32(0),
+            nothing,
+            UInt8[],
             0,
             false,
         )