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