Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » Removing support for the disable_classes INI setting »

Removing support for the disable_classes INI setting

php.internals

Girgias

3 years ago
Hello internals, While working on some DNF type bugs, I discovered some major issues around the disable_classes INI setting implementation. Such as: - A double-free of the type of a typed property - A use after free (which segfaults) when trying to access a property defined on a disabled class that was extended (e.g. disabling Exception) - A use after free when var_dumping a disabled class that has its own handler (as it will assume properties to be allocated) - And likely more considering the lack of tests surrounding this feature This feature seems of dubious nature, and the only justification given when adding support for this in the changelog of PHP 4.3.2 is: New "disable_classes" php.ini option to allow administrators to disable certain classes for security reasons. [1] However, only classes defined by extensions can be disabled, and such a class is critical for the correct operation of said extension. As such, what one should do for security reasons is to not enable the extension in the first place. Moreover, compared to the behaviour of disable_functions which, as of PHP 8.0, removes the function declaration completely, disable_classes does not remove the declaration of the class, but just "overloads" the object creation process to not initialize the object and emit a warning. Meaning, it is totally valid to instantiate a disabled class and pass it around to functions for them to blow up when they try to use the object as intended. Considering the major flaws in the implementation of said feature, the dubious nature of it, and the seeming lack of usage of it (considering none of the above breaking bugs have been reported). I would like to remove this feature in PHP 8.3, even though I know we are past feature freeze and close to the first RC. I have CCed the RMs for 8.3, and would like the opinion of other people on if this removal makes sense to the majority of people Sincerely, George P. Banyard [1] https://externals.io/message/2076

Larry Garfield

3 years ago
On Mon, Aug 14, 2023, at 2:17 PM, G. P. B. wrote:
> Hello internals, > > While working on some DNF type bugs, I discovered some major issues around > the disable_classes INI setting implementation. Such as: > > - A double-free of the type of a typed property > - A use after free (which segfaults) when trying to access a property > defined on a disabled class that was extended (e.g. disabling Exception) > - A use after free when var_dumping a disabled class that has its own > handler (as it will assume properties to be allocated) > - And likely more considering the lack of tests surrounding this feature > > This feature seems of dubious nature, and the only justification given when > adding support for this in the changelog of PHP 4.3.2 is: > > New "disable_classes" php.ini option to allow administrators to disable > certain classes for security reasons. [1] > > However, only classes defined by extensions can be disabled, and such a > class is critical for the correct operation of said extension. > As such, what one should do for security reasons is to not enable the > extension in the first place. > > Moreover, compared to the behaviour of disable_functions which, as of PHP > 8.0, removes the function declaration completely, disable_classes does not > remove the declaration of the class, but just "overloads" the object > creation process to not initialize the object and emit a warning. > Meaning, it is totally valid to instantiate a disabled class and pass it > around to functions for them to blow up when they try to use the object as > intended. > > Considering the major flaws in the implementation of said feature, the > dubious nature of it, and the seeming lack of usage of it (considering none > of the above breaking bugs have been reported). > I would like to remove this feature in PHP 8.3, even though I know we are > past feature freeze and close to the first RC. > > I have CCed the RMs for 8.3, and would like the opinion of other people on > if this removal makes sense to the majority of people > > Sincerely, > > George P. Banyard > > [1] https://externals.io/message/2076
I believe the intent for it was the same as disable_functions: A "seemed like a good idea at the time" attempt to let shared hosters lock down their environment. That's an increasingly small percentage of the PHP using market, though, so I am perfectly happy to lose disabled_classes entirely. However, I think it should not get an exception for code-freeze. At best, I could see a deprecation warning on it in 8.3 and remove in 8.4/9, but I defer to the RMs on that front. --Larry Garfield

Girgias

3 years ago
On Mon, 14 Aug 2023 at 20:03, Larry Garfield <larry@garfieldtech.com> wrote:
> However, I think it should not get an exception for code-freeze. At best, > I could see a deprecation warning on it in 8.3 and remove in 8.4/9, but I > defer to the RMs on that front. >
In its current state, this INI setting does nothing other than cause segfaults. This functionality is broken, and frankly has always been. The only reason I am asking for an exception is to remove broken functionality that doesn't work and cannot be easily fixed. George P. Banyard

Unnamed Person

