Skip to content

sapi/cli: guard Content-Length overflow and enforce post_max_size#22017

Open
iliaal wants to merge 1 commit into
php:masterfrom
iliaal:fix/gh-22003-cli-server-content-length
Open

sapi/cli: guard Content-Length overflow and enforce post_max_size#22017
iliaal wants to merge 1 commit into
php:masterfrom
iliaal:fix/gh-22003-cli-server-content-length

Conversation

@iliaal
Copy link
Copy Markdown
Contributor

@iliaal iliaal commented May 12, 2026

The dev server crashes when Content-Length wraps ssize_t (30+ digit value), or when a legitimately large Content-Length passes pemalloc and aborts the process.

Guard the parser's Content-Length and chunked-size accumulators against SSIZE_MAX, then reject oversize Content-Length in on_headers_complete and reply 413 with the configured post_max_size in the body.

Fixes #22003

The dev server's HTTP parser accumulates Content-Length digits into an
ssize_t without an overflow check; a 30-digit value wraps and the
consumer aborts on pemalloc. Guard the decimal and chunked-size
accumulators against SSIZE_MAX, then reject in on_headers_complete when
the parsed length exceeds post_max_size and reply 413 with the
configured limit in the body.

Fixes phpGH-22003
# ifdef _WIN64
# define SSIZE_MAX _I64_MAX
# else
# define SSIZE_MAX INT_MAX
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.

more or less of the same (i.e. sizeof 4) but LONG_MAX is more accurate I think. Or even PTRDIFF_MAX.

Comment thread sapi/cli/php_cli_server.c

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remote DoS via overflowed Content-Length

2 participants