Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:120900
Feedback-ID: i8414410d:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-624-g7714e4406d-fm-20230801.001-g7714e440
Mime-Version: 1.0
Message-ID: <8a9b7bc7-5d3d-46a3-bf3c-0af66aa2796f@app.fastmail.com>
In-Reply-To: 
 <CAFPFaMLBZixUFa=7=9Gxj60D=4QyfSpF9se-U+6CRoShzCrFrA@mail.gmail.com>
References: 
 <CAFPFaMLBZixUFa=7=9Gxj60D=4QyfSpF9se-U+6CRoShzCrFrA@mail.gmail.com>
Date: Mon, 14 Aug 2023 19:03:19 +0000
To: "G. P. B." <george.banyard@gmail.com>,
 "php internals" <internals@lists.php.net>
Cc: "Jakub Zelenka" <bukka@php.net>, eric@eamann.com, adoy@php.net
Content-Type: text/plain
Subject: Re: [PHP-DEV] Removing support for the disable_classes INI setting
From: larry@garfieldtech.com ("Larry Garfield")

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