Skip to content

Fix GH-25093: Phar::webPhar() measures the redirect target by the on-disk path - #25104

Open
dhrupo wants to merge 1 commit into
php:PHP-8.4from
dhrupo:fix-gh25093-webphar-redirect
Open

dhrupo wants to merge 1 commit into
php:PHP-8.4from
dhrupo:fix-gh25093-webphar-redirect

Conversation

@dhrupo

@dhrupo dhrupo commented Oct 31, 2026

Copy link
Copy Markdown

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.

…on-disk path

Before appending the entry to the Location header, webPhar() terminates the
request URI right after the phar's file name. It advanced the pointer by the
whole on-disk path length rather than by the file name length, so the
terminator landed the length of the phar's directory prefix too far.

When the request ends at the file name, that write lands past the end of the
request URI buffer. When the URI is longer, which a rewrite callback that
routes to the index allows, the terminator lands inside the URI and the
redirect keeps part of the requested path.

The rest of the function already measures the file name as
fname_len - (basename - fname).
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.

1 participant