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

Comment thread sapi/cli/php_http_parser.c Outdated
# 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.

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.

Switched to PTRDIFF_MAX, drops the _WIN64 split.

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

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.

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
@iliaal iliaal force-pushed the fix/gh-22003-cli-server-content-length branch from 612aa33 to 3827cd3 Compare May 12, 2026 12:15
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