Skip to content

Fix GH-24509: HTTP wrapper leaks Location buffer on empty Location header - #24512

Closed
bercedev wants to merge 1 commit into
php:PHP-8.4from
bercedev:fix/gh24509-location-leak
Closed

bercedev wants to merge 1 commit into
php:PHP-8.4from
bercedev:fix/gh24509-location-leak

Conversation

@bercedev

Copy link
Copy Markdown
Contributor

Fixes GH-24509

php_stream_http_response_headers_parse() used location_len == 0 to mean "no Location buffer allocated yet". An empty Location: header allocates a 1-byte buffer and leaves location_len at 0, so a later Location: header took the emalloc() branch again and leaked the first buffer. It now checks header_info->location == NULL. The struct is zeroed by php_stream_http_response_header_info_init(), and the pointer is set back to NULL wherever the buffer is handed off or freed.

The new ext/standard/tests/http/gh24509.phpt sends Location: followed by Location: /abcdefghijklmno, then follows the redirect. On a debug build (--enable-debug --enable-pcntl --enable-posix) of PHP-8.4 without the fix it fails with:

ext/standard/http_fopen_wrapper.c(339) :  Freeing 0x... (16 bytes), script=.../gh24509.php
=== Total 1 memory leaks detected ===

With the fix it passes, and ext/standard/tests/http/ gives 71 passed, 14 skipped, 0 failed.

I targeted PHP-8.4 because the same code is there.

… header

location_len == 0 was used to mean that no buffer was allocated yet, but an empty Location header allocates a 1-byte buffer and leaves location_len at 0. A second Location header then allocated a new buffer and leaked the first. Check the pointer instead.

@ndossche ndossche left a comment

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 this is right, thanks

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.

HTTP wrapper leaks Location buffer when an empty Location header is followed by another Location header

2 participants