From 1b8c9b9327001d95e47401858e43052082f41188 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 20 Oct 2020 15:31:08 +0200 Subject: [PATCH 1/8] Security/Underscorejs: sniff all text string tokens So far, only single quoted text strings, heredocs and inline HTML were sniffed. However, there are two more text string token types: nowdoc and double quoted strings. This adds these tokens to the `register()` method to be sniffed. This commit also ensures that there is at least one unit test in place for each of these token types and that the tests use a variety of whitespace. --- .../Sniffs/Security/UnderscorejsSniff.php | 11 +++--- .../Tests/Security/UnderscorejsUnitTest.inc | 34 ++++++++++++++++++- .../Tests/Security/UnderscorejsUnitTest.php | 8 +++++ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php index fb1c0af0..0c6551b3 100644 --- a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php @@ -8,6 +8,7 @@ namespace WordPressVIPMinimum\Sniffs\Security; +use PHP_CodeSniffer\Util\Tokens; use WordPressVIPMinimum\Sniffs\Sniff; /** @@ -30,12 +31,10 @@ 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; + + return $targets; } /** diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc index 2e8b7cd6..a63efb3a 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc @@ -12,6 +12,38 @@ EOT; + +" data-origsrc="<%- data.originalSrc %>">'. // NOK x 1. + '<%=data.alt%>'. // NOK x 1. + ''; +} + +function single_quoted_string_with_concatenation( $data ) { + echo '<%- data.alt %>'; // NOK x 1. +} + +function double_quoted_string( $name, $value, $is_template ) { + echo $is_template ? "<%={$name}%>" : esc_attr( $value ); // NOK. +} + +$nowdoc = <<<'EOT' + +EOT; + +$heredoc = << + <%= name %> + +EOD; diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php index 1043c3e8..28b6b143 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php @@ -36,6 +36,14 @@ public function getWarningList() { return [ 6 => 1, 14 => 1, + 22 => 1, + 23 => 1, + 28 => 1, + 32 => 1, + 38 => 1, + 45 => 1, + 46 => 1, + 47 => 1, ]; } From 3b18b7726303ebf5664798707cdf21e4e7d7ad01 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 20 Oct 2020 16:21:12 +0200 Subject: [PATCH 2/8] Security/Underscorejs: throw an error for each violation So far, the sniff would throw one error per text string, independently of how often the unescaped output notation would occur in the text string. While for simple text strings, this is not much of an issue, for long and complex text strings, it may be more difficult to spot all the unescaped output notations in the text string. This changes the sniff to: * Throw a warning for each occurrence of the unescaped output notation in a text string. * Include a snippet from the text string with the details of the unescaped output notation. Old: `Found Underscore.js unescaped output notation: "<%=".` New: `Found Underscore.js unescaped output notation: "<%= post_url %>".` To allow the sniff to also match on a text string being concatenated together, the regex has to also match when `<%=` occurs at the end of the text string and will only display `<%=` in that case. To this end, we also need to remove the text string quotes (if they exist) from the token content. --- .../Sniffs/Security/UnderscorejsSniff.php | 23 +++++++++++++++---- .../Tests/Security/UnderscorejsUnitTest.php | 2 +- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php index 0c6551b3..05065104 100644 --- a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php @@ -18,6 +18,14 @@ */ 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*(?:.+?%>|$)`'; + /** * A list of tokenizers this sniff supports. * @@ -46,13 +54,18 @@ public function register() { */ public function process_token( $stackPtr ) { - if ( strpos( $this->tokens[ $stackPtr ]['content'], '<%=' ) !== false ) { - // Underscore.js unescaped output. - $message = 'Found Underscore.js unescaped output notation: "<%=".'; - $this->phpcsFile->addWarning( $message, $stackPtr, 'OutputNotation' ); + $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 ) { + // Underscore.js unescaped output. + $message = 'Found Underscore.js unescaped output notation: "%s".'; + $data = [ $match ]; + $this->phpcsFile->addWarning( $message, $stackPtr, 'OutputNotation', $data ); + } } - if ( strpos( $this->tokens[ $stackPtr ]['content'], 'interpolate' ) !== false ) { + if ( strpos( $content, 'interpolate' ) !== false ) { // Underscore.js unescaped output. $message = 'Found Underscore.js delimiter change notation.'; $this->phpcsFile->addWarning( $message, $stackPtr, 'InterpolateFound' ); diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php index 28b6b143..4c77e2d8 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php @@ -40,7 +40,7 @@ public function getWarningList() { 23 => 1, 28 => 1, 32 => 1, - 38 => 1, + 38 => 3, 45 => 1, 46 => 1, 47 => 1, From ece1afd6d0d9ec1033370382313b060e4c0d4c4b Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 20 Oct 2020 12:56:19 +0200 Subject: [PATCH 3/8] Security/Underscorejs: add JS test case file Add a basic JS test case file. Tests for `interpolate` will be added in a follow-up commit. --- .../Tests/Security/UnderscorejsUnitTest.js | 6 +++ .../Tests/Security/UnderscorejsUnitTest.php | 40 +++++++++++++------ 2 files changed, 33 insertions(+), 13 deletions(-) create mode 100644 WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.js diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.js b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.js new file mode 100644 index 00000000..9afbcdde --- /dev/null +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.js @@ -0,0 +1,6 @@ + +var html = _.template('
  • <%- name %>
    • ', { name: 'John Smith' }); // OK. + +var html = _.template('
  • <%= name %>
    • ', { name: 'John Smith' }); // NOK. +var html = _.template('
  • <%=type.item%>
    • ', { name: 'John Smith' }); // NOK. + diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php index 4c77e2d8..821ed42d 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php @@ -30,21 +30,35 @@ public function getErrorList() { /** * Returns the lines where warnings should occur. * + * @param string $testFile The name of the file being tested. + * * @return array => */ - public function getWarningList() { - return [ - 6 => 1, - 14 => 1, - 22 => 1, - 23 => 1, - 28 => 1, - 32 => 1, - 38 => 3, - 45 => 1, - 46 => 1, - 47 => 1, - ]; + public function getWarningList( $testFile = '' ) { + switch ( $testFile ) { + case 'UnderscorejsUnitTest.inc': + return [ + 6 => 1, + 14 => 1, + 22 => 1, + 23 => 1, + 28 => 1, + 32 => 1, + 38 => 3, + 45 => 1, + 46 => 1, + 47 => 1, + ]; + + case 'UnderscorejsUnitTest.js': + return [ + 4 => 1, + 5 => 1, + ]; + + default: + return []; + } } } From 70ec8114d7f099547a732c5367ee49123ef6a757 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 20 Oct 2020 17:42:57 +0200 Subject: [PATCH 4/8] Security/Underscorejs: improve check for `interpolate` in JS files The `interpolate` property in JS can be set using various syntaxes. While the current check will in no way catch them all, the changes now made should improve the accuracy of detecting a change in the delimiter via `interpolate` in JS files. Notes: * Add checking for `T_STRING` to detect `_.templateSettings.interpolate =` syntax. This was previously not detected (false negative). * Add some defensive coding to prevent false positives when the `interpolate` keyword is used in another context than underscorejs. * Limit the check for `T_STRING` and `T_PROPERTY` to Javascript files only to prevent false positives when scanning PHP files. * Limit the check for `interpolate` in text strings to PHP only. This prevents false positives when the keyword is used in a text string in JS. Includes unit tests covering all the above. --- .../Sniffs/Security/UnderscorejsSniff.php | 47 ++++++++++++++++++- .../Tests/Security/UnderscorejsUnitTest.inc | 3 ++ .../Tests/Security/UnderscorejsUnitTest.js | 20 ++++++++ .../Tests/Security/UnderscorejsUnitTest.php | 7 ++- 4 files changed, 73 insertions(+), 4 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php index 05065104..8e81e602 100644 --- a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php @@ -41,6 +41,7 @@ class UnderscorejsSniff extends Sniff { public function register() { $targets = Tokens::$textStringTokens; $targets[] = T_PROPERTY; + $targets[] = T_STRING; return $targets; } @@ -53,6 +54,46 @@ public function register() { * @return void */ public function process_token( $stackPtr ) { + /* + * 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 ); @@ -65,8 +106,10 @@ public function process_token( $stackPtr ) { } } - if ( strpos( $content, 'interpolate' ) !== false ) { - // Underscore.js unescaped output. + if ( $this->phpcsFile->tokenizerType !== 'JS' + && strpos( $content, 'interpolate' ) !== false + ) { + // Underscore.js delimiter change. $message = 'Found Underscore.js delimiter change notation.'; $this->phpcsFile->addWarning( $message, $stackPtr, 'InterpolateFound' ); } diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc index a63efb3a..5412770b 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc @@ -47,3 +47,6 @@ $heredoc = << EOD; + +// Make sure the JS specific check does not trigger on PHP code. +$obj->interpolate = true; diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.js b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.js index 9afbcdde..b0dca205 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.js +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.js @@ -4,3 +4,23 @@ var html = _.template('
  • <%- name %>
    • ', { name: 'John Smith' }); // OK. var html = _.template('
  • <%= name %>
    • ', { name: 'John Smith' }); // NOK. var html = _.template('
  • <%=type.item%>
    • ', { name: 'John Smith' }); // NOK. +_.templateSettings.interpolate = /\{\{(.+?)\}\}/g; /* NOK */ +_.templateSettings = { + interpolate: /\{\{(.+?)\}\}/g /* NOK */ +}; + +options.interpolate=_.templateSettings.interpolate; /* NOK */ +var interpolate = options.interpolate || reNoMatch, /* Ignore */ + source = "__p += '"; + +var template = _.template('
  • {{ name }}
    • '); /* NOK, due to the interpolate, but not flagged. */ + +// 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. diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php index 821ed42d..8cc34042 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php @@ -52,8 +52,11 @@ public function getWarningList( $testFile = '' ) { case 'UnderscorejsUnitTest.js': return [ - 4 => 1, - 5 => 1, + 4 => 1, + 5 => 1, + 7 => 1, + 9 => 1, + 12 => 1, ]; default: From d3ce1d03c58d72a83ffeb482ba0a283c1b0262a1 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 21 Oct 2020 04:00:31 +0200 Subject: [PATCH 5/8] Security/Underscorejs: improve check for `interpolate` in PHP files Match the `interpolate` property in PHP files with the similar precision as in JS files. Includes unit tests. --- .../Sniffs/Security/UnderscorejsSniff.php | 9 +++++++- .../Tests/Security/UnderscorejsUnitTest.inc | 23 +++++++++++++++++++ .../Tests/Security/UnderscorejsUnitTest.php | 2 ++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php index 8e81e602..33f9f4a7 100644 --- a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php @@ -26,6 +26,13 @@ class UnderscorejsSniff extends Sniff { */ const UNESCAPED_INTERPOLATE_REGEX = '`<%=\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. * @@ -107,7 +114,7 @@ public function process_token( $stackPtr ) { } if ( $this->phpcsFile->tokenizerType !== 'JS' - && strpos( $content, 'interpolate' ) !== false + && preg_match( self::INTERPOLATE_KEYWORD_REGEX, $content ) > 0 ) { // Underscore.js delimiter change. $message = 'Found Underscore.js delimiter change notation.'; diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc index 5412770b..48ff5ea6 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc @@ -50,3 +50,26 @@ 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() { + ?> + + 1, 46 => 1, 47 => 1, + 58 => 1, + 60 => 1, ]; case 'UnderscorejsUnitTest.js': From 1ab279496f6eb4a634163514406f5f97816a8c30 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 21 Oct 2020 04:26:10 +0200 Subject: [PATCH 6/8] Security/Underscorejs: bug fix - don't error when variable is escaped Prevent triggering a warning when the variable being printed is escaped using `_.escape()`. Includes unit tests. Fixes 345 --- .../Sniffs/Security/UnderscorejsSniff.php | 4 +++ .../Tests/Security/UnderscorejsUnitTest.inc | 33 +++++++++++++++++++ .../Tests/Security/UnderscorejsUnitTest.js | 15 +++++++++ 3 files changed, 52 insertions(+) diff --git a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php index 33f9f4a7..2bd7da70 100644 --- a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php @@ -106,6 +106,10 @@ public function process_token( $stackPtr ) { $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 ]; diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc index 48ff5ea6..f3103f02 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc @@ -73,3 +73,36 @@ function test_interpolate_match_precision() { <%= _.escape(name) %>', { name: 'John Smith' }); // OK. + + var html = _.template( + "
    The \"<% __p+=_.escape(o.text) %>\" is the same
    " + // OK.
+				"as the  \"<%= _.escape(o.text) %>\" and the same
    " + // OK.
+				"as the \"<%- o.text %>\"
    ", // OK. + { + text: "some text and \n it's a line break" + }, + { + variable: "o" + } + ); +EOD; + + echo $script; +} + +function display_foo { +?> + +<%= _.escape(name) %>', { name: 'John Smith' }); // OK. + +var html = _.template( + "
    The \"<% __p+=_.escape(o.text) %>\" is the same
    " + // OK.
