Skip to content

Bound proxy HTTP response body read to a fixed size limit - #1202

Open
iaohkut-from-NightWolf-Team wants to merge 1 commit into
centrifugal:masterfrom
iaohkut-from-NightWolf-Team:hardening/proxy-response-size-limit
Open

Bound proxy HTTP response body read to a fixed size limit#1202
iaohkut-from-NightWolf-Team wants to merge 1 commit into
centrifugal:masterfrom
iaohkut-from-NightWolf-Team:hardening/proxy-response-size-limit

Conversation

@iaohkut-from-NightWolf-Team

Copy link
Copy Markdown

Summary

CallHTTP in internal/proxy/http.go reads the entire response body from a proxy backend via io.ReadAll(resp.Body) with no upper bound. Every HTTP proxy call type (connect, refresh, publish, subscribe, sub_refresh, rpc, map_publish, map_remove, shared_poll_refresh) shares this same function, so a slow-but-huge or fast-but-massive response from the configured proxy backend gets fully buffered into memory regardless of size. The client Timeout only bounds wall-clock time, not response size.

This is defense-in-depth hardening, not an exploit against a normal client — the proxy Endpoint is fixed at startup from operator config and is never influenced by request/client data. But if a configured proxy backend is buggy, misconfigured, or itself compromised, it can currently force unbounded memory growth in Centrifugo, since many concurrent client connections can each be mid-proxy-call at once.

Other request-body reads in this codebase already follow a bounded-size pattern (unisse/unihttpstream's MaxRequestBodySize + http.MaxBytesReader), so this brings the proxy response path in line with that existing convention.

Change

limitedReader := io.LimitReader(resp.Body, maxProxyResponseBodySize+1)
respData, err := io.ReadAll(limitedReader)
if err != nil {
    return nil, fmt.Errorf("error reading HTTP body: %w", err)
}
if len(respData) > maxProxyResponseBodySize {
    return nil, fmt.Errorf("proxy response exceeds max allowed size of %d bytes", maxProxyResponseBodySize)
}

maxProxyResponseBodySize is a new 5MB constant in the same file. The explicit over-limit check is needed because io.LimitReader truncates silently rather than erroring — reading limit+1 bytes and checking the actual length is what turns that into a real error instead of silently accepting a truncated response.

Test plan

  • go build ./internal/proxy/...
  • go test ./internal/proxy/... — existing tests pass unchanged
  • Maintainer call on whether 5MB is the right default, and whether it should be made configurable per-proxy (kept it as a fixed constant here to keep the change minimal — happy to add a config option if preferred)

CallHTTP buffered the entire proxy backend response via io.ReadAll with
no upper bound. A slow, huge, or misbehaving proxy backend response can
force unbounded memory growth on every proxied call (connect, refresh,
publish, subscribe, rpc, map_publish, map_remove, shared_poll_refresh),
since they all share this code path. Bound it the same way other request
bodies are already bounded elsewhere in the codebase (unisse/unihttpstream
MaxRequestBodySize pattern), using io.LimitReader plus an explicit
over-limit check since LimitReader silently truncates rather than erroring.

Co-Authored-By: iaohkut <thb2601@gmail.com>
@FZambia

FZambia commented Aug 18, 2026

Copy link
Copy Markdown
Member

Hi @iaohkut-from-NightWolf-Team, thx! Can we make const 16 MB? For this exact defense-in-depth hardening and this part this value seems better fit. I don't think we need to have this configurable, so larger value seems safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants