Skip to content

Commit 09d5b92

Browse files
committed
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.
1 parent d9317e5 commit 09d5b92

File tree

4 files changed

+56
-13
lines changed

4 files changed

+56
-13
lines changed

WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ class UnderscorejsSniff extends Sniff {
2626
*/
2727
const UNESCAPED_INTERPOLATE_REGEX = '`<%=\s*(?:.+?%>|$)`';
2828

29+
/**
30+
* Regex to match execute notations containing a print command
31+
* and retrieve a code snippet.
32+
*
33+
* @var string
34+
*/
35+
const UNESCAPED_PRINT_REGEX = '`<%\s*(?:print\s*\(.+?\)\s*;|__p\s*\+=.+?)\s*%>`';
36+
2937
/**
3038
* Regex to match the "interpolate" keyword when used to overrule the ERB-style delimiters.
3139
*
@@ -112,7 +120,8 @@ public function process_token( $stackPtr ) {
112120
return;
113121
}
114122

115-
$content = $this->strip_quotes( $this->tokens[ $stackPtr ]['content'] );
123+
$content = $this->strip_quotes( $this->tokens[ $stackPtr ]['content'] );
124+
116125
$match_count = preg_match_all( self::UNESCAPED_INTERPOLATE_REGEX, $content, $matches );
117126
if ( $match_count > 0 ) {
118127
foreach ( $matches[0] as $match ) {
@@ -127,6 +136,20 @@ public function process_token( $stackPtr ) {
127136
}
128137
}
129138

139+
$match_count = preg_match_all( self::UNESCAPED_PRINT_REGEX, $content, $matches );
140+
if ( $match_count > 0 ) {
141+
foreach ( $matches[0] as $match ) {
142+
if ( strpos( $match, '_.escape(' ) !== false ) {
143+
continue;
144+
}
145+
146+
// Underscore.js unescaped output.
147+
$message = 'Found Underscore.js unescaped print execution: "%s".';
148+
$data = [ $match ];
149+
$this->phpcsFile->addWarning( $message, $stackPtr, 'PrintExecution', $data );
150+
}
151+
}
152+
130153
if ( $this->phpcsFile->tokenizerType !== 'JS'
131154
&& preg_match( self::INTERPOLATE_KEYWORD_REGEX, $content ) > 0
132155
) {

WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.inc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,15 @@ function display_foo {
106106
</script>
107107
<?php
108108
}
109+
110+
function print_foo() {
111+
?>
112+
<script id="template" type="text/template">
113+
var compiled = _.template("<% print('Hello ' + _.escape(epithet)); %>"); /* OK */
114+
var compiled = _.template("<% print('Hello ' + epithet); %>"); /* NOK */
115+
var compiled = _.template("<% __p+=o.text %>"); /* NOK */
116+
117+
compiled({epithet: "stooge"});
118+
</script>
119+
<?php
120+
}

WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,7 @@ var html = _.template(
3939
variable: "o"
4040
}
4141
);
42+
43+
var compiled = _.template("<% print('Hello ' + _.escape(epithet)); %>"); /* OK */
44+
var compiled = _.template("<% print('Hello ' + epithet); %>"); /* NOK */
45+
var compiled = _.template("<% __p+=o.text %>"); /* NOK */

WordPressVIPMinimum/Tests/Security/UnderscorejsUnitTest.php

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,20 @@ public function getWarningList( $testFile = '' ) {
5353
switch ( $testFile ) {
5454
case 'UnderscorejsUnitTest.inc':
5555
return [
56-
6 => 1,
57-
14 => 1,
58-
22 => 1,
59-
23 => 1,
60-
28 => 1,
61-
32 => 1,
62-
38 => 3,
63-
45 => 1,
64-
46 => 1,
65-
47 => 1,
66-
58 => 1,
67-
60 => 1,
56+
6 => 1,
57+
14 => 1,
58+
22 => 1,
59+
23 => 1,
60+
28 => 1,
61+
32 => 1,
62+
38 => 3,
63+
45 => 1,
64+
46 => 1,
65+
47 => 1,
66+
58 => 1,
67+
60 => 1,
68+
114 => 1,
69+
115 => 1,
6870
];
6971

7072
case 'UnderscorejsUnitTest.js':
@@ -74,6 +76,8 @@ public function getWarningList( $testFile = '' ) {
7476
7 => 1,
7577
9 => 1,
7678
12 => 1,
79+
44 => 1,
80+
45 => 1,
7781
];
7882

7983
default:

0 commit comments

Comments
 (0)