Skip to content

Commit 6e470de

Browse files
committed
Sniff::is_sanitized(): implement new is_in_function_call() method
This fixes potential false negatives when a method/namespaced function mirroring the name of the WP unslashing/sanitization functions would be used. Includes unit test.
1 parent 95e3a3d commit 6e470de

File tree

3 files changed

+24
-8
lines changed

3 files changed

+24
-8
lines changed

WordPress/Sniff.php

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1703,9 +1703,16 @@ protected function is_sanitized( $stackPtr, $require_unslash = false ) {
17031703
return true;
17041704
}
17051705

1706-
// If this isn't a call to a function, it sure isn't a sanitizing function.
1707-
if ( \T_STRING !== $this->tokens[ $functionPtr ]['code'] ) {
1708-
if ( $require_unslash ) {
1706+
$valid_functions = $this->sanitizingFunctions;
1707+
$valid_functions += $this->unslashingSanitizingFunctions;
1708+
$valid_functions['wp_unslash'] = true;
1709+
$valid_functions['array_map'] = true;
1710+
1711+
$functionPtr = $this->is_in_function_call( $stackPtr, $valid_functions );
1712+
1713+
// If this isn't a call to one of the valid functions, it sure isn't a sanitizing function.
1714+
if ( false === $functionPtr ) {
1715+
if ( true === $require_unslash ) {
17091716
$this->add_unslash_error( $stackPtr );
17101717
}
17111718

@@ -1717,15 +1724,17 @@ protected function is_sanitized( $stackPtr, $require_unslash = false ) {
17171724
// Check if wp_unslash() is being used.
17181725
if ( 'wp_unslash' === $functionName ) {
17191726

1720-
$is_unslashed = true;
1721-
$function_opener = array_pop( $nested_openers );
1727+
$is_unslashed = true;
1728+
1729+
unset( $valid_functions['wp_unslash'] );
1730+
$higherFunctionPtr = $this->is_in_function_call( $functionPtr, $valid_functions );
17221731

1723-
// If there is no other function being used, this value is unsanitized.
1724-
if ( ! isset( $function_opener ) ) {
1732+
// If there is no other valid function being used, this value is unsanitized.
1733+
if ( false === $higherFunctionPtr ) {
17251734
return false;
17261735
}
17271736

1728-
$functionPtr = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $function_opener - 1 ), null, true, null, true );
1737+
$functionPtr = $higherFunctionPtr;
17291738
$functionName = $this->tokens[ $functionPtr ]['content'];
17301739

17311740
} else {

WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.inc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,3 +211,8 @@ if ( ClassName::array_key_exists( 'my_field5', $_POST ) ) {
211211
}
212212

213213
echo sanitize_text_field (wp_unslash ($_GET['test'])); // OK.
214+
215+
if ( isset( $_GET['unslash_check'] ) ) {
216+
$clean = sanitize_text_field( WP_Faker::wp_unslash( $_GET['unslash_check'] ) ); // Bad x1 - unslash.
217+
$clean = WP_Faker\sanitize_text_field( wp_unslash( $_GET['unslash_check'] ) ); // Bad x1 - sanitize.
218+
}

WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ public function getErrorList() {
5858
202 => 1,
5959
206 => 1,
6060
210 => 1,
61+
216 => 1,
62+
217 => 1,
6163
);
6264
}
6365

0 commit comments

Comments
 (0)