WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.54k stars 479 forks source link

"empty" will ignore sanitizing errors even when insecure #2346

Closed kkmuffme closed 1 year ago

kkmuffme commented 1 year ago

Bug Description

When passing an unsafe variable to a function, but wrapping the result in empty won't report an error for missing sanitizing, even though this is unsafe.

Same also for "isset"

Minimal Code Snippet

The issue happens when running this command:

./bin/phpcs --config-set installed_paths ../wpcs

... over a file containing this code:

ìf ( empty( foo( $_POST['hello'] ) ) {

}

Error Code

None. Expected WordPress.Security.ValidatedSanitizedInput.InputNotSanitized

Environment

Question Answer
PHP version 7.4
PHP_CodeSniffer version 3.7.4
WPCS version 2.3.0
WPCS install type https://github.com/WordPress/WordPress-Coding-Standards#standalone

Tested Against develop branch?

dingo-d commented 1 year ago

I cannot reproduce this on the develop branch. I have the following file

<?php

ìf ( empty( foo( $_POST['hello'] ) ) {

}

Which returned these errors when running vendor/bin/phpcs --standard=WordPress test.php -sp

----------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------------------------------------------------------------------------------------
 1 | ERROR | Missing file doc comment (Squiz.Commenting.FileComment.Missing)
 3 | ERROR | PHP syntax error: syntax error, unexpected '{', expecting ')' (Generic.PHP.Syntax.PHPSyntax)
 3 | ERROR | $_POST data not unslashed before sanitization. Use wp_unslash() or similar
   |       | (WordPress.Security.ValidatedSanitizedInput.MissingUnslash)
 3 | ERROR | Detected usage of a non-sanitized input variable: $_POST['hello'] (WordPress.Security.ValidatedSanitizedInput.InputNotSanitized)
 3 | ERROR | Processing form data without nonce verification. (WordPress.Security.NonceVerification.Missing)
----------------------------------------------------------------------------------------------------------------------------------------------------

so the ValidatedSanitizedInput.InputNotSanitized is there...

jrfnl commented 1 year ago

Just checked and this is not even reproducable with WPCS 2.3.0.... even with the parse error still in the code....

dingo-d commented 1 year ago

Just checked and this is not even reproducable with WPCS 2.3.0.... even with the parse error still in the code....

Totally missed the missing ) 😂 😅