Skip to content

ext/phar: Fix double-free in webPhar() without PATH_INFO - #24166

Closed
bukka wants to merge 1 commit into
php:PHP-8.6from
bukka:phar_fix_webphar_double_free_no_path_info
Closed

bukka wants to merge 1 commit into
php:PHP-8.6from
bukka:phar_fix_webphar_double_free_no_path_info

Conversation

@bukka

@bukka bukka commented Oct 6, 2026

Copy link
Copy Markdown
Member

In the CGI/FastCGI branch of webPhar(), when SCRIPT_NAME is present but PATH_INFO is absent, path_info was aliased to the testit buffer and free_pathinfo was set. Since commit 3ee2f44 added an unconditional efree(testit) after that branch, path_info became a dangling pointer. This causes a use-after-free when path_info is read later and a double-free at the cleanup_skip_entry label where free_pathinfo triggers efree(path_info).

This only affects PHP-8.6 as earlier branches do not free testit there.

Allocate a dedicated copy for path_info so its lifetime outlives the efree(testit).

Reported by RigelYoung.

@bukka
bukka requested a review from LamentXU123 as a code owner October 6, 2026 16:45
@bukka
bukka changed the base branch from master to PHP-8.6 October 6, 2026 16:45
@bukka bukka closed this Oct 6, 2026
@bukka bukka reopened this Oct 6, 2026
In the CGI/FastCGI branch of webPhar(), when SCRIPT_NAME is present but
PATH_INFO is absent, path_info was aliased to the testit buffer and
free_pathinfo was set. Since commit 3ee2f44 added an unconditional
efree(testit) after that branch, path_info became a dangling pointer.
This causes a use-after-free when path_info is read later and a
double-free at the cleanup_skip_entry label where free_pathinfo triggers
efree(path_info).

This only affects PHP-8.6 as earlier branches do not free testit there.

Allocate a dedicated copy for path_info so its lifetime outlives the
efree(testit).

Reported by RigelYoung.

Closes phpGH-24166
@bukka
bukka force-pushed the phar_fix_webphar_double_free_no_path_info branch from 8675718 to 569cf59 Compare October 6, 2026 16:46

@LamentXU123 LamentXU123 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 will check this stuff again before I merge this but seems ok.

@LamentXU123 LamentXU123 changed the title phar: Fix double-free in webPhar() without PATH_INFO ext/phar: Fix double-free in webPhar() without PATH_INFO Oct 6, 2026
LamentXU123 added a commit that referenced this pull request Oct 6, 2026
* PHP-8.6:
  ext/phar: Fix double-free in webPhar() without PATH_INFO (#24166)
@LamentXU123

Copy link
Copy Markdown
Member

Thanks!

bukka added a commit to bukka/php-src that referenced this pull request Oct 7, 2026
* master: (109 commits)
  ext/uri: Fixes inconsistent validation ordering (php#23823)
  ext/uri: Address UrlBuilder todo comments
  Fix phpGH-24139: NULL dereference in php_ini.c when expand_filepath() fails
  PHP-8.5 is now for PHP 8.5.13-dev
  Fix phpGH-23352: DOMDocument::adoptNode() stale document references
  Fix merge
  ext/soap: fix use of uninitialized func in do_request() on OOM bailout
  Fix merge
  ext/curl: Use try conversion functions in curl (php#24067)
  ext/standard: Retry getpwnam_r() on ERANGE in php_get_uid_by_name()
  Fix phpGH-20890: Segfault in zval_undefined_cv with non-simple property hook with minimal tracing JIT
  PHP 8.4 is now for PHP 8.4.28-dev
  ext/phar: Fix double-free in webPhar() without PATH_INFO (php#24166)
  [ci skip] Update NEWS for 8.6.0RC4
  std: refactor php_array_find() to only use FCC
  std: add trampoline tests for array_find based user functions
  std: move some array find tests to a dedicated folder
  Fix alternate form flag for %x testing stale signed value
  Fix bzopen() ownership of the stream it wraps
  Preserve bare NUL access with open_basedir on Windows (php#24158)
  ...

# Conflicts:
#	ext/openssl/tests/stream_poll_handle_cast.phpt
#	ext/openssl/xp_ssl.c
#	main/php_streams.h
#	main/streams/plain_wrapper.c
#	main/streams/userspace.c
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.

2 participants