3 years ago
On 14-8-2023 16:17, G. P. B. wrote:
> Hello internals, > > While working on some DNF type bugs, I discovered some major issues around > the disable_classes INI setting implementation. Such as: > > - A double-free of the type of a typed property > - A use after free (which segfaults) when trying to access a property > defined on a disabled class that was extended (e.g. disabling Exception) > - A use after free when var_dumping a disabled class that has its own > handler (as it will assume properties to be allocated) > - And likely more considering the lack of tests surrounding this feature > > This feature seems of dubious nature, and the only justification given when > adding support for this in the changelog of PHP 4.3.2 is: > > New "disable_classes" php.ini option to allow administrators to disable > certain classes for security reasons. [1] > > However, only classes defined by extensions can be disabled, and such a > class is critical for the correct operation of said extension. > As such, what one should do for security reasons is to not enable the > extension in the first place. > > Moreover, compared to the behaviour of disable_functions which, as of PHP > 8.0, removes the function declaration completely, disable_classes does not > remove the declaration of the class, but just "overloads" the object > creation process to not initialize the object and emit a warning. > Meaning, it is totally valid to instantiate a disabled class and pass it > around to functions for them to blow up when they try to use the object as > intended. > > Considering the major flaws in the implementation of said feature, the > dubious nature of it, and the seeming lack of usage of it (considering none > of the above breaking bugs have been reported). > I would like to remove this feature in PHP 8.3, even though I know we are > past feature freeze and close to the first RC. > > I have CCed the RMs for 8.3, and would like the opinion of other people on > if this removal makes sense to the majority of people > > Sincerely, > > George P. Banyard > > [1] https://externals.io/message/2076 >
Hi George, For what its worth: in my experience, the `disable_classes` and `disable_functions` ini directives are mostly used by hosting companies providing shared/virtual host environments and, most often for those environments, not editable by their users via a control panel or nor do these users have (edit) access to the php.ini file. Declaring an ini directive in the php.ini file which has been removed will instantly cause a fatal error on starting PHP, so I wonder what the impact will be when a user switches PHP version (which they often can choose to do via a control panel). Hosts should (of course) know what they are doing and this should be handled when the user initiates the PHP version switch, but if hosts knew what they were doing, they would make the extension unavailable (as you already suggested above), so I wonder how many shared hosting users will run into trouble if this is removed without deprecation period.... ? Would deprecating it in PHP 8.3 and removing it in PHP 9.0 be an option ? As for the lack of bug reports - the typical type of end users using those hosts will not know to report these type of issues to PHP (or even be able to properly identify the issue). Instead they will complain extensively to whatever open source project (read: WordPress) they are running on the shared hosting without providing enough information for the open source maintainers to even begin to identify the actual issue... ;-) Just my two cents. Smile, Juliette

Girgias

3 years ago
On Mon, 14 Aug 2023 at 20:04, Juliette Reinders Folmer < php-internals_nospam@adviesenzo.nl> wrote:
> Hi George, > > For what its worth: in my experience, the `disable_classes` and > `disable_functions` ini directives are mostly used by hosting companies > providing shared/virtual host environments and, most often for those > environments, not editable by their users via a control panel or nor do > these users have (edit) access to the php.ini file. >
To be clear, I have no intention of deprecating or removing disable_functions. This INI setting works very well and does not cause any issue within the engine. [...]
> > Would deprecating it in PHP 8.3 and removing it in PHP 9.0 be an option ? >
One option is to "keep" the INI setting, but have it basically do nothing. Is this what you had in mind?
> As for the lack of bug reports - the typical type of end users using > those hosts will not know to report these type of issues to PHP (or even > be able to properly identify the issue). Instead they will complain > extensively to whatever open source project (read: WordPress) they are > running on the shared hosting without providing enough information for > the open source maintainers to even begin to identify the actual > issue... ;-) >
On a minimal build of PHP, with only the mandatory extensions enabled, there are 148 classes/interfaces/traits defined. [1] Other than the SPL ones (and even then), disabling any of these classes will cause issues within the engine. Moreover, the SPL ones are not a security concern. Therefore, any other class that can be disabled must come from an extension that can be disabled altogether. And "disabling" a class from an extension without disabling said extension will render it useless anyway. If a hosting provided is concerned about an extension, then it should not enable it in the first place. Not break it ad hoc. Considering the above, I cannot see how this functionality was ever useful. This is in stark contrast to disable_functions, which can be used to selectively remove functionality of an extension without breaking it overall. George P. Banyard [1] https://gist.github.com/Girgias/63d55ba1e50b580412b004046daed02b

