Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 52 additions & 9 deletions sapi/cli/php_cli_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ typedef struct php_cli_server_client {
zend_string *addr_str;
php_http_parser parser;
bool request_read;
bool too_large_post;
zend_string *current_header_name;
zend_string *current_header_value;
enum { HEADER_NONE=0, HEADER_FIELD, HEADER_VALUE } last_header_element;
Expand Down Expand Up @@ -209,6 +210,7 @@ static const php_cli_server_http_response_status_code_pair template_map[] = {
{ 400, "<h1>%s</h1><p>Your browser sent a request that this server could not understand.</p>" },
{ 404, "<h1>%s</h1><p>The requested resource <code class=\"url\">%s</code> was not found on this server.</p>" },
{ 405, "<h1>%s</h1><p>Requested method not allowed.</p>" },
{ 413, "<h1>%s</h1><p>The request body exceeds the configured <code>post_max_size</code> of " ZEND_LONG_FMT " bytes.</p>" },
{ 500, "<h1>%s</h1><p>The server is temporarily unavailable.</p>" },
{ 501, "<h1>%s</h1><p>Request method not supported.</p>" }
};
Expand Down Expand Up @@ -1779,14 +1781,36 @@ static int php_cli_server_client_read_request_on_headers_complete(php_http_parse
break;
}
client->last_header_element = HEADER_NONE;

if (parser->content_length > 0
&& SG(post_max_size) > 0
&& (zend_long) parser->content_length > SG(post_max_size)) {
client->request.protocol_version = parser->http_major * 100 + parser->http_minor;
client->too_large_post = true;
client->request_read = true;
return 2;
}

return 0;
}

