logoalt Hacker News

63stack • today at 7:06 AM • 1 reply • view on HN

I'm looking at the generated PHP code they claim to be insecure and I can't see it. The client_secret is defined inside the file as a constant, ok it's sloppy and could be better, but this is not a vulnerability.

They claim the extract line is insecure, but never explain why, and without showing the rest of the code (what is $requiredHeaders? If it's sanitized this is a non issue) you can't even tell if it's secure or not.

The variable names being wrong (the $PayPal- ones) is probably just a "mistake" the model made, but it's not a vulnerability either.

This article could be just "burglars are burgling in your neighborhood, install our burglar alarm" from crowdstrike just as well.


Replies

callahad • today at 8:58 AM

Not a PHP dev, but it seems calling `extract` is itself potentially insecure. The php docs warn against it (https://www.php.net/manual/en/function.extract.php):

> Do not use extract() on untrusted data, like user input (e.g. $_GET, $_FILES). If you do, make sure you use one of the non-overwriting flags values such as EXTR_SKIP and be aware that you should extract in the same order that's defined in variables_order within the php.ini.

Looks like calling it can shadow previously defined, unrelated variables?

Edit: On re-reading your comment, yes, if $requiredHeaders is previously sanitized then the code is technically safe. But it still presents as an insecure coding practice.