Skip to content

fix: reject request headers containing CR or LF - #275

Open
hamodywe wants to merge 1 commit into
supabase:masterfrom
hamodywe:fix/reject-crlf-in-request-headers
Open

fix: reject request headers containing CR or LF#275
hamodywe wants to merge 1 commit into
supabase:masterfrom
hamodywe:fix/reject-crlf-in-request-headers

Conversation

@hamodywe

Copy link
Copy Markdown

Closes #274.

Header values went straight to curl_slist_append with no validation. libcurl appends the CRLF terminator itself, so a value carrying its own CR or LF closes the header early and everything after it is read by the receiving parser as further headers or as the start of the body. A value interpolated from untrusted input can therefore inject headers into the outgoing request, or split it.

This rejects any header containing CR or LF before it reaches libcurl, so a poisoned request fails instead of being sent. The check is in pg_text_array_to_slist rather than in the SQL API so it also covers rows inserted directly into net.http_request_queue, and it uses the same ereport(ERROR, ...) style already used in that function.

Only the header name is included in the error. The value is the attacker-controlled part and commonly holds credentials, so it must not be copied into the server log; when the CR or LF appears before any :, the reported name is truncated at that point so the smuggled text cannot reach the log that way either.

Verification

  • Builds clean in-tree against PostgreSQL 17 (postgresql-server-dev-17, libcurl4-openssl-dev), including the repo's -Werror -Wextra -Wall settings.
  • The matching logic was exercised standalone over 11 cases: ordinary headers (including X-Empty: and a header with no colon) are unaffected; LF, CR, CRLF, trailing-LF and leading-LF are all rejected; and the no-colon-before-CRLF case confirms the injected text never appears in the error.
  • test/test_http_header_crlf.py adds regression tests in the style of test_http_malformed_headers.py, including one asserting ordinary headers are still sent. I was not able to run the Nix net-with-nginx harness locally, so those tests are unrun on my side — worth a look in CI.

Header values went straight to curl_slist_append with no validation.
libcurl appends the CRLF terminator itself, so a value carrying its own
CR or LF closes the header early and everything after it is read by the
receiving parser as further headers or as the start of the body. A value
interpolated from untrusted input can therefore inject headers into the
outgoing request, or split it.

Reject any header containing CR or LF before it reaches libcurl, so a
poisoned request fails instead of being sent.

Only the header name is included in the error. The value is the
attacker-controlled part and commonly holds credentials, so it must not
be copied into the server log; when the CR or LF appears before any ':'
the reported name is truncated at that point, so the smuggled text
cannot reach the log that way either.
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.

pg_net doesn't reject headers with \r or \n in them

1 participant