MDL-48980 Security: Always clean the result from min_get_slash_argument

The result from this function is used in send_file calls and if unclean
(windows dir separators, or .. path components) it could expose sensitive
files (e.g. .php files). Now we always clean the result from this function
even if it means double cleaning.

I also fixed the unit test for this function and added a new test for this cleaning.

I also updated the comments to point to get_file_argument as the full version of
min_get_slash_argument.
This commit is contained in:
Damyon Wiese 2015-01-27 23:16:10 +08:00 committed by David Monllao
parent e5d0b0492e
commit af9a7937cc
2 changed files with 18 additions and 6 deletions

View File

@ -171,15 +171,16 @@ function min_enable_zlib_compression() {
* @return string
*/
function min_get_slash_argument() {
// Note: This code has to work in the same cases as normal get_slash_argument(),
// Note: This code has to work in the same cases as normal get_file_argument(),
// but at the same time it may be simpler because we do not have to deal
// with encodings and other tricky stuff.
$relativepath = '';
if (!empty($_GET['file']) and strpos($_GET['file'], '/') === 0) {
// server is using url rewriting, most probably IIS
return $_GET['file'];
// Server is using url rewriting, most probably IIS.
// Always clean the result of this function as it may be used in unsafe calls to send_file.
return min_clean_param($_GET['file'], 'SAFEPATH');
} else if (stripos($_SERVER['SERVER_SOFTWARE'], 'iis') !== false) {
if (isset($_SERVER['PATH_INFO']) and $_SERVER['PATH_INFO'] !== '') {
@ -197,5 +198,6 @@ function min_get_slash_argument() {
$relativepath = $matches[1];
}
return $relativepath;
// Always clean the result of this function as it may be used in unsafe calls to send_file.
return min_clean_param($relativepath, 'SAFEPATH');
}

View File

@ -89,11 +89,11 @@ class core_configonlylib_testcase extends advanced_testcase {
/**
* Test fail-safe minimalistic slashargument processing.
*/
public function min_get_slash_argument() {
public function test_min_get_slash_argument() {
global $CFG;
$this->resetAfterTest();
$this->assertEquals('http://www.example.com/moode', $CFG->wwwroot);
$this->assertEquals('http://www.example.com/moodle', $CFG->wwwroot);
$_SERVER = array();
$_SERVER['SERVER_SOFTWARE'] = 'Apache/2.2.22 (Unix)';
@ -140,5 +140,15 @@ class core_configonlylib_testcase extends advanced_testcase {
$_SERVER['SCRIPT_NAME'] = '/moodle/theme/image.php';
$_GET = array();
$this->assertSame('/standard/core/5/u/f1', min_get_slash_argument());
$_SERVER = array();
$_SERVER['SERVER_SOFTWARE'] = 'Hacker server';
$_SERVER['QUERY_STRING'] = '';
$_SERVER['REQUEST_URI'] = '/moodle/theme/image.php/standard/core/5/u/f1';
$_SERVER['PATH_INFO'] = '/moodle/theme/image.php/standard\\core/..\\../5/u/f1';
$_SERVER['SCRIPT_NAME'] = '/moodle/theme/image.php';
$_GET = array();
// Windows dir separators are removed, multiple ... gets collapsed to one .
$this->assertSame('/standardcore/./5/u/f1', min_get_slash_argument());
}
}