From 151d2ba59faec295586de385885e2156d546fc3c Mon Sep 17 00:00:00 2001 From: Daniel Thee Roperto Date: Thu, 9 Feb 2017 13:34:38 +1100 Subject: [PATCH 1/4] Adding timeout to fetch local files. --- classes/local/outagelib.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/classes/local/outagelib.php b/classes/local/outagelib.php index 8037eb0..a514b68 100644 --- a/classes/local/outagelib.php +++ b/classes/local/outagelib.php @@ -58,7 +58,8 @@ class outagelib { curl_setopt($curl, CURLOPT_RETURNTRANSFER, true); curl_setopt($curl, CURLOPT_SSL_VERIFYHOST, false); curl_setopt($curl, CURLOPT_SSL_VERIFYPEER, false); - curl_setopt($curl, CURLOPT_TIMEOUT, 5); // Feching local files should be very fast. + curl_setopt($curl, CURLOPT_CONNECTTIMEOUT, 1); // It is localhost, time to connect is enough. + curl_setopt($curl, CURLOPT_TIMEOUT, 5); // It is localhost, time to fetch index is enough. $contents = curl_exec($curl); $mime = curl_getinfo($curl, CURLINFO_CONTENT_TYPE); curl_close($curl); From 0b17efedbadb0c47f83e70d5c07bdd92186c7d77 Mon Sep 17 00:00:00 2001 From: Daniel Thee Roperto Date: Mon, 20 Feb 2017 12:01:51 +1100 Subject: [PATCH 2/4] Issue #104 - Allowed symlinks in dataroot, added unit tests. --- file.php | 17 ++---- lib.php | 31 ++++++++++ tests/phpunit/lib_test.php | 114 +++++++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 12 deletions(-) create mode 100644 tests/phpunit/lib_test.php diff --git a/file.php b/file.php index 3bdc90f..d534a2d 100644 --- a/file.php +++ b/file.php @@ -60,23 +60,16 @@ header('Accept-Ranges: none'); * @SupressWarnings(PHPMD) */ function auth_outage_bootstrap_callback() { - global $CFG; - - // We are not using any external libraries or references in this file (cli maintenance is active). - // If you change the path below maybe you need to change maintenance_static_page::get_resources_folder() as well. - $resourcedir = rtrim($CFG->dataroot, '/'); // In case the configuration has a trailing slash. - $resourcedir = $resourcedir.'/auth_outage/climaintenance'; - - // Protect against path traversal attacks. - $file = $resourcedir.'/'.$_GET['file']; - if (realpath($file) !== $file) { + // Not using classes as classloader has not been initialized yet. Keep it minimalist. + require_once(__DIR__.'/lib.php'); + $file = auth_outage_get_climaintenance_resource_file($_GET['file']); + if (is_null($file)) { // @codingStandardsIgnoreStart error_log('Invalid file: '.$_GET['file']); // @codingStandardsIgnoreEnd http_response_code(404); - die('Not found.'); + die('File not found.'); } - readfile($file); exit(0); } diff --git a/lib.php b/lib.php index 142befc..dfda31a 100644 --- a/lib.php +++ b/lib.php @@ -56,3 +56,34 @@ function auth_outage_extend_navigation_user() { function auth_outage_outagelib_prepare_next_outage() { outagelib::prepare_next_outage(); } + +/** + * Used by file.php to fetch a file from sitedata, protecting it from path traversal attacks. + * + * To keep it minimalist it was not added to the outagelib.php class. + * + * @param $file string Filename to fetch from sitedata + * @return string|null Full path to the sitedata file or null if file is not valid. + */ +function auth_outage_get_climaintenance_resource_file($file) { + global $CFG; + + // We are not using any external libraries or references in this file (we have not gully loaded config.php yet). + // If you change the path below maybe you need to change maintenance_static_page::get_resources_folder() as well. + $resourcedir = rtrim($CFG->dataroot, '/'); // In case the configuration has a trailing slash. + $resourcedir = $resourcedir.'/auth_outage/climaintenance'; + + // Protect against path traversal attacks. + $basename = basename($file); + if ($basename !== $file) { + // @codingStandardsIgnoreStart + if (!PHPUNIT_TEST) { + error_log('Possible attempt for Path Traversal Attack (only filename expected): '.$file); + } + // @codingStandardsIgnoreEnd + return null; + } + + $realpath = realpath($resourcedir.'/'.$file); + return ($realpath == false) ? null : $realpath; +} diff --git a/tests/phpunit/lib_test.php b/tests/phpunit/lib_test.php new file mode 100644 index 0000000..f72fb56 --- /dev/null +++ b/tests/phpunit/lib_test.php @@ -0,0 +1,114 @@ +. + +/** + * tests for lib.php + * + * @package auth_outage + * @author Daniel Thee Roperto + * @copyright 2017 Catalyst IT + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); +require_once(__DIR__.'/base_testcase.php'); +require_once(__DIR__.'/../../lib.php'); + +/** + * tests for lib.php + * + * @package auth_outage + * @author Daniel Thee Roperto + * @copyright 2017 Catalyst IT + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @SuppressWarnings(public) Allow as many methods as needed. + */ +class lib_test extends auth_outage_base_testcase { + public function test_auth_outage_get_climaintenance_resource_file_resolves_a_file() { + global $CFG; + $dir = $CFG->dataroot.'/auth_outage/climaintenance'; + mkdir($dir, 0777, true); + + // Create a file. + $expected = $dir.'/example.txt'; + file_put_contents($expected, 'Outage Unit Test Message'); + + // Get that file. + $actual = auth_outage_get_climaintenance_resource_file('example.txt'); + + // Clean up. + unlink($expected); + rmdir($dir); + + self::assertSame($expected, $actual); + } + + /** + * Regression test for issue #104. + */ + public function test_auth_outage_get_climaintenance_resource_file_resolves_a_file_with_symlink() { + global $CFG; + + // Create a file. + $realdir = $CFG->dataroot.'/auth_outage/climaintenance_real'; + mkdir($realdir, 0777, true); + $realfile = $realdir.'/example.txt'; + file_put_contents($realfile, 'Outage Unit Test Message'); + + // Create a symlink + $symdir = $CFG->dataroot.'/auth_outage/climaintenance'; + if (!symlink($realdir, $symdir)) { + unlink($realfile); + rmdir($realdir); + $this->markTestSkipped('Canont create symlinks, maybe OS does not support.'); + return; + } + + // Get that file. + $actual = auth_outage_get_climaintenance_resource_file('example.txt'); + + // Clean up. + unlink($symdir); + unlink($realfile); + rmdir($realdir); + + self::assertSame($realfile, $actual); + } + + public function test_auth_outage_get_climaintenance_resource_file_prevent_path_traversal() { + global $CFG; + + $dir = $CFG->dataroot.'/auth_outage/climaintenance'; + mkdir($dir, 0777, true); + + // Create a file. + $expected = $dir.'/example.txt'; + file_put_contents($expected, 'Outage Unit Test Message'); + + // Create a sensitive file. + $sensitivefile = $CFG->dataroot.'/auth_outage/nuclear_silo_passwords.txt'; + file_put_contents($sensitivefile, 'The password to launch the ICBM: 123456'); + + // Path Traversal Attack. + $actual = auth_outage_get_climaintenance_resource_file('../n\\uclear_silo_passwords.txt'); + + // Clean up. + unlink($expected); + rmdir($dir); + + self::assertNull($actual); + } +} From 2b25eb92db6f8ab12ab7a10c4a6a6059d273cee5 Mon Sep 17 00:00:00 2001 From: Daniel Thee Roperto Date: Mon, 20 Feb 2017 14:42:14 +1100 Subject: [PATCH 3/4] Removed Travis support for Moodle 32 and 33, see issue #106 --- .travis.yml | 16 ---------------- README.md | 2 +- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/.travis.yml b/.travis.yml index d4ba6c1..3a1975d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,29 +13,13 @@ cache: php: - 5.5 - - 5.6 - 7.0 env: - DB=pgsql MOODLE_BRANCH=MOODLE_30_STABLE - DB=pgsql MOODLE_BRANCH=MOODLE_31_STABLE - - DB=pgsql MOODLE_BRANCH=MOODLE_32_STABLE - - DB=pgsql MOODLE_BRANCH=master - DB=mysqli MOODLE_BRANCH=MOODLE_30_STABLE - DB=mysqli MOODLE_BRANCH=MOODLE_31_STABLE - - DB=mysqli MOODLE_BRANCH=MOODLE_32_STABLE - - DB=mysqli MOODLE_BRANCH=master - -matrix: - exclude: - - php: 5.5 - env: DB=pgsql MOODLE_BRANCH=MOODLE_32_STABLE - - php: 5.5 - env: DB=pgsql MOODLE_BRANCH=master - - php: 5.5 - env: DB=mysqli MOODLE_BRANCH=MOODLE_32_STABLE - - php: 5.5 - env: DB=mysqli MOODLE_BRANCH=master before_install: - phpenv config-rm xdebug.ini diff --git a/README.md b/README.md index cf482b8..bb42c62 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ and testers letting them know what is about to happen and why. Moodle Requirements ------------------- -This plugin will work out-of-the-box with Moodle 3+. +This plugin will work out-of-the-box with Moodle 3.0 and Moodle 3.1. If you have an older version of Moodle you can still make it work but you will need to manually add one extra plugin, please check: From d80ae326fd03f58856a31d65facaf5c9fffb39a6 Mon Sep 17 00:00:00 2001 From: Daniel Thee Roperto Date: Thu, 2 Mar 2017 18:13:44 +1100 Subject: [PATCH 4/4] Fixes #107 - Check in bootstrap.php if this is a behat environment. --- README.md | 2 +- bootstrap.php | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index bb42c62..6aba6dd 100644 --- a/README.md +++ b/README.md @@ -74,7 +74,7 @@ https://github.com/catalyst/moodle-auth_outage/issues enable the `Outage manager` plugin and place it on the top. 4. If you need to use the IP Blocking, please add the following lines into your `config.php` -after your `$CFG->dataroot` is set: +before the `require('/lib/setup.php')` call: ``` // Insert this after $CFG->dataroot is defined. diff --git a/bootstrap.php b/bootstrap.php index 6355386..82c6d5d 100644 --- a/bootstrap.php +++ b/bootstrap.php @@ -37,12 +37,36 @@ if (!isset($CFG->dataroot)) { return; } -// 1) Check and run the hook. +// 1) Make sure we replace the configurations for behat as we have not ran 'lib/setup.php' yet. +if (!empty($CFG->behat_wwwroot) or !empty($CFG->behat_dataroot) or !empty($CFG->behat_prefix)) { + require_once(__DIR__.'/../../lib/behat/lib.php'); + behat_update_vars_for_process(); + if (behat_is_test_site()) { + $beforebehatcfg = $CFG; + $CFG = clone($CFG); + clearstatcache(); + behat_check_config_vars(); + behat_clean_init_config(); + $CFG->wwwroot = $CFG->behat_wwwroot; + $CFG->dataroot = $CFG->behat_dataroot; + // We should not access database in bootstrap. + $CFG->dbtype = null; + $CFG->dblibrary = null; + $CFG->dbhost = null; + $CFG->dbname = null; + $CFG->dbuser = null; + $CFG->dbpass = null; + $CFG->prefix = null; + $CFG->dboptions = null; + } +} + +// 2) Check and run the hook. if (is_callable('auth_outage_bootstrap_callback')) { call_user_func('auth_outage_bootstrap_callback'); } -// 2) Check for allowed scripts or IPs during outages. +// 3) Check for allowed scripts or IPs during outages. $allowed = !file_exists($CFG->dataroot.'/climaintenance.php') // Not in maintenance mode. || (defined('ABORT_AFTER_CONFIG') && ABORT_AFTER_CONFIG) // Only config requested. || (defined('CLI_SCRIPT') && CLI_SCRIPT); // Allow CLI scripts. @@ -52,5 +76,10 @@ if (!$allowed) { require($CFG->dataroot.'/climaintenance.php'); // This call may terminate the script here or not. } -// 3) Set flag this file was loaded. +// 4) Set flag this file was loaded. $CFG->auth_outage_bootstrap_loaded = true; + +// 5) Restore behat config as needed (let setup.php execute which is more complex than our quick-check). +if (isset($beforebehatcfg)) { + $CFG = $beforebehatcfg; +}