Set register_argc_argv to Off by default

[thomas-chauchefoin-sonarsource]

(I'm not reporting this as a security issue as it's about a setting "[...] not recommended for production - ex. error reporting to output" or "[...] known to be insecure".)

Many PHP CLI tools are shipped in the form of Phar files (e.g. Composer), and while never really recommended, some users tend to put these archives under the web root (tutorials from shared hosting providers, when you need per-project Composer releases, etc.). On distributions like Debian and Ubuntu, Apache is treating these files as PHP scripts.

Since these scripts use $_SERVER['argc'] to find out how they are invoked and parse their arguments, direct access to these files is not a problem when register_argc_argv is set to Off. Looking into this topic, I noticed that PHP is still shipped with register_argc_argv set to On by default:

php-src/main/main.c

Line 709 in 21d9fd3

STD_PHP_INI_BOOLEAN("register_argc_argv", "1", PHP_INI_PERDIR|PHP_INI_SYSTEM, OnUpdateBool, register_argc_argv, php_core_globals, core_globals)

The recommended default configuration for the production environment and shipped with most distributions set it to Off–note that it only mentions performance reasons and not security:

php-src/php.ini-production

Lines 677 to 690 in 21d9fd3

	; This directive determines whether PHP registers $argv & $argc each time it
	; runs. $argv contains an array of all the arguments passed to PHP when a script
	; is invoked. $argc contains an integer representing the number of arguments
	; that were passed when the script was invoked. These arrays are extremely
	; useful when running scripts from the command line. When this directive is
	; enabled, registering these variables consumes CPU cycles and memory each time
	; a script is executed. For performance reasons, this feature should be disabled
	; on production servers.
	; Note: This directive is hardcoded to On for the CLI SAPI
	; Default Value: On
	; Development Value: Off
	; Production Value: Off
	; https://php.net/register-argc-argv
	register_argc_argv = Off

There are still environments in which this setting can be set to On, either involuntarily by keeping the development configuration or voluntarily by manually setting it. For instance, the main PHP Docker image for PHP has it set to On.

We can then assume that there is a non-zero chance of deployments processing Phar files as PHP scripts and with this setting left to its default value, introducing potential vulnerabilities. I've already reached out to Composer and they now refuse to run in non-CLI SAPIs if register_argc_argv is On (CVE-2023-43655).

Outside of the risk caused by Phar files, register_argc_argv is also a known "trick" to exploit limited Local File Inclusion vulnerabilities in a generic way in Docker php images, using /usr/local/lib/php/pearcmd.php (i.e. 2linephp by @w181496 during Balsn CTF 2021). This exploitation method was also shared with a wider audience in a video of @JohnHammond (https://www.youtube.com/watch?v=yq2rq50IMSQ).

I think it would be great to set register_argc_argv to Off by default, keeping it to On only for these SAPIs: embed, phpdbg and cli. I'm not sure about litespeed but from what I'm reading in the code, it seems important too. The documentation in php.ini could also mention the potential security risks caused by this setting.

I'll be happy to work on the PR if this sounds like something that could happen to be merged, let me know!

[iluuu1994]

register_argc_argv is already forced to true for CLI:

php-src/sapi/cli/php_cli.c

Line 131 in 94c1e55

"register_argc_argv=1\n"

Disabling the setting by default for other SAPIs is reasonable. There's little reason to use it in a non-CLI environment given that GET params are available in $_GET. I think a short e-mail to the internals mailing list would be warranted, with the suggestion to disable the config in PHP 8.4 by default.

Would you like to do that? If you're not interested I can send the e-mail for you.

[thomas-chauchefoin-sonarsource]

Thank you for your initial feedback! I'll send an email and reference this issue.

[thomas-chauchefoin-sonarsource]

For reference: https://news-web.php.net/php.internals/121603.

[nicolas-grekas]

Definitely 👍 here: this setting is dangerous, it should be off on non-CLI SAPIs.

[nicolas-grekas]

The thread on internals basically says: go ahead; https://externals.io/message/121603

[nicolas-grekas]

For the record, this behavior is the root of cve-2024-50340 in Symfony:
https://symfony.com/blog/cve-2024-50340-ability-to-change-environment-from-query

Changing the default value would definitely help. The better move would be to just remove this setting altogether, it serves no purpose nowadays.

[nicolas-grekas]

register_argc_argv is hardcoded to On when using the CLI, which is legit. What you want is checking what happens when PHP is used to serve HTTP.

[Girgias]

If the INI setting serves no purpose, please add it to https://wiki.php.net/rfc/deprecations_php_8_5 so that it is not forgotten

[nicolas-grekas]

Thanks for the hint, RFC updated.