From 983769f8c5ae71cd99ff1347730db55737de9875 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Fri, 29 May 2026 19:18:32 +0000 Subject: [PATCH] http1: reject fragment/backslash in target + duplicate Content-Type (#47) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Optional RFC hardening of the request-target and singleton headers — all were Warn in Http11Probe (200 accepted) but stricter rejection closes real path-confusion / framing-ambiguity vectors: - raw '#' in the request-target (fragment, never sent per RFC 9112 §3.2) → 400 - raw '\' in the request-target (not a URI char, RFC 3986; '\' vs '/' parsed inconsistently) → 400 - duplicate Content-Type (RFC 9110 §8.3 singleton) → 400 (new ct_seen_count) Percent-encoded %23 / %5C stay legal. New h1/021. Re-ran the WHOLE tests/phpt (202/202) — lesson from the version→505 misfire: validate the full set after a parser change, not one case. Multi-space request-line is NOT done: llhttp consumes the spaces before our callbacks, so it's not detectable post-parse without llhttp leniency changes. --- include/http1/http_parser.h | 1 + src/http1/http_parser.c | 25 ++++++ .../h1/021-request-target-hardening.phpt | 78 +++++++++++++++++++ 3 files changed, 104 insertions(+) create mode 100644 tests/phpt/server/h1/021-request-target-hardening.phpt diff --git a/include/http1/http_parser.h b/include/http1/http_parser.h index 466b047..6ed51dc 100644 --- a/include/http1/http_parser.h +++ b/include/http1/http_parser.h @@ -278,6 +278,7 @@ typedef struct http1_parser_t { uint16_t cl_seen_count; /* Number of Content-Length headers seen */ uint16_t header_count; /* Number of header fields parsed (DoS cap) */ uint16_t host_seen_count; /* Number of Host headers seen (RFC 9112 §3.2 — exactly one) */ + uint16_t ct_seen_count; /* Number of Content-Type headers seen (RFC 9110 §8.3 — singleton) */ bool te_chunked_seen; /* Transfer-Encoding: chunked seen */ /* Boolean flags (clustered to avoid interior padding) */ diff --git a/src/http1/http_parser.c b/src/http1/http_parser.c index d7861bb..e63b75c 100644 --- a/src/http1/http_parser.c +++ b/src/http1/http_parser.c @@ -224,6 +224,7 @@ static int on_message_begin(llhttp_t* llhttp_parser) parser->cl_seen_count = 0; parser->header_count = 0; parser->host_seen_count = 0; + parser->ct_seen_count = 0; parser->te_chunked_seen = false; return 0; @@ -433,6 +434,15 @@ static void save_current_header(http1_parser_t *parser) parser->parse_error = HTTP_PARSE_ERR_INVALID_HOST; return; } + } else if (zend_string_equals_literal(name, "content-type")) { + /* RFC 9110 §8.3: Content-Type is a singleton. Two of them is + * ambiguous (which one frames the body?) — reject rather than + * comma-combine into a malformed media type. */ + parser->ct_seen_count++; + if (UNEXPECTED(parser->ct_seen_count > 1)) { + parser->parse_error = HTTP_PARSE_ERR_MALFORMED; + return; + } } else if (zend_string_equals_literal(name, "connection")) { if (strncasecmp(ZSTR_VAL(value), "keep-alive", 10) == 0) { req->keep_alive = true; @@ -572,6 +582,21 @@ static int on_headers_complete(llhttp_t* llhttp_parser) parser->uri_builder.s = NULL; /* Transfer ownership */ } + /* Reject a request-target carrying a raw fragment ('#', RFC 9112 §3.2 — + * fragments are never sent on the wire) or a backslash (not valid in a + * URI per RFC 3986; '\' vs '/' is parsed inconsistently and enables + * path-confusion). Percent-encoded forms (%23 / %5C) are untouched. */ + if (req->uri != NULL) { + const char *u = ZSTR_VAL(req->uri); + const size_t ulen = ZSTR_LEN(req->uri); + for (size_t i = 0; i < ulen; i++) { + if (UNEXPECTED(u[i] == '#' || u[i] == '\\')) { + parser->parse_error = HTTP_PARSE_ERR_MALFORMED; + return -1; + } + } + } + /* Get HTTP version */ req->http_major = llhttp_parser->http_major; req->http_minor = llhttp_parser->http_minor; diff --git a/tests/phpt/server/h1/021-request-target-hardening.phpt b/tests/phpt/server/h1/021-request-target-hardening.phpt new file mode 100644 index 0000000..7793db3 --- /dev/null +++ b/tests/phpt/server/h1/021-request-target-hardening.phpt @@ -0,0 +1,78 @@ +--TEST-- +HttpServer: request-target + singleton-header hardening (fragment, backslash, dup Content-Type) +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + +--FILE-- +addListener('127.0.0.1', $port)->setReadTimeout(5)->setWriteTimeout(5)); +$server->addHttpHandler(function ($req, $res) { + $res->setStatusCode(200)->setBody('ok'); +}); + +function probe(int $port, string $raw): int { + $fp = false; + for ($t = 0; $t < 20 && $fp === false; $t++) { + $fp = @stream_socket_client("tcp://127.0.0.1:$port", $e, $es, 2); + if ($fp === false) { usleep(10000); } + } + if ($fp === false) { return -1; } + fwrite($fp, $raw); + stream_set_timeout($fp, 2); + $r = ''; + while (!feof($fp)) { + $c = fread($fp, 4096); + if ($c === '' || $c === false) break; + $r .= $c; + if (str_contains($r, "\r\n")) break; + } + fclose($fp); + return preg_match('#^HTTP/1\.[01] (\d{3})#', $r, $m) ? (int)$m[1] : 0; +} + +$client = spawn(function () use ($port, $server) { + usleep(20000); + $h = "Host: a.com\r\nConnection: close\r\n\r\n"; + echo "fragment=", probe($port, "GET /p#frag HTTP/1.1\r\n$h"), "\n"; + echo "backslash=", probe($port, "GET /a\\b HTTP/1.1\r\n$h"), "\n"; + echo "dup_ct=", probe($port, + "POST / HTTP/1.1\r\nHost: a.com\r\nContent-Type: text/a\r\n" + . "Content-Type: text/b\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"), "\n"; + echo "encoded_hash=", probe($port, "GET /p%23frag HTTP/1.1\r\n$h"), "\n"; + echo "normal=", probe($port, "GET /ok?x=1 HTTP/1.1\r\n$h"), "\n"; + echo "single_ct=", probe($port, + "POST / HTTP/1.1\r\nHost: a.com\r\nContent-Type: text/a\r\n" + . "Content-Length: 0\r\nConnection: close\r\n\r\n"), "\n"; + $server->stop(); +}); + +$server->start(); +await($client); +echo "done\n"; +--EXPECT-- +fragment=400 +backslash=400 +dup_ct=400 +encoded_hash=200 +normal=200 +single_ct=200 +done