[php-src] PR #25104: Fix GH-25093: Phar::webPhar() measures the redirect target by the on-disk path
| From: | dhrupo | Date: | Sat, 31 Oct 2026 18:09:28 +0000 |
| Subject: | [php-src] PR #25104: Fix GH-25093: Phar::webPhar() measures the redirect target by the on-disk path | ||
| Groups: | php.git-pulls | ||
| Request: | Send a blank email to git-pulls+get-40153@lists.php.net to get a copy of this message | ||
Pull Request: https://github.com/php/php-src/pull/25104
Author: dhrupo
Drafted with an LLM; I reviewed it myself.
> Fixes GH-25093.
>
> Before it appends the entry to the
Location header,
> webPhar() terminates the request URI right after the phar's
> file name. The pointer advanced by fname_len, the length of the
> phar's path on disk, instead of by the file name's length, so the terminator landed the
> length of the phar's directory prefix too far. The same function already measures the file name
> as fname_len - (basename - fname) in the three places where it
> builds pt and entry.
>
> Two effects, both with a phar in a subdirectory, under the cli-server SAPI:
>
> * A request ending at the phar (GET /sub/app.phar): the write
> lands past the end of the request URI buffer. That is the out-of-bounds access in the report. The
> Location header is unaffected, so observing it needs a sanitizer,
> and whether ASan reports it depends on the heap layout.
> * A longer request URI: the terminator lands inside the URI and the redirect keeps part of the
> requested path. The test drives this with a rewrite callback that routes to the index, so the
> redirect still fires while the URI is long. Before: Location:
> /sub/app.phar/aaa…/index.php. After: Location:
> /sub/app.phar/index.php.
>
> The test asserts the second effect, so it fails on an ordinary build rather than only under a
> sanitizer, and it also checks the redirect for the request in the report.
>
> Tested on PHP-8.4: with an ASan build,
> ext/phar/tests 457 passed / 0 failed (119 skipped, no zlib or bz2 in
> that build) and sapi/cli/tests 96 passed / 0 failed. The new test
> also passes on an --enable-phar=shared build.