Skip to content

proxy/http: fix panic in readResponseAndHandle100Continue on malformed upstream response - #6545

Open
younesvatan78 wants to merge 1 commit into
XTLS:mainfrom
younesvatan78:fix-http-1xx-panic
Open

proxy/http: fix panic in readResponseAndHandle100Continue on malformed upstream response#6545
younesvatan78 wants to merge 1 commit into
XTLS:mainfrom
younesvatan78:fix-http-1xx-panic

Conversation

@younesvatan78

Copy link
Copy Markdown

Bug

readResponseAndHandle100Continue (HTTP inbound, plain-HTTP path) accumulates the upstream's 1xx response header into ResponseHeader1xx and checks for the end of headers with:

if bytes.Equal(ResponseHeader1xx[len(ResponseHeader1xx)-4:], []byte{'\r', '\n', '\r', '\n'}) {

There's no check that at least 4 bytes have been accumulated yet. ResponseHeader1xx is filled one r.ReadSlice('\n') result at a time, and if the upstream response contains a bare \n before the real status line, the first ReadSlice('\n') can return only 1-3 bytes. len(ResponseHeader1xx)-4 then goes negative and the slice panics:

panic: runtime error: slice bounds out of range [-2:]

github.com/xtls/xray-core/proxy/http.readResponseAndHandle100Continue(...)
	proxy/http/server.go:335

Minimal trigger, reproduced with a unit test (included in this PR): an upstream response body of

"X\nHTTP/1.1 100 Continue\r\n\r\n" + <padding>

The leading X\n gets picked up by r.ReadSlice('\n') as a 2-byte first read (since bufio's read position hasn't advanced past the earlier Peek(56)), while the 56-byte peek used to detect "is this a 1xx response" sees "...Continue\r\n..." and classifies it as 1xx based on the next line — so the 1xx-forwarding loop is entered, then panics on the first iteration.

This connection handler runs in a per-connection goroutine with no recover() anywhere in the call chain (app/proxyman/inboundproxy/http), so the panic terminates the whole process, not just the one connection. Any HTTP target reached through the HTTP inbound's plain-HTTP mode that returns a response shaped like this brings down the entire server for all users on it.

Fix

Guard the slice with a length check before comparing:

if len(ResponseHeader1xx) >= 4 && bytes.Equal(ResponseHeader1xx[len(ResponseHeader1xx)-4:], []byte{'\r', '\n', '\r', '\n'}) {

Test plan

Added proxy/http/server_test.go (this package previously had no tests):

  • TestReadResponseAndHandle100ContinueDoesNotPanicOnEarlyNewline — reproduces the crash above. Confirmed it panics against the unpatched code and passes with the fix.
  • TestReadResponseAndHandle100ContinueForwardsAndParsesFinalResponse — sanity check that legitimate 1xx forwarding is unaffected: a real 100 Continue followed by a 200 OK is still forwarded and parsed correctly.
$ go test -v ./proxy/http/...
=== RUN   TestReadResponseAndHandle100ContinueDoesNotPanicOnEarlyNewline
--- PASS: TestReadResponseAndHandle100ContinueDoesNotPanicOnEarlyNewline (0.00s)
=== RUN   TestReadResponseAndHandle100ContinueForwardsAndParsesFinalResponse
--- PASS: TestReadResponseAndHandle100ContinueForwardsAndParsesFinalResponse (0.00s)
PASS

@RPRX

RPRX commented Jul 27, 2026

Copy link
Copy Markdown
Member

Rebase

…d upstream response

ResponseHeader1xx[len(ResponseHeader1xx)-4:] was sliced with no check
that at least 4 bytes had been accumulated. A malformed upstream
response containing a bare '\n' before the real status line (e.g.
sniffed as 1xx from the initial 56-byte peek, but with the actual
first line break arriving after only 1-3 bytes) makes
len(ResponseHeader1xx)-4 negative, causing:

  panic: runtime error: slice bounds out of range [-2:]

This runs in a per-connection goroutine with no recover() in the call
chain, so any target HTTP server reached through the HTTP inbound's
plain-HTTP path can crash the entire process, not just its own
connection.

Guard the slice with a length check, and add regression tests: one
reproducing the crash on unpatched code, and one confirming legitimate
1xx forwarding still parses the final response correctly.
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.

2 participants