search

magento-ecg / coding-standard

Magento PHP_CodeSniffer Coding Standard
MIT License
308 stars 99 forks source link

Can someone explain to me what is bad about a good use of call_user_func(_array) #53

Closed JeroenBoersma closed 7 years ago

JeroenBoersma commented 7 years ago

In https://github.com/magento-ecg/coding-standard/blob/master/Ecg/Sniffs/Security/ForbiddenFunctionSniff.php#L16 I find both call_user_func functions as forbidden.

I couldn't find a direct source for the reason, so that's why I'm posting it here.

Thanks in advance.

zlik commented 7 years ago

Hi @JeroenBoersma,

It's is OK to use call_user_func, when used wisely. It is often called in Magento core modules. However, we really need to watch for call_user_func as it might be used in an insecure way. A function injection attack is possible when passing an invalidated user-supplied string as an argument: https://www.owasp.org/index.php/Function_Injection.

We should probably revisit call_user_func and other functions that are OK to use (keeping in mind security) in the forbidden functions list and move them to the discouraged list. We still want to report usages of those functions as they might be dangerous.

JeroenBoersma commented 7 years ago

Hi @zlik ,

Thanks for the quick response and the additional explanation. It would be cool if could be a warning instead of an error. (maybe if I got some spare time I'll create a PR myself(don't wait for it of coarse))

Looking forward to see future updates and thanks for this project.