Merge pull request #595 from Automattic/fix/345-underscorejs-output-e…

…scaping

Security/Underscorejs: bug fixes and enhancements

WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php

@@ -8,6 +8,7 @@
 
 namespace WordPressVIPMinimum\Sniffs\Security;
 
+use PHP_CodeSniffer\Util\Tokens;
 use WordPressVIPMinimum\Sniffs\Sniff;
 
 /**
@@ -17,6 +18,29 @@
  */
 class UnderscorejsSniff extends Sniff {
 
+	/**
+	 * Regex to match unescaped output notations containing variable interpolation
+	 * and retrieve a code snippet.
+	 *
+	 * @var string
+	 */
+	const UNESCAPED_INTERPOLATE_REGEX = '`<%=\s*(?:.+?%>|$)`';
+
+	/**
+	 * Regex to match execute notations containing a print command
+	 * and retrieve a code snippet.
+	 *
+	 * @var string
+	 */
+	const UNESCAPED_PRINT_REGEX = '`<%\s*(?:print\s*\(.+?\)\s*;|__p\s*\+=.+?)\s*%>`';
+
+	/**
+	 * Regex to match the "interpolate" keyword when used to overrule the ERB-style delimiters.
+	 *
+	 * @var string
+	 */
+	const INTERPOLATE_KEYWORD_REGEX = '`(?:templateSettings\.interpolate|\.interpolate\s*=\s*/|interpolate\s*:\s*/)`';
+
 	/**
 	 * A list of tokenizers this sniff supports.
 	 *
@@ -30,12 +54,11 @@ class UnderscorejsSniff extends Sniff {
 	 * @return array
 	 */
 	public function register() {
-		return [
-			T_CONSTANT_ENCAPSED_STRING,
-			T_PROPERTY,
-			T_INLINE_HTML,
-			T_HEREDOC,
-		];
+		$targets   = Tokens::$textStringTokens;
+		$targets[] = T_PROPERTY;
+		$targets[] = T_STRING;
+
+		return $targets;
 	}
 
 	/**
@@ -46,15 +69,91 @@ public function register() {
 	 * @return void
 	 */
 	public function process_token( $stackPtr ) {
+		/*
+		 * Ignore Gruntfile.js files as they are configuration, not code.
+		 */
+		$file_name = $this->strip_quotes( $this->phpcsFile->getFileName() );
+		$file_name = strtolower( basename( $file_name ) );
+
+		if ( $file_name === 'gruntfile.js' ) {
+			return;
+		}
+
+		/*
+		 * Check for delimiter change in JS files.
+		 */
+		if ( $this->tokens[ $stackPtr ]['code'] === T_STRING
+			|| $this->tokens[ $stackPtr ]['code'] === T_PROPERTY
+		) {
+			if ( $this->phpcsFile->tokenizerType !== 'JS' ) {
+				// These tokens are only relevant for JS files.
+				return;
+			}
+
+			if ( $this->tokens[ $stackPtr ]['content'] !== 'interpolate' ) {
+				return;
+			}
+
+			// Check the context to prevent false positives.
+			if ( $this->tokens[ $stackPtr ]['code'] === T_STRING ) {
+				$prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true );
+				if ( $prev === false || $this->tokens[ $prev ]['code'] !== T_OBJECT_OPERATOR ) {
+					return;
+				}
+
+				$prevPrev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true );
+				$next     = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
+				if ( ( $prevPrev === false
+					|| $this->tokens[ $prevPrev ]['code'] !== T_STRING
+					|| $this->tokens[ $prevPrev ]['content'] !== 'templateSettings' )
+					&& ( $next === false
+					|| $this->tokens[ $next ]['code'] !== T_EQUAL )
+				) {
+					return;
+				}
+			}
+
+			// Underscore.js delimiter change.
+			$message = 'Found Underscore.js delimiter change notation.';
+			$this->phpcsFile->addWarning( $message, $stackPtr, 'InterpolateFound' );
+
+			return;
+		}
+
+		$content = $this->strip_quotes( $this->tokens[ $stackPtr ]['content'] );
+
+		$match_count = preg_match_all( self::UNESCAPED_INTERPOLATE_REGEX, $content, $matches );
+		if ( $match_count > 0 ) {
+			foreach ( $matches[0] as $match ) {
+				if ( strpos( $match, '_.escape(' ) !== false ) {
+					continue;
+				}
+
+				// Underscore.js unescaped output.
+				$message = 'Found Underscore.js unescaped output notation: "%s".';
+				$data    = [ $match ];
+				$this->phpcsFile->addWarning( $message, $stackPtr, 'OutputNotation', $data );
+			}
+		}
+
+		$match_count = preg_match_all( self::UNESCAPED_PRINT_REGEX, $content, $matches );
+		if ( $match_count > 0 ) {
+			foreach ( $matches[0] as $match ) {
+				if ( strpos( $match, '_.escape(' ) !== false ) {
+					continue;
+				}
 
-		if ( strpos( $this->tokens[ $stackPtr ]['content'], '<%=' ) !== false ) {
-			// Underscore.js unescaped output.
-			$message = 'Found Underscore.js unescaped output notation: "<%=".';
-			$this->phpcsFile->addWarning( $message, $stackPtr, 'OutputNotation' );
+				// Underscore.js unescaped output.
+				$message = 'Found Underscore.js unescaped print execution: "%s".';
+				$data    = [ $match ];
+				$this->phpcsFile->addWarning( $message, $stackPtr, 'PrintExecution', $data );
+			}
 		}
 
-		if ( strpos( $this->tokens[ $stackPtr ]['content'], 'interpolate' ) !== false ) {
-			// Underscore.js unescaped output.
+		if ( $this->phpcsFile->tokenizerType !== 'JS'
+			&& preg_match( self::INTERPOLATE_KEYWORD_REGEX, $content ) > 0
+		) {
+			// Underscore.js delimiter change.
 			$message = 'Found Underscore.js delimiter change notation.';
 			$this->phpcsFile->addWarning( $message, $stackPtr, 'InterpolateFound' );
 		}

WordPressVIPMinimum/Tests/Security/Gruntfile.js

@@ -0,0 +1,100 @@
+
+module.exports = function(grunt) {
+
+	require('load-grunt-tasks')(grunt);
+
+	// Project configuration.
+	grunt.initConfig({
+		pkg: grunt.file.readJSON('package.json'),
+
+		checktextdomain: {
+			options:{
+				text_domain: '<%= pkg.name %>',
+				correct_domain: true,
+				keywords: [
+					'__:1,2d',
+					'_e:1,2d',
+					'_x:1,2c,3d',
+					'esc_html__:1,2d',
+					'esc_html_e:1,2d',
+					'esc_html_x:1,2c,3d',
+					'esc_attr__:1,2d',
+					'esc_attr_e:1,2d',
+					'esc_attr_x:1,2c,3d',
+					'_ex:1,2c,3d',
+					'_n:1,2,4d',
+					'_nx:1,2,4c,5d',
+					'_n_noop:1,2,3d',
+					'_nx_noop:1,2,3c,4d'
+				]
+			},
+			files: {
+				src:  [
+					'**/*.php',
+				],
+				expand: true
+			}
+		},
+
+		makepot: {
+			target: {
+				options: {
+					domainPath: '/languages/',    // Where to save the POT file.
+					mainFile: 'style.css',      // Main project file.
+					potFilename: '<%= pkg.name %>.pot',   // Name of the POT file.
+					type: 'wp-theme',  // Type of project (wp-plugin or wp-theme).
+					processPot: function( pot, options ) {
+						pot.headers['plural-forms'] = 'nplurals=2; plural=n != 1;';
+						pot.headers['x-poedit-basepath'] = '.\n';
+						pot.headers['x-poedit-language'] = 'English\n';
+						pot.headers['x-poedit-country'] = 'UNITED STATES\n';
+						pot.headers['x-poedit-sourcecharset'] = 'utf-8\n';
+						pot.headers['X-Poedit-KeywordsList'] = '__;_e;__ngettext:1,2;_n:1,2;__ngettext_noop:1,2;_n_noop:1,2;_c,_nc:4c,1,2;_x:1,2c;_ex:1,2c;_nx:4c,1,2;_nx_noop:4c,1,2;\n';
+						pot.headers['x-textdomain-support'] = 'yes\n';
+						return pot;
+					}
+				}
+			}
+		},
+
+		// Clean up build directory
+		clean: {
+			main: ['build/<%= pkg.name %>']
+		},
+
+		// Copy the theme into the build directory
+		copy: {
+			main: {
+				src:  [
+					'**',
+					'!build/**',
+					'!.git/**',
+					'!Gruntfile.js',
+					'!package.json',
+					'!.gitignore',
+					'!.gitmodules',
+				],
+				dest: 'build/<%= pkg.name %>/'
+			}
+		},
+
+		//Compress build directory into <name>.zip and <name>-<version>.zip
+		compress: {
+			main: {
+				options: {
+					mode: 'zip',
+					archive: './build/<%= pkg.name %>.zip'
+				},
+				expand: true,
+				cwd: 'build/<%= pkg.name %>/',
+				src: ['**/*'],
+				dest: '<%= pkg.name %>/'
+			}
+		}
+
+	});
+
+	// Default task(s).
+	grunt.registerTask( 'build', [ 'clean', 'copy', 'compress' ] );
+	grunt.registerTask( 'i18n', [ 'checktextdomain', 'makepot' ] );
+};

WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc

@@ -12,6 +12,109 @@ EOT;
 <script type="text/template" id="js-tpl--example">
 			<a
 				href="<%= post_url %>"> <!-- NOK. -->
-				<h3><%- post_title %></h3>
+				<h3><%- post_title %></h3><!-- OK -->
 			</a>
 </script>
+
+<?php
+
+function single_quoted_string() {
+	echo '<div class="thumb" data-attid="<%=   data.id   %>" data-origsrc="<%- data.originalSrc %>">'. // NOK x 1.
+		'<img src="<%- data.src %>" alt="<%=data.alt%>"/>'. // NOK x 1.
+	'</div>';
+}
+
+function single_quoted_string_with_concatenation( $data ) {
+	echo '<img src="<%= ' . $data['src'] . ' %>" alt="<%- data.alt %>"/>'; // NOK x 1.
+}
+
+function double_quoted_string( $name, $value, $is_template ) {
+	echo $is_template ? "<%={$name}%>" : esc_attr( $value ); // NOK.
+}
+
+$nowdoc = <<<'EOT'
+<script type="text/template" id="prefix-template">
+	<section class="prefix-photo prefix-image">
+		<img src="<%= img.src %>" class="prefix-image" width="<%= img.width %>" height="<%= img.height %>" /><!-- NOK x 3 -->
+	</section>
+</script>
+EOT;
+
+$heredoc = <<<EOD
+	<label
+		class="$classes prefix-form-ui-label-<%- htmlAttr.id %><%= ordinal %>"
+		for="<%- htmlAttr.id %><%= ordinal %>">
+		<%= name %>
+	</label><!-- NOK - 1 per line -->
+EOD;
+
+// Make sure the JS specific check does not trigger on PHP code.
+$obj->interpolate = true;
+
+// Test matching the "interpolate" keyword with higher precision (mirrors same check in JS).
+function test_interpolate_match_precision() {
+	?>
+	<script type="text/javascript">
+	_.templateSettings.interpolate = /\{\{(.+?)\}\}/g; /* NOK */
+
+	options.interpolate=_.templateSettings.interpolate; /* NOK */
+	var interpolate = options.interpolate || reNoMatch, /* Ignore */
+		source = "__p += '";
+
+	// Prevent false positives on "interpolate".
+	var preventMisidentification = 'text interpolate text'; // OK.
+	var interpolate = THREE.CurveUtils.interpolate; // OK.
+
+	var p = function(f, d) {
+		return s.interpolate(m(f), _(d), 0.5, e.color_space) // OK.
+	}
+
+	y.interpolate.bezier = b; // OK.
+	</script>
+	<?php
+}
+
+// Recognize escaping.
+function dont_trigger_when_escaped() {
+	$script = <<<EOD
+		var html = _.template('<li><%= _.escape(name) %></li>', { name: 'John Smith' }); // OK.
+		
+		var html = _.template(
+			"<pre>The \"<% __p+=_.escape(o.text) %>\" is the same<br />" + // OK.
+				"as the  \"<%= _.escape(o.text) %>\" and the same<br />" + // OK.
+				"as the \"<%- o.text %>\"</pre>", // OK.
+			{
+				text: "<b>some text</b> and \n it's a line break"
+			},
+			{
+				variable: "o"
+			}
+		);
+EOD;
+
+	echo $script;
+}
+
+function display_foo {
+?>
+	<script id="template" type="text/template">
+		<li class="dashboard-post-item" dashboard-id="<%= _.escape( id ) %>"><!-- OK -->
+			<div class="image-wrapper">
+				<img src="<%= _.escape( image_url ) %>" class="dashboard-image"><!-- OK -->
+			</div>
+		</li>
+	</script>
+<?php
+}
+
+function print_foo() {
+?>
+	<script id="template" type="text/template">
+		var compiled = _.template("<% print('Hello ' + _.escape(epithet)); %>"); /* OK */
+		var compiled = _.template("<% print('Hello ' + epithet); %>"); /* NOK */
+		var compiled = _.template("<% __p+=o.text %>"); /* NOK */
+
+		compiled({epithet: "stooge"});
+	</script>
+<?php
+}