static int php_cli_server_client_read_request_on_body(php_http_parser *parser, const char *at, size_t length)
{
php_cli_server_client *client = parser->data;

if (SG(post_max_size) > 0 && client->request.content_len + length > (size_t) SG(post_max_size)) {
client->request.protocol_version = parser->http_major * 100 + parser->http_minor;
client->too_large_post = true;
client->request_read = true;
return 1;
}

if (!client->request.content) {
client->request.content = pemalloc(parser->content_length, 1);
size_t reserve = (size_t) parser->content_length;
if (SG(post_max_size) > 0 && reserve > (size_t) SG(post_max_size)) {
reserve = (size_t) SG(post_max_size);
}
client->request.content = pemalloc(reserve, 1);
client->request.content_len = 0;
}
client->request.content = perealloc(client->request.content, client->request.content_len + length, 1);
Expand Down Expand Up @@ -1866,7 +1890,7 @@ static int php_cli_server_client_read_request(php_cli_server_client *client, cha
}
client->parser.data = client;
nbytes_consumed = php_http_parser_execute(&client->parser, &settings, buf, nbytes_read);
if (nbytes_consumed != (size_t)nbytes_read) {
if (nbytes_consumed != (size_t)nbytes_read && !client->too_large_post) {
if (php_cli_server_log_level >= PHP_CLI_SERVER_LOG_ERROR) {
if ((buf[0] & 0x80) /* SSLv2 */ || buf[0] == 0x16 /* SSLv3/TLSv1 */) {
*errstr = estrdup("Unsupported SSL request");
Expand Down Expand Up @@ -1960,6 +1984,7 @@ static void php_cli_server_client_ctor(php_cli_server_client *client, php_cli_se

php_http_parser_init(&client->parser, PHP_HTTP_REQUEST);
client->request_read = false;
client->too_large_post = false;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too_large_post is only cleared in client_ctor. Works now since each request rebuilds the client, but if keep-alive ever reuses the struct, the flag
lingers and every follow-up request gets a bogus 413. Worth resetting it with the other per-request state, or leaving a note at the declaration

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

request_read, last_header_element, and current_header_* are all initialized only in client_ctor for the same reason: the client is torn down at request end. Keep-alive would need a shared per-request reset covering the lot, not just too_large_post.


client->last_header_element = HEADER_NONE;
client->current_header_name = NULL;
Expand All @@ -1983,10 +2008,16 @@ static void php_cli_server_client_dtor(php_cli_server_client *client) /* {{{ */
pefree(client->addr, 1);
zend_string_release_ex(client->addr_str, /* persistent */ true);

if (client->current_header_name) {
zend_string_release_ex(client->current_header_name, /* persistent */ true);
client->current_header_name = NULL;
}
if (client->current_header_value) {
zend_string_release_ex(client->current_header_value, /* persistent */ true);
client->current_header_value = NULL;
}

if (client->content_sender_initialized) {
/* Headers must be set if we reached the content initialisation */
assert(client->current_header_name == NULL);
assert(client->current_header_value == NULL);
php_cli_server_content_sender_dtor(&client->content_sender);
}
} /* }}} */
Expand Down Expand Up @@ -2038,11 +2069,20 @@ static zend_result php_cli_server_send_error_page(php_cli_server *server, php_cl
php_cli_server_buffer_append(&client->content_sender.buffer, chunk);
}
{
php_cli_server_chunk *chunk = php_cli_server_chunk_heap_new_self_contained(strlen(content_template) + ZSTR_LEN(escaped_request_uri) + 3 + strlen(status_string) + 1);
if (!chunk) {
goto fail;
php_cli_server_chunk *chunk;
if (status == 413) {
chunk = php_cli_server_chunk_heap_new_self_contained(strlen(content_template) + strlen(status_string) + MAX_LENGTH_OF_LONG + 1);
if (!chunk) {
goto fail;
}
snprintf(chunk->data.heap.p, chunk->data.heap.len, content_template, status_string, SG(post_max_size));
} else {
chunk = php_cli_server_chunk_heap_new_self_contained(strlen(content_template) + ZSTR_LEN(escaped_request_uri) + 3 + strlen(status_string) + 1);
if (!chunk) {
goto fail;
}
snprintf(chunk->data.heap.p, chunk->data.heap.len, content_template, status_string, ZSTR_VAL(escaped_request_uri));
}
snprintf(chunk->data.heap.p, chunk->data.heap.len, content_template, status_string, ZSTR_VAL(escaped_request_uri));
chunk->data.heap.len = strlen(chunk->data.heap.p);
php_cli_server_buffer_append(&client->content_sender.buffer, chunk);
}
Expand Down Expand Up @@ -2641,6 +2681,9 @@ static zend_result php_cli_server_recv_event_read_request(php_cli_server *server
if (client->request.request_method == PHP_HTTP_NOT_IMPLEMENTED) {
return php_cli_server_send_error_page(server, client, 501);
}
if (client->too_large_post) {
return php_cli_server_send_error_page(server, client, 413);
}
php_cli_server_poller_remove(&server->poller, POLLIN, client->sock);
return php_cli_server_dispatch(server, client);
case 0:
Expand Down
29 changes: 22 additions & 7 deletions sapi/cli/php_http_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,18 @@
*/
#include <assert.h>
#include <stddef.h>
#include <stdint.h>
#include "php_http_parser.h"


#ifndef MIN
# define MIN(a,b) ((a) < (b) ? (a) : (b))
#endif

#ifndef SSIZE_MAX
# define SSIZE_MAX PTRDIFF_MAX
#endif


#define CALLBACK2(FOR) \
do { \
Expand Down Expand Up @@ -1228,8 +1233,10 @@ size_t php_http_parser_execute (php_http_parser *parser,
case h_content_length:
if (ch == ' ') break;
if (ch < '0' || ch > '9') goto error;
parser->content_length *= 10;
parser->content_length += ch - '0';
if (parser->content_length > (SSIZE_MAX - (ch - '0')) / 10) {
goto error;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point the header name has been copied in php_cli_server_client_read_request_on_header_field() and will leak. We likely need to free current_header_name / current_header_value in php_cli_server_client_dtor().

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Freed both in php_cli_server_client_dtor() and dropped the two asserts, which sat behind content_sender_initialized and so never fired on that teardown. Valgrind on the Content-Length overflow case: 40 bytes definitely lost before, clean after.

}
parser->content_length = parser->content_length * 10 + (ch - '0');
break;

/* Transfer-Encoding: chunked */
Expand Down Expand Up @@ -1384,7 +1391,9 @@ size_t php_http_parser_execute (php_http_parser *parser,

to_read = MIN((size_t)(pe - p), (size_t)parser->content_length);
if (to_read > 0) {
if (settings->on_body) settings->on_body(parser, p, to_read);
if (settings->on_body && 0 != settings->on_body(parser, p, to_read)) {
return (p - data);
}
p += to_read - 1;
parser->content_length -= to_read;
if (parser->content_length == 0) {
Expand All @@ -1398,7 +1407,9 @@ size_t php_http_parser_execute (php_http_parser *parser,
case s_body_identity_eof:
to_read = pe - p;
if (to_read > 0) {
if (settings->on_body) settings->on_body(parser, p, to_read);
if (settings->on_body && 0 != settings->on_body(parser, p, to_read)) {
return (p - data);
}
p += to_read - 1;
}
break;
Expand Down Expand Up @@ -1433,8 +1444,10 @@ size_t php_http_parser_execute (php_http_parser *parser,
goto error;
}

parser->content_length *= 16;
parser->content_length += c;
if (parser->content_length > (SSIZE_MAX - c) / 16) {
goto error;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it's still possible to exceed post_max_size with Transfer-Encoding: chunked, as php_cli_server_client_read_request_on_body() will accumulate chunks without checking the limit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. The headers-complete check reads parser->content_length, which a chunked request never sets to the body size, so the limit now applies as the chunks accumulate in on_body(). That needed a parser change as well: all three on_body call sites discarded the callback's return value. Separately, the body buffer was reserved from the declared chunk size, so 7FFFFFFFFFFFFF followed by five bytes exited the server with "Out of memory" before any limit applied; that reservation is clamped now. Covered in gh22003.phpt.

parser->content_length = parser->content_length * 16 + c;
break;
}

Expand Down Expand Up @@ -1471,7 +1484,9 @@ size_t php_http_parser_execute (php_http_parser *parser,
to_read = MIN((size_t)(pe - p), (size_t)(parser->content_length));

if (to_read > 0) {
if (settings->on_body) settings->on_body(parser, p, to_read);
if (settings->on_body && 0 != settings->on_body(parser, p, to_read)) {
return (p - data);
}
p += to_read - 1;
}

Expand Down
95 changes: 95 additions & 0 deletions sapi/cli/tests/gh22003.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
--TEST--
GH-22003 (CLI server: overflow in Content-Length parser + post_max_size enforcement)
--SKIPIF--
<?php
include "skipif.inc";
?>
--FILE--
<?php
include "php_cli_server.inc";
php_cli_server_start("echo 'OK';", null, ['-d', 'post_max_size=1024']);

$host = PHP_CLI_SERVER_HOSTNAME;

// 1. Content-Length above the configured post_max_size but within ssize_t:
// consumer must reject with a 413 page before allocating a body buffer.
$fp = php_cli_server_connect();
fwrite($fp, "POST / HTTP/1.1\r\nHost: $host\r\nContent-Length: 999999\r\nConnection: close\r\n\r\n");
$response = stream_get_contents($fp);
fclose($fp);
echo "over post_max_size: ", str_contains($response, "413 Request Entity Too Large") ? "413" : "FAIL", "\n";
echo "shows configured limit: ", str_contains($response, "1024 bytes") ? "yes" : "no", "\n";

// 2. Same case but with body bytes piggybacked in the same write. The parser sees
// the body bytes after on_headers_complete bails; without a guard in the
// read-request error path the response would be 400 instead of 413.
$fp = php_cli_server_connect();
fwrite($fp, "POST / HTTP/1.1\r\nHost: $host\r\nContent-Length: 999999\r\nConnection: close\r\n\r\n0123456789");
$response = stream_get_contents($fp);
fclose($fp);
echo "over limit with body bytes: ", str_contains($response, "413 Request Entity Too Large") ? "413" : "FAIL", "\n";

// 3. Content-Length wide enough to overflow ssize_t accumulation in the parser:
// parser-level guard rejects as a malformed request before headers-complete fires.
$fp = php_cli_server_connect();
fwrite($fp, "POST / HTTP/1.1\r\nHost: $host\r\nContent-Length: 999999999999999999999999999999\r\nConnection: close\r\n\r\n");
$response = stream_get_contents($fp);
fclose($fp);
echo "content-length overflow: ", str_contains($response, "200 OK") ? "FAIL" : "rejected", "\n";

// 4. Transfer-Encoding: chunked with an oversized hex chunk size: same parser guard
// on the chunked accumulator must reject without aborting the server.
$fp = php_cli_server_connect();
fwrite($fp, "POST / HTTP/1.1\r\nHost: $host\r\nTransfer-Encoding: chunked\r\nConnection: close\r\n\r\n"
. str_repeat("F", 32) . "\r\n");
$response = stream_get_contents($fp);
fclose($fp);
echo "chunked overflow: ", str_contains($response, "200 OK") ? "FAIL" : "rejected", "\n";

// 5. Chunked body whose accumulated size exceeds post_max_size. No Content-Length
// header exists, so the limit can only be enforced as the chunks arrive.
$fp = php_cli_server_connect();
$request = "POST / HTTP/1.1\r\nHost: $host\r\nTransfer-Encoding: chunked\r\nConnection: close\r\n\r\n";
for ($i = 0; $i < 4; $i++) {
$request .= sprintf("%x\r\n%s\r\n", 1000, str_repeat("A", 1000));
}
fwrite($fp, $request . "0\r\n\r\n");
$response = stream_get_contents($fp);
fclose($fp);
echo "chunked over post_max_size: ", str_contains($response, "413 Request Entity Too Large") ? "413" : "FAIL", "\n";

// 6. Chunk size under the ssize_t guard but far above post_max_size, with only a few
// body bytes behind it: the buffer must not be reserved from the declared size.
$fp = php_cli_server_connect();
fwrite($fp, "POST / HTTP/1.1\r\nHost: $host\r\nTransfer-Encoding: chunked\r\nConnection: close\r\n\r\n"
. "7FFFFFFFFFFFFF\r\nHELLO");
stream_socket_shutdown($fp, STREAM_SHUT_WR);
$response = stream_get_contents($fp);
fclose($fp);
echo "oversize chunk size: ", str_contains($response, "200 OK") ? "FAIL" : "rejected", "\n";

// 7. A chunked body within the limit still reaches the script.
$fp = php_cli_server_connect();
fwrite($fp, "POST / HTTP/1.1\r\nHost: $host\r\nTransfer-Encoding: chunked\r\nConnection: close\r\n\r\n"
. sprintf("%x\r\n%s\r\n", 100, str_repeat("B", 100)) . "0\r\n\r\n");
$response = stream_get_contents($fp);
fclose($fp);
echo "chunked within limit: ", str_contains($response, "200 OK") ? "200 OK" : "FAIL", "\n";

// 8. Server must still be alive and serving normal requests.
$fp = php_cli_server_connect();
fwrite($fp, "GET / HTTP/1.1\r\nHost: $host\r\nConnection: close\r\n\r\n");
$response = stream_get_contents($fp);
fclose($fp);
echo "follow-up: ", str_contains($response, "200 OK") ? "200 OK" : "FAILED", "\n";
?>
--EXPECT--
over post_max_size: 413
shows configured limit: yes
over limit with body bytes: 413
content-length overflow: rejected
chunked overflow: rejected
chunked over post_max_size: 413
oversize chunk size: rejected
chunked within limit: 200 OK
follow-up: 200 OK
Loading