Everything

php

Set register_argc_argv to Off by default

php.internals

Unnamed Person

2 years ago

Hey,

I recently opened an issue on GitHub [1] to discuss setting
register_argc_argv to Off by default for all SAPIs but cli, embed, and
phpdbg. Ilija Tovilo suggested including this change in 8.4.0.

Even though most downstream distributions already turn it off, that's
not the case everywhere. For instance,  the official Docker image has
it on [2]. Outside of performance reasons, this also has a security
impact because it eases the exploitation of limited LFI bugs [3] and
CLI tools stored under the web root [4].

-Thomas

[1]: https://github.com/php/php-src/issues/12344
[2]: https://hub.docker.com/_/php
[3]: https://www.youtube.com/watch?v=yq2rq50IMSQ
[4]: https://github.com/advisories/GHSA-jm6m-4632-36hf

Sebastian Bergmann

2 years ago

Am 07.11.2023 um 11:33 schrieb Thomas Chauchefoin via internals:

> For instance, the official Docker image has it on [2].

"Official" is relative here. That image is maintained by (the) Docker
(community), it is not maintained by the PHP project.

Kévin Dunglas

2 years ago

This change seems reasonable to me: safer, with little chance of breaking
things, and easy to reverse for the end user by changing a single parameter.

Matteo Beccati

2 years ago

Hi,

Il 07/11/2023 14:13, Kévin Dunglas ha scritto:

> This change seems reasonable to me: safer, with little chance of breaking
> things, and easy to reverse for the end user by changing a single parameter.

/me runs to remove the 16-years old "register_argc_argv = on" check from
the web installer of a legacy open source software ;-)

Seriously though, I do agree with changing the default. I believe such
check was added in a time when it was still common to use the cgi
executable to run cli commands and the configuration was shared with
mod_php.


Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Unnamed Person

2 years ago

On Tue, Nov 7, 2023 at 11:46 AM Sebastian Bergmann <sebastian@php.net> wrote:

> Am 07.11.2023 um 11:33 schrieb Thomas Chauchefoin via internals:
> > For instance, the official Docker image has it on [2].
>
> "Official" is relative here. That image is maintained by (the) Docker
> (community), it is not maintained by the PHP project.

Indeed, my bad. I've updated the GitHub issue to reflect that this
Docker image is not maintained by the PHP project.

On Tue, Nov 7, 2023 at 2:12 PM Kévin Dunglas <kevin@dunglas.fr> wrote:

> This change seems reasonable to me: safer, with little chance of breaking
> things, and easy to reverse for the end user by changing a single parameter.

On Tue, Nov 7, 2023 at 5:33 PM Matteo Beccati <matteo@beccati.com> wrote:

> Seriously though, I do agree with changing the default. I believe such
> check was added in a time when it was still common to use the cgi
> executable to run cli commands and the configuration was shared with
> mod_php.

On Tue, Nov 7, 2023 at 4:48 PM G. P. B. <george.banyard@gmail.com> wrote:

> This sounds sensible to me.

Thank you for your early feedback on this suggestion. Without rushing
things, I assume that the next step for me would be to draft an RFC to
formalize this change?

Best,
-Thomas

Girgias

2 years ago

On Wed, 8 Nov 2023 at 16:59, Thomas Chauchefoin <
thomas.chauchefoin@sonarsource.com> wrote:

> Thank you for your early feedback on this suggestion. Without rushing
> things, I assume that the next step for me would be to draft an RFC to
> formalize this change?
>

The change seems to be uncontroversial, so an RFC feels unnecessary here.
Just make a PR and I think we can merge this into master.

Best regards,

Gina P. Banyard

Girgias

2 years ago

On Tue, 7 Nov 2023 at 10:33, Thomas Chauchefoin via internals <
internals@lists.php.net> wrote:

> Hey,
>
> I recently opened an issue on GitHub [1] to discuss setting
> register_argc_argv to Off by default for all SAPIs but cli, embed, and
> phpdbg. Ilija Tovilo suggested including this change in 8.4.0.
>
> Even though most downstream distributions already turn it off, that's
> not the case everywhere. For instance,  the official Docker image has
> it on [2]. Outside of performance reasons, this also has a security
> impact because it eases the exploitation of limited LFI bugs [3] and
> CLI tools stored under the web root [4].
>
> -Thomas
>
> [1]: https://github.com/php/php-src/issues/12344
> [2]: https://hub.docker.com/_/php
> [3]: https://www.youtube.com/watch?v=yq2rq50IMSQ
> [4]: https://github.com/advisories/GHSA-jm6m-4632-36hf
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>

This sounds sensible to me.

Best regards,

Gina/George P. Banyard