NonceVerification: bug fix - sanitization is no alternative for nonce check

Any usage of superglobals in calls to sanitization functions were up to now ignored. As the output of sanitization is normally assigned to a names variable, this meant that it was possible to bypass the nonce verification sniff that way.

I don't believe that was the intended behaviour. Instead IMO, a sanitization call should be allowed prior to the nonce verification, but should not negate the nonce verification check.

This fixes that.

Includes unit tests.

Author: jrfnl

WordPress/Sniff.php

@@ -1456,6 +1456,7 @@ protected function has_nonce_check( $stackPtr ) {
 			|| $this->is_comparison( $stackPtr )
 			|| $this->is_in_array_comparison( $stackPtr )
 			|| $this->is_in_function_call( $stackPtr, $this->unslashingFunctions ) !== false
+			|| $this->is_only_sanitized( $stackPtr )
 		) {
 			$allow_nonce_after = true;
 		}

WordPress/Sniffs/Security/NonceVerificationSniff.php

@@ -121,10 +121,6 @@ public function process_token( $stackPtr ) {
 
 		$this->mergeFunctionLists();
 
-		if ( $this->is_only_sanitized( $stackPtr ) ) {
-			return;
-		}
-
 		if ( $this->has_nonce_check( $stackPtr ) ) {
 			return;
 		}

WordPress/Tests/Security/NonceVerificationUnitTest.inc

@@ -264,3 +264,14 @@ function allow_for_unslash_in_sanitization() {
 	wp_verify_nonce( $var );
 	echo $var;
 }
+
+function dont_allow_bypass_nonce_via_sanitization() {
+	$var = sanitize_text_field( $_POST['foo'] ); // Bad.
+	echo $var;
+}
+
+function dont_allow_bypass_nonce_via_sanitization() {
+	$var = sanitize_text_field( $_POST['foo'] ); // OK.
+	wp_verify_nonce( $var );
+	echo $var;
+}

WordPress/Tests/Security/NonceVerificationUnitTest.php

@@ -52,6 +52,7 @@ public function getErrorList() {
 			198 => 1,
 			202 => 1,
 			252 => 1,
+			269 => 1,
 		);
 	}