Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:120896
MIME-Version: 1.0
Date: Mon, 14 Aug 2023 15:17:45 +0100
Message-ID: <CAFPFaMLBZixUFa=7=9Gxj60D=4QyfSpF9se-U+6CRoShzCrFrA@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Cc: Jakub Zelenka <bukka@php.net>, eric@eamann.com, adoy@php.net
Content-Type: multipart/alternative; boundary="0000000000009ea24c0602e2bab4"
Subject: [PHP-DEV] Removing support for the disable_classes INI setting
From: george.banyard@gmail.com ("G. P. B.")

--0000000000009ea24c0602e2bab4
Content-Type: text/plain; charset="UTF-8"

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

--0000000000009ea24c0602e2bab4--