iluuu1994
commented
10 months ago register_argc_argv is already forced to true for CLI:
https://github.com/php/php-src/blob/94c1e559f9d35eec768fabebc763164850d41843/sapi/cli/php_cli.c#L131
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
(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 whenregister_argc_argvis set toOff. Looking into this topic, I noticed that PHP is still shipped withregister_argc_argvset toOnby default:https://github.com/php/php-src/blob/21d9fd3bc1ccf376438e4b5c38bb1945ae3bfe8c/main/main.c#L709
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:https://github.com/php/php-src/blob/21d9fd3bc1ccf376438e4b5c38bb1945ae3bfe8c/php.ini-production#L677-L690
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 toOn.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_argvisOn(CVE-2023-43655).Outside of the risk caused by Phar files,
register_argc_argvis also a known "trick" to exploit limited Local File Inclusion vulnerabilities in a generic way in Dockerphpimages, 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_argvtoOffby default, keeping it toOnonly for these SAPIs:embed,phpdbgandcli. I'm not sure aboutlitespeedbut from what I'm reading in the code, it seems important too. The documentation inphp.inicould 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!