Commit 795440a704d for php.net
commit 795440a704d4c2523c43666aef43c6a0fa193fee
Author: Jakub Zelenka <bukka@php.net>
Date: Tue Sep 22 11:35:38 2026 +0200
Fix GHSA-fpwc-w8rq-cr92: do not cut the request short when stripping the last user header
Port the line-aware strip_header() from the PHP-8.2 fix. When the stripped
header was the last line of the user header bag, the previous implementation
left the line break in front of it behind, and the CRLF appended after the bag
then ended the header block early. With Authorization, Cookie or
Proxy-Authorization now being stripped on cross-origin redirects, this could
hit a body-preserving 307/308 POST: the target received a header block cut off
after the preceding header and the wrong bytes as the body.
The ported version also removes folded continuation lines of the stripped
header and tolerates whitespace before the colon, so a header written as
"Authorization : ..." cannot slip through.
diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c
index b7a05e2942d..4e6317a079a 100644
--- a/ext/standard/http_fopen_wrapper.c
+++ b/ext/standard/http_fopen_wrapper.c
@@ -85,34 +85,68 @@
#define HTTP_WRAPPER_KEEP_METHOD 4
#define HTTP_WRAPPER_STRIP_AUTH 8
-/* Removes every line whose header name matches. Neither a repeated header nor an
- * occurrence of the name inside another header's value may leave the real header
- * behind, as that would defeat HTTP_WRAPPER_STRIP_AUTH. */
+static char *next_header_line(char *line)
+{
+ while (*line != '\0' && *line != '\r' && *line != '\n') {
+ line++;
+ }
+ if (*line == '\r') {
+ line++;
+ }
+ if (*line == '\n') {
+ line++;
+ }
+
+ return line;
+}
+
+/* Removes every line whose header name matches, along with the folded
+ * continuation lines carrying the rest of its value. Neither a repeated header
+ * nor an occurrence of the name inside another header's value may leave the real
+ * header behind, as that would defeat HTTP_WRAPPER_STRIP_AUTH. */
static inline void strip_header(char *header_bag, char *lc_header_bag,
const char *lc_header_name)
{
- char *lc_header_start = lc_header_bag;
+ size_t name_len = strlen(lc_header_name);
+ char *lc_line = lc_header_bag;
- while ((lc_header_start = strstr(lc_header_start, lc_header_name))) {
- if (lc_header_start != lc_header_bag && *(lc_header_start-1) != '\n') {
- lc_header_start += strlen(lc_header_name);
+ while (*lc_line != '\0') {
+ if (strncmp(lc_line, lc_header_name, name_len) != 0) {
+ lc_line = next_header_line(lc_line);
continue;
}
- char *header_start = header_bag + (lc_header_start - lc_header_bag);
- char *lc_eol = strchr(lc_header_start, '\n');
+ /* the whitespace RFC 7230 forbids before the colon is tolerated by some
+ * servers, so it must not hide the header from us either */
+ const char *lc_colon = lc_line + name_len;
+ while (*lc_colon == ' ' || *lc_colon == '\t') {
+ lc_colon++;
+ }
+
+ if (*lc_colon != ':') {
+ lc_line = next_header_line(lc_line);
+ continue;
+ }
- if (!lc_eol) {
- *lc_header_start = '\0';
- *header_start = '\0';
- return;
+ char *lc_next = next_header_line(lc_line);
+ while (*lc_next == ' ' || *lc_next == '\t') {
+ lc_next = next_header_line(lc_next);
+ }
+
+ if (*lc_next == '\0') {
+ /* drop the preceding line break too, or the one appended after the bag
+ * would close the header block early */
+ while (lc_line > lc_header_bag
+ && (*(lc_line - 1) == '\r' || *(lc_line - 1) == '\n')) {
+ --lc_line;
+ }
}
- char *eol = header_start + (lc_eol - lc_header_start);
- size_t eollen = strlen(lc_eol);
+ size_t tail_len = strlen(lc_next) + 1;
+ char *line = header_bag + (lc_line - lc_header_bag);
- memmove(lc_header_start, lc_eol+1, eollen);
- memmove(header_start, eol+1, eollen);
+ memmove(line, header_bag + (lc_next - lc_header_bag), tail_len);
+ memmove(lc_line, lc_next, tail_len);
}
}
@@ -709,15 +743,15 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
if (!header_init && !redirect_keep_method) {
/* strip POST headers on redirect */
- strip_header(user_headers, t, "content-length:");
- strip_header(user_headers, t, "content-type:");
+ strip_header(user_headers, t, "content-length");
+ strip_header(user_headers, t, "content-type");
}
if (flags & HTTP_WRAPPER_STRIP_AUTH) {
- strip_header(user_headers, t, "authorization:");
- strip_header(user_headers, t, "cookie:");
+ strip_header(user_headers, t, "authorization");
+ strip_header(user_headers, t, "cookie");
if (!use_proxy) {
- strip_header(user_headers, t, "proxy-authorization:");
+ strip_header(user_headers, t, "proxy-authorization");
}
}
diff --git a/ext/standard/tests/http/bug61548.phpt b/ext/standard/tests/http/bug61548.phpt
index 5f21b3769dd..ba46e65704f 100644
--- a/ext/standard/tests/http/bug61548.phpt
+++ b/ext/standard/tests/http/bug61548.phpt
@@ -55,7 +55,6 @@ function do_test($header) {
First:1
Second:2
-
POST / HTTP/1.1
Host: %s:%d
Connection: close
@@ -69,7 +68,6 @@ function do_test($header) {
First:1
Second:2
-
POST / HTTP/1.1
Host: %s:%d
Connection: close
diff --git a/ext/standard/tests/http/ghsa-fpwc-w8rq-cr92-002.phpt b/ext/standard/tests/http/ghsa-fpwc-w8rq-cr92-002.phpt
new file mode 100644
index 00000000000..c314a24ea2e
--- /dev/null
+++ b/ext/standard/tests/http/ghsa-fpwc-w8rq-cr92-002.phpt
@@ -0,0 +1,61 @@
+--TEST--
+GHSA-fpwc-w8rq-cr92: stripping the last user header must not cut the redirected request short
+--INI--
+allow_url_fopen=1
+--SKIPIF--
+<?php require 'server.inc'; http_server_skipif(); ?>
+--FILE--
+<?php
+require 'server.inc';
+
+/* Authorization is the last user header and has no trailing line break. When it
+ * is stripped on the cross-origin hop, the line break in front of it must go too,
+ * otherwise the header block ends early and the body of the 307 POST is lost. */
+$ctx = stream_context_create(['http' => [
+ 'method' => 'POST',
+ 'header' => "X-Test: 1\r\nContent-Type: text/plain\r\nAuthorization: Basic Zm9vOmJhcg==",
+ 'content' => 'hello=world',
+ 'follow_location' => 1,
+]]);
+
+$captureB = null;
+['pid' => $pidB, 'uri' => $uriB] = http_server([
+ "data://text/plain,HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nOK",
+], $captureB);
+
+$captureA = null;
+['pid' => $pidA, 'uri' => $uriA] = http_server([
+ "data://text/plain,HTTP/1.1 307 Temporary Redirect\r\nLocation: $uriB/target\r\nContent-Length: 0\r\n\r\n",
+], $captureA);
+
+var_dump(file_get_contents($uriA . '/start', false, $ctx));
+
+http_server_kill($pidA);
+http_server_kill($pidB);
+
+rewind($captureA);
+echo "--- origin A ---\n", stream_get_contents($captureA), "\n";
+rewind($captureB);
+echo "--- origin B ---\n", stream_get_contents($captureB), "\n";
+?>
+--EXPECTF--
+string(2) "OK"
+--- origin A ---
+POST /start HTTP/1.1
+Host: %s:%d
+Connection: close
+Content-Length: 11
+X-Test: 1
+Content-Type: text/plain
+Authorization: Basic Zm9vOmJhcg==
+
+hello=world
+--- origin B ---
+POST /target HTTP/1.1
+Host: %s:%d
+Connection: close
+Content-Length: 11
+X-Test: 1
+Content-Type: text/plain
+
+hello=world