Hey Kamil,
On 23.7.2025 21:01:14, Kamil Tekiela wrote:
This would prevent most of the
vulnerabilities found in the dataset and we cannot think of a
valid use
case for allowing this behavior
But not all. This function is dangerous on its own. Attack vectors are
99% through superglobals but could come from other sources too.
However, this function is handy when used correctly.
I agree with everything stated in the email. Using extract() on
superglobals is definitely an incorrect usage and should be forbidden.
If it's something that we can do then we should do it as soon as
possible even if it means breaking some poorly written code.
Empty prefix should be a bug and as such I recommend adding an error
for this in PHP 8.5 without deprecation or RFC.
One more thing that would improve security is to change the default
flag to EXTR_SKIP. It would be a major BC though so we could probably
only do it in PHP 9.
I can agree on making EXTR_SKIP the default.
I do not agree that empty prefix is a bug though. That's a common
operation for templating for example.
Regarding extract on superglobals, I have some legacy code from PHP 3
times, which still lives on with the following lines of code:
// Emulate register_globals on extract($_POST,EXTR_SKIP);
extract($_GET,EXTR_SKIP);
Yeah, not the most awesome. But it works. Obviously, you can work around
that with foreach ($_GET as $key => $val) $$key = $val;. So, I'm not
strictly against warning here, but it doesn't necessarily mean the code
is incorrect. Just not necessarily to todays standards.
Bob