Unnamed Person

3 years ago
On 14-8-2023 22:11, G. P. B. wrote:
> On Mon, 14 Aug 2023 at 20:04, Juliette Reinders Folmer < > php-internals_nospam@adviesenzo.nl> wrote: > >> Would deprecating it in PHP 8.3 and removing it in PHP 9.0 be an option ? > One option is to "keep" the INI setting, but have it basically do nothing. > Is this what you had in mind?
That sounds iffy as well in a way as people (hosts) may then "think" it works, while it doesn't. Disabling the actually functionality (as broken) and then throwing a warning could possibly be a middle-of-the-road solution ? Note - warning, not deprecation if the functionality is turned off in PHP 8.3. With any luck, the hosts' error logs would fill up quickly enough for them to take notice ;-)
> >> As for the lack of bug reports - the typical type of end users using >> those hosts will not know to report these type of issues to PHP (or even >> be able to properly identify the issue). Instead they will complain >> extensively to whatever open source project (read: WordPress) they are >> running on the shared hosting without providing enough information for >> the open source maintainers to even begin to identify the actual >> issue... ;-) > On a minimal build of PHP, with only the mandatory extensions enabled, > there are 148 classes/interfaces/traits defined. [1] > > Other than the SPL ones (and even then), disabling any of these classes > will cause issues within the engine. > Moreover, the SPL ones are not a security concern. > > Therefore, any other class that can be disabled must come from an extension > that can be disabled altogether. And "disabling" a class from an extension > without disabling said extension will render it useless anyway. > > If a hosting provided is concerned about an extension, then it should not > enable it in the first place. Not break it ad hoc.
100% agreed. Unfortunately (in my experience), the hosts using this ini setting are generally not what anyone would consider high quality hosts who know what they are doing.... more the penny-and-dime teenager with a server in the attic type... (which is hopefully a dying breed, but I've seen too many of them come and go over the years - oh and apologies to the potential teenager-host-exception-to-the-rule-who-does-know-what-they-are-doing who may be reading this... ). And the type of end-user I was talking about would describe it as "You broke my website"... (not realizing that it's not the open source package, but the hosting which broke it)
> Considering the above, I cannot see how this functionality was ever useful.
I have absolutely no doubts at all about your analysis. My only remarks were about how to communicate this effectively to the type of ill-informed hosts/end-users who will be affected by the removal. Smile, Juliette

Derick Rethans

3 years ago
On 14 August 2023 20:04:44 BST, Juliette Reinders Folmer <php-internals_nospam@adviesenzo.nl> wrote:
> Declaring an ini directive in the php.ini > file which has been removed will instantly > cause a fatal error on starting PHP, so > I wonder what the impact will be > when a user switches PHP version > (which they often can choose to do > via a control panel).
Are you sure about that? Setting ini entries that don't exist are simply ignored. For example this will generate no warning, and certainly not a fatal error: php -d elephpant.smells=no your-script.php cheers Derick

Unnamed Person

3 years ago
On 15-8-2023 0:52, Derick Rethans wrote:
> On 14 August 2023 20:04:44 BST, Juliette Reinders Folmer <php-internals_nospam@adviesenzo.nl> wrote: > >> Declaring an ini directive in the php.ini >> file which has been removed will instantly >> cause a fatal error on starting PHP, so >> I wonder what the impact will be >> when a user switches PHP version >> (which they often can choose to do >> via a control panel). > Are you sure about that? Setting ini entries that don't exist are simply ignored. > > For example this will generate no warning, and certainly not a fatal error: > > php -d elephpant.smells=no your-script.php > > cheers > Derick >
I though the same, then I ran some tests locally.... (before sending the mail) don't tell me I managed to test with the *one* ini setting which does cause a fatal ? php -d allow_call_time_pass_reference=true your-script.php Smile, Juliette

Kalle Sommer Nielsen