+		"as the  \"<%= _.escape(o.text) %>\" and the same
    " + // OK.
+		"as the \"<%- o.text %>\"
    ", // OK. + { + text: "some text and \n it's a line break" + }, + { + variable: "o" + } +); From d9317e5ab9d661c123812205f9480e47e77b1544 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 31 Oct 2020 19:01:07 +0100 Subject: [PATCH 7/8] Security/Underscorejs: ignore Gruntfile.js files These are configuration files and not part of the production code. This check does not verify whether this file is in the project root as we don't know what the project root is. It will plainly ignore any file called `Gruntfile.js` in a case-insensitive manner. Includes unit tests. --- .../Sniffs/Security/UnderscorejsSniff.php | 10 ++ .../Tests/Security/Gruntfile.js | 100 ++++++++++++++++++ .../Tests/Security/UnderscorejsUnitTest.php | 15 +++ 3 files changed, 125 insertions(+) create mode 100644 WordPressVIPMinimum/Tests/Security/Gruntfile.js diff --git a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php index 2bd7da70..f5459e72 100644 --- a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php @@ -61,6 +61,16 @@ 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. */ diff --git a/WordPressVIPMinimum/Tests/Security/Gruntfile.js b/WordPressVIPMinimum/Tests/Security/Gruntfile.js new file mode 100644 index 00000000..b423d7af --- /dev/null +++ b/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 .zip and -.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' ] ); +}; diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php index 21b4f70e..01745499 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php @@ -18,6 +18,21 @@ */ class UnderscorejsUnitTest extends AbstractSniffUnitTest { + /** + * Get a list of all test files to check. + * + * @param string $testFileBase The base path that the unit tests files will have. + * + * @return string[] + */ + protected function getTestFiles( $testFileBase ) { + return [ + $testFileBase . 'inc', + $testFileBase . 'js', + __DIR__ . DIRECTORY_SEPARATOR . 'Gruntfile.js', + ]; + } + /** * Returns the lines where errors should occur. * From 09d5b924421a19204f2b84d50b8afa6cae570c8c Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 1 Nov 2020 02:37:49 +0100 Subject: [PATCH 8/8] Security/Underscorejs: enhancement - check for print execution statements Add a new check for the JS native `print` command and the `_p+=` variation, when used without being combined with `_.escape()`. Includes unit tests. --- .../Sniffs/Security/UnderscorejsSniff.php | 25 ++++++++++++++++- .../Tests/Security/UnderscorejsUnitTest.inc | 12 ++++++++ .../Tests/Security/UnderscorejsUnitTest.js | 4 +++ .../Tests/Security/UnderscorejsUnitTest.php | 28 +++++++++++-------- 4 files changed, 56 insertions(+), 13 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php index f5459e72..6ea36135 100644 --- a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php @@ -26,6 +26,14 @@ class UnderscorejsSniff extends Sniff { */ 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. * @@ -112,7 +120,8 @@ public function process_token( $stackPtr ) { return; } - $content = $this->strip_quotes( $this->tokens[ $stackPtr ]['content'] ); + $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 ) { @@ -127,6 +136,20 @@ public function process_token( $stackPtr ) { } } + $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; + } + + // Underscore.js unescaped output. + $message = 'Found Underscore.js unescaped print execution: "%s".'; + $data = [ $match ]; + $this->phpcsFile->addWarning( $message, $stackPtr, 'PrintExecution', $data ); + } + } + if ( $this->phpcsFile->tokenizerType !== 'JS' && preg_match( self::INTERPOLATE_KEYWORD_REGEX, $content ) > 0 ) { diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc index f3103f02..addf103d 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc @@ -106,3 +106,15 @@ function display_foo { + +"); /* OK */ +var compiled = _.template("<% print('Hello ' + epithet); %>"); /* NOK */ +var compiled = _.template("<% __p+=o.text %>"); /* NOK */ diff --git a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php index 01745499..0c0e08c5 100644 --- a/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php @@ -53,18 +53,20 @@ public function getWarningList( $testFile = '' ) { switch ( $testFile ) { case 'UnderscorejsUnitTest.inc': return [ - 6 => 1, - 14 => 1, - 22 => 1, - 23 => 1, - 28 => 1, - 32 => 1, - 38 => 3, - 45 => 1, - 46 => 1, - 47 => 1, - 58 => 1, - 60 => 1, + 6 => 1, + 14 => 1, + 22 => 1, + 23 => 1, + 28 => 1, + 32 => 1, + 38 => 3, + 45 => 1, + 46 => 1, + 47 => 1, + 58 => 1, + 60 => 1, + 114 => 1, + 115 => 1, ]; case 'UnderscorejsUnitTest.js': @@ -74,6 +76,8 @@ public function getWarningList( $testFile = '' ) { 7 => 1, 9 => 1, 12 => 1, + 44 => 1, + 45 => 1, ]; default: