Skip to content

Commit a83f273

Browse files
authored
Merge pull request #842 from Automattic/feature/security-mustache-sniff-improvements
2 parents 714bc00 + 0aebe1f commit a83f273

File tree

3 files changed

+57
-12
lines changed

3 files changed

+57
-12
lines changed

WordPressVIPMinimum/Sniffs/Security/MustacheSniff.php

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
namespace WordPressVIPMinimum\Sniffs\Security;
1010

11+
use PHP_CodeSniffer\Util\Tokens;
1112
use WordPressVIPMinimum\Sniffs\Sniff;
1213

1314
/**
@@ -28,12 +29,10 @@ class MustacheSniff extends Sniff {
2829
* @return array<int|string>
2930
*/
3031
public function register() {
31-
return [
32-
T_CONSTANT_ENCAPSED_STRING,
33-
T_STRING,
34-
T_INLINE_HTML,
35-
T_HEREDOC,
36-
];
32+
$targets = Tokens::$textStringTokens;
33+
$targets[ T_STRING ] = T_STRING;
34+
35+
return $targets;
3736
}
3837

3938
/**
@@ -57,15 +56,22 @@ public function process_token( $stackPtr ) {
5756
$this->phpcsFile->addWarning( $message, $stackPtr, 'VariableNotation' );
5857
}
5958

60-
if ( strpos( $this->tokens[ $stackPtr ]['content'], '{{=' ) !== false ) {
59+
$start_delimiter_change = strpos( $this->tokens[ $stackPtr ]['content'], '{{=' );
60+
if ( $start_delimiter_change !== false ) {
6161
// Mustache delimiter change.
62-
$new_delimiter = trim( str_replace( [ '{{=', '=}}' ], '', substr( $this->tokens[ $stackPtr ]['content'], 0, strpos( $this->tokens[ $stackPtr ]['content'], '=}}' ) + 3 ) ) );
63-
$message = 'Found Mustache delimiter change notation. New delimiter is: %s.';
64-
$data = [ $new_delimiter ];
65-
$this->phpcsFile->addWarning( $message, $stackPtr, 'DelimiterChange', $data );
62+
$end_delimiter_change = strpos( $this->tokens[ $stackPtr ]['content'], '=}}' );
63+
if ( $end_delimiter_change !== false && $start_delimiter_change < $end_delimiter_change ) {
64+
$start_new_delimiter = $start_delimiter_change + 3;
65+
$new_delimiter_length = $end_delimiter_change - ( $start_delimiter_change + 3 );
66+
$new_delimiter = substr( $this->tokens[ $stackPtr ]['content'], $start_new_delimiter, $new_delimiter_length );
67+
68+
$message = 'Found Mustache delimiter change notation. New delimiter is: %s.';
69+
$data = [ $new_delimiter ];
70+
$this->phpcsFile->addWarning( $message, $stackPtr, 'DelimiterChange', $data );
71+
}
6672
}
6773

68-
if ( strpos( $this->tokens[ $stackPtr ]['content'], 'SafeString' ) !== false ) {
74+
if ( strpos( $this->tokens[ $stackPtr ]['content'], '.SafeString' ) !== false ) {
6975
// Handlebars.js Handlebars.SafeString does not get escaped.
7076
$message = 'Found Handlebars.SafeString call which does not get escaped.';
7177
$this->phpcsFile->addWarning( $message, $stackPtr, 'SafeString' );

WordPressVIPMinimum/Tests/Security/MustacheUnitTest.inc

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,34 @@ echo '<a href="{{href}}">{{&data}}</div></a>'; // NOK: data.
2121

2222
// Issue 541#issuecomment-1692323177: don't flag GB syntax.
2323
<div class="wp-block-group"><!-- wp:heading {"textAlign":"center","style":{"spacing":{"margin":{"top":"0","right":"0","bottom":"var:preset|spacing|medium","left":"0"}}}} --><!-- OK. -->
24+
25+
<?php
26+
echo "<a href='${foo->{$baz}}'>{{{data}}}</div></a>"; // NOK: data.
27+
28+
echo <<<HERE
29+
<script type="text/html" id="tmpl-example">
30+
<a href="${(href)}">{{{data}}}</div></a><!-- NOK: data. -->
31+
<a href="{{href}}">{{&data}}</div></a><!-- NOK: data. -->
32+
HERE;
33+
echo <<<"HERE"
34+
{{=<% %>=}} <!-- NOK: delimiter change -->
35+
</script>
36+
HERE;
37+
38+
echo <<<'NOW'
39+
<script type="text/html" id="tmpl-example">
40+
<a href="{{href}}">{{{data}}}</div></a><!-- NOK: data. -->
41+
<a href="{{href}}">{{&data}}</div></a><!-- NOK: data. -->
42+
{{=<% %>=}} <!-- NOK: delimiter change -->
43+
</script>
44+
NOW;
45+
46+
// Don't throw false positives on incorrect/incomplete mustache delimiter change.
47+
$m = '{{=<%'; // Missing end of delimiter change.
48+
$m = '%>=}} {{=<%'; // Incorrect order, not a delimiter change.
49+
50+
// Correctly recognize mid-line delimiter change.
51+
$m = '{{default_tags}} {{=<% %>=}} <% erb_style_tags %> <%={{ }}=%> {{ default_tags_again }}'; // NOK: delimiter change.
52+
53+
// Prevent false positives on SafeString being a partial name.
54+
echo 'MySafeString';

WordPressVIPMinimum/Tests/Security/MustacheUnitTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,14 @@ public function getWarningList() {
3838
7 => 1,
3939
8 => 1,
4040
18 => 1,
41+
26 => 1,
42+
30 => 1,
43+
31 => 1,
44+
34 => 1,
45+
40 => 1,
46+
41 => 1,
47+
42 => 1,
48+
51 => 1,
4149
];
4250
}
4351
}

0 commit comments

Comments
 (0)