XSS sniff: require escaping for die(), exit(), printf()

WordPress/Sniffs/XSS/EscapeOutputSniff.php

@@ -233,6 +233,8 @@ class WordPress_Sniffs_XSS_EscapeOutputSniff implements PHP_CodeSniffer_Sniff
 		'_ngettext',
 		'_nx',
 		'_x',
+		'printf',
+		'vprintf',
 	);
 
 	public static $addedCustomFunctions = false;
@@ -247,6 +249,7 @@ public function register()
 		return array(
 			T_ECHO,
 			T_PRINT,
+			T_EXIT,
 			T_STRING,
 		);
 
@@ -275,13 +278,25 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
 
 		$tokens = $phpcsFile->getTokens();
 
+		$needs_sanitizing_function = false;
+
+		$function = $tokens[ $stackPtr ]['content'];
+
 		// If function, not T_ECHO nor T_PRINT
 		if ( $tokens[$stackPtr]['code'] == T_STRING ) {
 			// Skip if it is a function but is not of the printing functions ( self::needSanitizingFunctions )
 			if ( ! in_array( $tokens[$stackPtr]['content'], $this->needSanitizingFunctions ) ) {
 				return;
 			}
 
+			$needs_sanitizing_function = true;
+
+			$stackPtr++; // Ignore the starting bracket
+
+			$end_of_statement = $tokens[ $stackPtr ]['parenthesis_closer'];
+		}
+
+		if ( $tokens[ $stackPtr ]['code'] === T_EXIT ) {
 			$stackPtr++; // Ignore the starting bracket
 		}
 
@@ -310,7 +325,10 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
 			}
 		}
 
-		$end_of_statement = $phpcsFile->findNext( T_SEMICOLON, $stackPtr );
+		// This is already determined if $needs_sanitizing_function.
+		if ( ! isset( $end_of_statement ) ) {
+			$end_of_statement = $phpcsFile->findNext( T_SEMICOLON, $stackPtr );
+		}
 
 		// Check for the ternary operator.
 		$ternary = $phpcsFile->findNext( T_INLINE_THEN, $stackPtr, $end_of_statement );
@@ -326,6 +344,11 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
 			if ( $tokens[$i]['code'] == T_WHITESPACE )
 				continue;
 
+			if ( 'vprintf' === $function && $tokens[ $i ]['code'] === T_ARRAY ) {
+				$i++; // Skip the opening parenthesis.
+				continue;
+			}
+
 			// Wake up on concatenation characters, another part to check
 			if ( in_array( $tokens[$i]['code'], array( T_STRING_CONCAT ) ) ) {
 				$watch = true;
@@ -338,6 +361,12 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
 				continue;
 			}
 
+			// Wake up for commas.
+			if ( $needs_sanitizing_function && $tokens[$i]['code'] === T_COMMA ) {
+				$watch = true;
+				continue;
+			}
+
 			if ( $watch === false )
 				continue;
 

WordPress/Tests/XSS/EscapeOutputUnitTest.inc

@@ -60,4 +60,26 @@ echo $foo ? $foo : 'no foo'; // Bad
 echo empty( $foo ) ? 'no foo' : $foo; // Bad
 echo $foo ? esc_html( $foo ) : 'no foo'; // OK
 
-echo 4; // OK
+echo 4; // OK
+
+exit( $foo ); // Bad
+exit( esc_html( $foo ) ); // OK
+
+die( $foo ); // Bad
+die( esc_html( $foo ) ); // OK
+
+printf( 'Hello %s', $foo ); // Bad
+printf( 'Hello %s', esc_html( $foo ) ); // OK
+printf( 'Hello %s! Hi %s!', esc_html( $foo ), $bar ); // Bad
+
+vprintf( 'Hello %s', array( $foo ) ); // Bad
+vprintf( 'Hello %s', array( esc_html( $foo ) ) ); // OK
+
+// The below checks that functions which are marked as needed further sanitization
+// don't spill over into later arguments when nested in a function call. There was
+// a bug which would cause line 84 to be marked as needing sanitization because _x()
+// is marked as needing sanitization.
+do_something(
+	_x( 'Some string', 'context', 'domain' )
+	, array( $foo ) // OK
+);

WordPress/Tests/XSS/EscapeOutputUnitTest.php

@@ -53,6 +53,11 @@ public function getErrorList()
                 46 => 1,
                 59 => 1,
                 60 => 1,
+                65 => 1,
+                68 => 1,
+                71 => 1,
+                73 => 1,
+                75 => 1,
                );
 
     }//end getErrorList()