Skip to content

Commit b417db3

Browse files
committed
WP/AlternativeFunctions: allow for more input streams with file related functions
Similar to 1649 which allowed for using `php://input` with `file_get_contents()` and surprisingly inspired by the closing of issue 295. This PR expands on the earlier work done in relation to the PHP native input streams by: * recognizing more PHP native input streams; * recognizing the PHP input stream constants; * allowing for these in a number of the `file_system_read` group functions as well as for the `file_get_contents` function. Refs: * http://php.net/manual/en/wrappers.php.php * http://php.net/manual/en/features.commandline.io-streams.php Includes unit tests. Related 1649 Related 295 Notes: * I have not checked the WP FileSystem to see if it even could handle these input streams. If it can, we may need to discuss what is the preferred option in that case. Personally, this to me seems like something for which the WP FileSystem would be overkill/superfluous. * At a later point in time, the new method + properties could be a candidate for moving to `Sniff` or a separate utility class. As no other sniffs currently need them though, this is not necessary at this moment and could possible be combined with/actioned when 1465 comes into play.
1 parent d0717e7 commit b417db3

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)