Skip to content

Commit 95b904e

Browse files
authored
Merge pull request #1655 from WordPress-Coding-Standards/feature/295-alternativefunctions-recognizing-more-streams
WP/AlternativeFunctions: allow for more input streams with file related functions
2 parents d0717e7 + b417db3 commit 95b904e

File tree

3 files changed

+111
-13
lines changed

3 files changed

+111
-13
lines changed

WordPress/Sniffs/WP/AlternativeFunctionsSniff.php

Lines changed: 94 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,47 @@
2525
*/
2626
class AlternativeFunctionsSniff extends AbstractFunctionRestrictionsSniff {
2727

28+
/**
29+
* Local input streams which should not be flagged for the file system function checks.
30+
*
31+
* @link http://php.net/manual/en/wrappers.php.php
32+
*
33+
* @var array
34+
*/
35+
protected $allowed_local_streams = array(
36+
'php://input' => true,
37+
'php://output' => true,
38+
'php://stdin' => true,
39+
'php://stdout' => true,
40+
'php://stderr' => true,
41+
);
42+
43+
/**
44+
* Local input streams which should not be flagged for the file system function checks if
45+
* the $filename starts with them.
46+
*
47+
* @link http://php.net/manual/en/wrappers.php.php
48+
*
49+
* @var array
50+
*/
51+
protected $allowed_local_stream_partials = array(
52+
'php://temp/',
53+
'php://fd/',
54+
);
55+
56+
/**
57+
* Local input stream constants which should not be flagged for the file system function checks.
58+
*
59+
* @link http://php.net/manual/en/wrappers.php.php
60+
*
61+
* @var array
62+
*/
63+
protected $allowed_local_stream_constants = array(
64+
'STDIN' => true,
65+
'STDOUT' => true,
66+
'STDERR' => true,
67+
);
68+
2869
/**
2970
* Groups of functions to restrict.
3071
*
@@ -83,13 +124,13 @@ public function getGroups() {
83124
'since' => '2.5.0',
84125
'functions' => array(
85126
'readfile',
86-
'fopen',
87-
'fsockopen',
88-
'pfsockopen',
89127
'fclose',
128+
'fopen',
90129
'fread',
91130
'fwrite',
92131
'file_put_contents',
132+
'fsockopen',
133+
'pfsockopen',
93134
),
94135
),
95136

@@ -202,13 +243,34 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
202243
return;
203244
}
204245

205-
$raw_stripped = $this->strip_quotes( $params[1]['raw'] );
206-
if ( 'php://input' === $raw_stripped ) {
207-
// This is not a file, but the read-only raw data stream from the request body.
246+
if ( $this->is_local_data_stream( $params[1]['raw'] ) === true ) {
247+
// Local data stream.
208248
return;
209249
}
210250

211-
unset( $params, $raw_stripped );
251+
unset( $params );
252+
253+
break;
254+
255+
case 'readfile':
256+
case 'fopen':
257+
case 'file_put_contents':
258+
/*
259+
* Allow for handling raw data streams from the request body.
260+
*/
261+
$first_param = $this->get_function_call_parameter( $stackPtr, 1 );
262+
263+
if ( false === $first_param ) {
264+
// If the file to work with is not set, local data streams don't come into play.
265+
break;
266+
}
267+
268+
if ( $this->is_local_data_stream( $first_param['raw'] ) === true ) {
269+
// Local data stream.
270+
return;
271+
}
272+
273+
unset( $first_param );
212274

213275
break;
214276
}
@@ -223,4 +285,29 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
223285
}
224286
}
225287

288+
/**
289+
* Determine based on the "raw" parameter value, whether a file parameter points to
290+
* a local data stream.
291+
*
292+
* @param string $raw_param_value Raw parameter value.
293+
*
294+
* @return bool True if this is a local data stream. False otherwise.
295+
*/
296+
protected function is_local_data_stream( $raw_param_value ) {
297+
298+
$raw_stripped = $this->strip_quotes( $raw_param_value );
299+
if ( isset( $this->allowed_local_streams[ $raw_stripped ] )
300+
|| isset( $this->allowed_local_stream_constants[ $raw_param_value ] )
301+
) {
302+
return true;
303+
}
304+
305+
foreach ( $this->allowed_local_stream_partials as $partial ) {
306+
if ( strpos( $raw_stripped, $partial ) === 0 ) {
307+
return true;
308+
}
309+
}
310+
311+
return false;
312+
}
226313
}

WordPress/Tests/WP/AlternativeFunctionsUnitTest.inc

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,18 @@ file_get_contents( MUPLUGINDIR . 'some-file.xml' ); // OK.
5151
file_get_contents( plugin_dir_path( __FILE__ ) . 'subfolder/*.conf' ); // OK.
5252
file_get_contents(WP_Upload_Dir()['path'] . 'subdir/file.inc'); // OK.
5353
file_get_contents( 'php://input' ); // OK.
54+
55+
// Loosely related to issue 295.
56+
file_get_contents( 'php://stdin' ); // OK.
57+
$input_stream = fopen( 'php://stdin', 'w' ); // OK.
58+
$csv_ar = fopen(STDIN); // OK.
59+
60+
$output_stream = fopen( 'php://output', 'w' ); // OK.
61+
$output_stream = fopen( 'php://stdout', 'w' ); // OK.
62+
$output_stream = fopen( 'php://stderr', 'w' ); // OK.
63+
$output_stream = fopen( STDOUT, 'w' ); // OK.
64+
$output_stream = fopen( STDERR, 'w' ); // OK.
65+
$output_stream = fopen( 'php://fd/3', 'w' ); // OK.
66+
$fp = fopen("php://temp/maxmemory:$fiveMBs", 'r+'); // OK.
67+
readfile( 'php://filter/resource=http://www.example.com' ); // Warning.
68+
file_put_contents("php://filter/write=string.rot13/resource=example.txt","Hello World"); // Warning.

WordPress/Tests/WP/AlternativeFunctionsUnitTest.php

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,9 @@ public function getWarningList() {
4040
5 => 1,
4141
6 => 1,
4242
7 => 1,
43-
4443
10 => 1,
45-
4644
12 => 1,
47-
4845
14 => 1,
49-
5046
16 => 1,
5147
17 => 1,
5248
18 => 1,
@@ -60,13 +56,13 @@ public function getWarningList() {
6056
26 => 1,
6157
27 => 1,
6258
28 => 1,
63-
6459
40 => 1,
65-
6660
44 => 1,
6761
46 => 1,
6862
47 => 1,
6963
49 => 1,
64+
67 => 1,
65+
68 => 1,
7066
);
7167
}
7268

0 commit comments

Comments
 (0)