3 years ago
Hi Den man. 14. aug. 2023 kl. 22.05 skrev Juliette Reinders Folmer <php-internals_nospam@adviesenzo.nl>:
> Hi George, > > For what its worth: in my experience, the `disable_classes` and > `disable_functions` ini directives are mostly used by hosting companies > providing shared/virtual host environments and, most often for those > environments, not editable by their users via a control panel or nor do > these users have (edit) access to the php.ini file. > > Declaring an ini directive in the php.ini file which has been removed > will instantly cause a fatal error on starting PHP, so I wonder what the > impact will be when a user switches PHP version (which they often can > choose to do via a control panel).
This depends on whether we choose to explicitly mark the ini directive as one such. As the original implementor of that logic, it was meant as an upgrade tool for PHP 5.3.0 when we also reorganized the php.ini files, rather than have developers just copy over their old, non-working files over to cause unexpected features to still be available. So there is always the option to not enlist it there, though it would be ideal to do so to prevent developers from getting a false sense of "security", but that is just my personal take.
-- regards, Kalle Sommer Nielsen kalle@php.net

Unnamed Person

3 years ago
On 15-8-2023 3:43, Kalle Sommer Nielsen wrote:
> Hi > > Den man. 14. aug. 2023 kl. 22.05 skrev Juliette Reinders Folmer > <php-internals_nospam@adviesenzo.nl>: >> Hi George, >> >> For what its worth: in my experience, the `disable_classes` and >> `disable_functions` ini directives are mostly used by hosting companies >> providing shared/virtual host environments and, most often for those >> environments, not editable by their users via a control panel or nor do >> these users have (edit) access to the php.ini file. >> >> Declaring an ini directive in the php.ini file which has been removed >> will instantly cause a fatal error on starting PHP, so I wonder what the >> impact will be when a user switches PHP version (which they often can >> choose to do via a control panel). > This depends on whether we choose to explicitly mark the ini directive > as one such. As the original implementor of that logic, it was meant > as an upgrade tool for PHP 5.3.0 when we also reorganized the php.ini > files, rather than have developers just copy over their old, > non-working files over to cause unexpected features to still be > available. > > So there is always the option to not enlist it there, though it would > be ideal to do so to prevent developers from getting a false sense of > "security", but that is just my personal take. >
Ah, that explains it! Thanks for adding that context Kalle!

Peter Kokot

3 years ago
On Mon, 14 Aug 2023 at 16:18, G. P. B. <george.banyard@gmail.com> wrote:
> Hello internals, > > While working on some DNF type bugs, I discovered some major issues around > the disable_classes INI setting implementation. Such as: > > - A double-free of the type of a typed property > - A use after free (which segfaults) when trying to access a property > defined on a disabled class that was extended (e.g. disabling Exception) > - A use after free when var_dumping a disabled class that has its own > handler (as it will assume properties to be allocated) > - And likely more considering the lack of tests surrounding this feature > > This feature seems of dubious nature, and the only justification given when > adding support for this in the changelog of PHP 4.3.2 is: > > New "disable_classes" php.ini option to allow administrators to disable > certain classes for security reasons. [1] > > However, only classes defined by extensions can be disabled, and such a > class is critical for the correct operation of said extension. > As such, what one should do for security reasons is to not enable the > extension in the first place. > > Moreover, compared to the behaviour of disable_functions which, as of PHP > 8.0, removes the function declaration completely, disable_classes does not > remove the declaration of the class, but just "overloads" the object > creation process to not initialize the object and emit a warning. > Meaning, it is totally valid to instantiate a disabled class and pass it > around to functions for them to blow up when they try to use the object as > intended. > > Considering the major flaws in the implementation of said feature, the > dubious nature of it, and the seeming lack of usage of it (considering none > of the above breaking bugs have been reported). > I would like to remove this feature in PHP 8.3, even though I know we are > past feature freeze and close to the first RC. > > I have CCed the RMs for 8.3, and would like the opinion of other people on > if this removal makes sense to the majority of people > > Sincerely, > > George P. Banyard > > [1] https://externals.io/message/2076
I'm not against removing the disable_classes, but fuzzer SAPI is using this ini setting like this: https://github.com/php/php-src/blob/1fe7dc31ef149db20ea3813c92a45deff80c21a3/sapi/fuzzer/fuzzer-sapi.c#L60 From the user point of view, it would otherwise be good to have a certain set of PHP functionality that they can always rely on. Checking if some class or function is disabled in some environment is kind of strange indeed. But from the PHP configuration point of view, more options to customize the PHP environment is maybe good to have.