search

FloeDesignTechnologies / phpcs-security-audit

phpcs-security-audit is a set of PHP_CodeSniffer rules that finds vulnerabilities and weaknesses related to security in PHP code
GNU General Public License v3.0
712 stars 85 forks source link

add TypeJuggleSniff.php #53

Closed kevcooper closed 4 years ago

kevcooper commented 4 years ago

This sniff throws a warning if you use == instead of ===. Most of the time it's best to use === to prevent accidentally having types converted and evaluating in an unexpected way.

kevcooper commented 4 years ago

also checks for != and <>

jmarcil commented 4 years ago

Hello @kevcooper,

thank you for your contribution.

I believe it is a good thing to check that is sometimes related to security.

However I do believe we should only return findings for this sniff when in paranoia mode since the odds of it being a security concern are low. See https://github.com/FloeDesignTechnologies/phpcs-security-audit/blob/master/Security/Sniffs/BadFunctions/CryptoFunctionsSniff.php#L45 for how to do it. As a reminder paranoia mode is enabled by default, but is a way to get substantially less false positive from this tool when turned off.

I'm not used to get new rules, so apologies if the contribution guidelines are non-existent, but usually I like to add a one liner test in tests.php for every rule and also include them into the example ruleset files. I'll try to rectify that partially in a upcoming update.

Note that we have now changed the namespace to PHPCS_SecurityAudit\Security\Sniffs\ with the additional Security\ in it.

I'll let you make the changes since I want more people to test the major updates we just pushed to master after #50. Let me know if you encounter any problems.

Thanks!

kevcooper commented 4 years ago

Ok, I've updated the namespace, added a check for paranoia mode, and added the requested lines to tests.php and example_base_ruleset.xml

jmarcil commented 4 years ago

Thank you very much for this sniff!