Skip to content

fix: avoid out-of-bounds read when parsing the response status line#2464

Merged
alexandre-daubois merged 1 commit into
php:mainfrom
iliaal:fix/send-headers-status-line-oob
Jun 26, 2026
Merged

fix: avoid out-of-bounds read when parsing the response status line#2464
alexandre-daubois merged 1 commit into
php:mainfrom
iliaal:fix/send-headers-status-line-oob

Conversation

@iliaal

@iliaal iliaal commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

frankenphp_send_headers parsed the status with atoi(http_status_line + 9), assuming a fixed 9-byte "HTTP/x.y " prefix. header("HTTP/") (5 bytes) makes +9 read past the buffer. ASan confirms a heap-buffer-overflow read in atoi, 3 bytes past the estrndup'd line.

PHP already parses the code into http_response_code (via sapi_update_response_code()), so the reparse was redundant and unsafe. Use it directly. Tests in statusline_test.go (run under -asan) cover the short line and a valid HTTP/1.1 418 line.

frankenphp_send_headers parsed the status code with
atoi(SG(sapi_headers).http_status_line + 9), assuming a fixed
"HTTP/x.y " 9-byte prefix. A script setting a shorter status line, e.g.
header("HTTP/"), leaves http_status_line only 5 bytes long, so the +9
offset reads past the buffer. Confirmed with AddressSanitizer
(heap-buffer-overflow read in atoi, 3 bytes past a 6-byte estrndup'd
region) via an embed build with USE_ZEND_ALLOC=0.

PHP already parses the code into http_response_code through
sapi_update_response_code() whenever a status line is set, so the
reparse was redundant as well as unsafe. Use http_response_code
directly.
@alexandre-daubois alexandre-daubois force-pushed the fix/send-headers-status-line-oob branch from 95cf395 to f8f78ed Compare June 26, 2026 07:23
@alexandre-daubois alexandre-daubois merged commit ccb268d into php:main Jun 26, 2026
65 of 74 checks passed
@alexandre-daubois

Copy link
Copy Markdown
Member

Thank you @iliaal!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants