Skip to content

Commit 801fb45

Browse files
authored
Merge pull request #258 from rebeccahum/rebecca/windowbom_sniff
Add Window object Sniff for JS
2 parents 3b1b95e + 3235622 commit 801fb45

File tree

4 files changed

+242
-0
lines changed

4 files changed

+242
-0
lines changed

WordPress-VIP-Go/ruleset.xml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@
8787
<type>error</type>
8888
<severity>5</severity>
8989
</rule>
90+
<rule ref="WordPressVIPMinimum.JS.Window">
91+
<severity>5</severity>
92+
</rule>
9093
<rule ref="WordPressVIPMinimum.JS.InnerHTML.innerHTML">
9194
<type>error</type>
9295
<severity>5</severity>
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
<?php
2+
/**
3+
* WordPressVIPMinimum_Sniffs_JS_WindowSniff.
4+
*
5+
* @package VIPCS\WordPressVIPMinimum
6+
*/
7+
8+
namespace WordPressVIPMinimum\Sniffs\JS;
9+
10+
use PHP_CodeSniffer\Files\File;
11+
use PHP_CodeSniffer\Sniffs\Sniff;
12+
use PHP_CodeSniffer\Util\Tokens;
13+
14+
/**
15+
* WordPressVIPMinimum_Sniffs_JS_WindowSniff.
16+
*
17+
* Looks for instances of window properties that should be flagged.
18+
*
19+
* @package VIPCS\WordPressVIPMinimum
20+
*/
21+
class WindowSniff implements Sniff {
22+
23+
/**
24+
* A list of tokenizers this sniff supports.
25+
*
26+
* @var array
27+
*/
28+
public $supportedTokenizers = array(
29+
'JS',
30+
);
31+
32+
/**
33+
* Returns an array of tokens this test wants to listen for.
34+
*
35+
* @return array
36+
*/
37+
public function register() {
38+
return [
39+
T_STRING,
40+
];
41+
}
42+
43+
/**
44+
* List of window properties that need to be flagged.
45+
*
46+
* @var array
47+
*/
48+
private $windowProperties = [
49+
'location' => [
50+
'href' => true,
51+
'protocol' => true,
52+
'host' => true,
53+
'hostname' => true,
54+
'pathname' => true,
55+
'protocol' => true,
56+
'search' => true,
57+
'hash' => true,
58+
'username' => true,
59+
'port' => true,
60+
'password' => true,
61+
],
62+
'name' => true,
63+
'status' => true,
64+
];
65+
66+
/**
67+
* A list of tokens that are allowed in the syntax.
68+
*
69+
* @var array
70+
*/
71+
private $syntaxTokens = [
72+
T_OBJECT_OPERATOR,
73+
];
74+
75+
/**
76+
* Processes this test, when one of its tokens is encountered.
77+
*
78+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
79+
* @param int $stackPtr The position of the current token in the
80+
* stack passed in $tokens.
81+
*
82+
* @return void
83+
*/
84+
public function process( File $phpcsFile, $stackPtr ) {
85+
$tokens = $phpcsFile->getTokens();
86+
87+
if ( 'window' !== $tokens[ $stackPtr ]['content'] ) {
88+
// Doesn't begin with 'window', bail.
89+
return;
90+
}
91+
92+
$nextTokenPtr = $phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true, null, true );
93+
$nextToken = $tokens[ $nextTokenPtr ]['code'];
94+
if ( T_OBJECT_OPERATOR !== $nextToken && T_OPEN_SQUARE_BRACKET !== $nextToken ) {
95+
// No . or [' next, bail.
96+
return;
97+
}
98+
99+
$nextNextTokenPtr = $phpcsFile->findNext( Tokens::$emptyTokens, ( $nextTokenPtr + 1 ), null, true, null, true );
100+
if ( false === $nextNextTokenPtr ) {
101+
// Something went wrong, bail.
102+
return;
103+
}
104+
105+
$nextNextToken = str_replace( [ '"', "'" ], '', $tokens[ $nextNextTokenPtr ]['content'] );
106+
if ( ! isset( $this->windowProperties[ $nextNextToken ] ) ) {
107+
// Not in $windowProperties, bail.
108+
return;
109+
}
110+
111+
$nextNextNextTokenPtr = $phpcsFile->findNext( array_merge( [ T_CLOSE_SQUARE_BRACKET ], Tokens::$emptyTokens ), ( $nextNextTokenPtr + 1 ), null, true, null, true );
112+
$nextNextNextToken = $tokens[ $nextNextNextTokenPtr ]['code'];
113+
114+
$nextNextNextNextToken = false;
115+
if ( T_OBJECT_OPERATOR === $nextNextNextToken || T_OPEN_SQUARE_BRACKET === $nextNextNextToken ) {
116+
$nextNextNextNextTokenPtr = $phpcsFile->findNext( Tokens::$emptyTokens, ( $nextNextNextTokenPtr + 1 ), null, true, null, true );
117+
if ( false === $nextNextNextNextTokenPtr ) {
118+
// Something went wrong, bail.
119+
return;
120+
}
121+
122+
$nextNextNextNextToken = str_replace( [ '"', "'" ], '', $tokens[ $nextNextNextNextTokenPtr ]['content'] );
123+
if ( ! isset( $this->windowProperties[ $nextNextToken ][ $nextNextNextNextToken ] ) ) {
124+
// Not in $windowProperties, bail.
125+
return;
126+
}
127+
}
128+
129+
$windowProperty = 'window.';
130+
$windowProperty .= $nextNextNextNextToken ? $nextNextToken . '.' . $nextNextNextNextToken : $nextNextToken;
131+
132+
$prevTokenPtr = $phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true, null, true );
133+
if ( T_EQUAL === $tokens[ $prevTokenPtr ]['code'] ) {
134+
// Variable assignment.
135+
$phpcsFile->addWarning( sprintf( 'Data from JS global "' . $windowProperty . '" may contain user-supplied values and should be checked.', $tokens[ $stackPtr ]['content'] ), $stackPtr, 'VarAssignment' );
136+
return;
137+
}
138+
139+
$phpcsFile->addError( sprintf( 'Data from JS global "' . $windowProperty . '" may contain user-supplied values and should be sanitized before output to prevent XSS.', $tokens[ $stackPtr ]['content'] ), $stackPtr, $nextNextToken );
140+
}
141+
142+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
var testString = '?url=www.evil.com';
2+
var url = "javascript:alert( 'xss' )";
3+
4+
window; // Ok.
5+
window.location.testing; // Ok.
6+
window.test = 'http://www.google.com' + testString; // Ok.
7+
const foo = window.location.testing; // Ok.
8+
9+
var bar = window.location.href; // Warning - variable assignment.
10+
var c=window.location.hash.split("#")[0]; // Warning - variable assignment.
11+
12+
window.location; // Error.
13+
window.name; // Error.
14+
window.status; // Error.
15+
window.location.href = 'http://www.google.com/' + testString; // Error.
16+
window.location.protocol; // Error.
17+
window.location.host; // Error.
18+
window.location.hostname; // Error.
19+
window.location.pathname; // Error.
20+
window.location.search; // Error.
21+
window.location.hash; // Error.
22+
window.location.username; // Error.
23+
window.location.password; // Error.
24+
window.location.port; // Error.
25+
26+
document.theform.reference.onchange = function( url ) {
27+
var id = document.theform.reference.selectedIndex;
28+
window.name = url; // Error.
29+
}
30+
31+
window['location']; // Error.
32+
window.location['href']; // Error.
33+
window['location']['protocol']; // Error.
34+
window['location'].host; // Error.
35+
36+
window['location']['test']; // Ok.
37+
window.location['test']; // Ok.
38+
window['test'].location; // Ok.
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php
2+
/**
3+
* Unit test class for WordPressVIPMinimum Coding Standard.
4+
*
5+
* @package VIPCS\WordPressVIPMinimum
6+
*/
7+
8+
namespace WordPressVIPMinimum\Tests\JS;
9+
10+
use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
11+
12+
/**
13+
* Unit test class for the HTML String concatenation in JS sniff.
14+
*
15+
* @package VIPCS\WordPressVIPMinimum
16+
*/
17+
class WindowUnitTest extends AbstractSniffUnitTest {
18+
19+
/**
20+
* Returns the lines where errors should occur.
21+
*
22+
* @return array <int line number> => <int number of errors>
23+
*/
24+
public function getErrorList() {
25+
return [
26+
12 => 1,
27+
13 => 1,
28+
14 => 1,
29+
15 => 1,
30+
16 => 1,
31+
17 => 1,
32+
18 => 1,
33+
19 => 1,
34+
20 => 1,
35+
21 => 1,
36+
22 => 1,
37+
23 => 1,
38+
24 => 1,
39+
28 => 1,
40+
31 => 1,
41+
32 => 1,
42+
33 => 1,
43+
34 => 1,
44+
];
45+
}
46+
47+
/**
48+
* Returns the lines where warnings should occur.
49+
*
50+
* @return array <int line number> => <int number of warnings>
51+
*/
52+
public function getWarningList() {
53+
return [
54+
9 => 1,
55+
10 => 1,
56+
];
57+
}
58+
59+
}

0 commit comments

Comments
 (0)