From: skodak Date: Thu, 21 Aug 2008 15:29:42 +0000 (+0000) Subject: MDL-15716 Tightened dataroot security checks and and 'loud' administrator warning X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=bba0beaee246ca3982840892329013412be7f57c;p=moodle.git MDL-15716 Tightened dataroot security checks and and 'loud' administrator warning --- diff --git a/admin/index.php b/admin/index.php index 09ebde323c..10d436ed3a 100644 --- a/admin/index.php +++ b/admin/index.php @@ -538,6 +538,12 @@ } } +/// setup critical warnings before printing admin tree block + $insecuredataroot = is_dataroot_insecure(true); + $register_globals_enabled = ini_get_bool('register_globals'); + + $SESSION->admin_critical_warning = ($register_globals_enabled || $insecuredataroot==INSECURE_DATAROOT_ERROR); + $adminroot =& admin_get_root(); /// Check if there are any new admin settings which have still yet to be set @@ -562,12 +568,15 @@ print_box(get_string("upgrade$CFG->upgrade", "admin", "$CFG->wwwroot/$CFG->admin/upgrade$CFG->upgrade.php")); } - if (ini_get_bool('register_globals')) { - print_box(get_string('globalswarning', 'admin'), 'generalbox adminwarning'); + if ($register_globals_enabled) { + print_box(get_string('globalswarning', 'admin'), 'generalbox adminerror'); } - if (is_dataroot_insecure()) { + if ($insecuredataroot == INSECURE_DATAROOT_WARNING) { print_box(get_string('datarootsecuritywarning', 'admin', $CFG->dataroot), 'generalbox adminwarning'); + } else if ($insecuredataroot == INSECURE_DATAROOT_ERROR) { + print_box(get_string('datarootsecurityerror', 'admin', $CFG->dataroot), 'generalbox adminerror'); + } if (defined('WARN_DISPLAY_ERRORS_ENABLED')) { diff --git a/blocks/admin_tree/block_admin_tree.php b/blocks/admin_tree/block_admin_tree.php index 32494c97b6..14cd47ee5c 100644 --- a/blocks/admin_tree/block_admin_tree.php +++ b/blocks/admin_tree/block_admin_tree.php @@ -70,6 +70,11 @@ class block_admin_tree extends block_base { // show hidden pages in tree if hidden page active if ($content->check_access() and (($content->name == $this->section) or !$content->is_hidden())) { $class = ($content->name == $this->section) ? 'link current' : 'link'; + if ($content->name === 'adminnotifications') { + if (admin_critical_warnings_present()) { + $class .= ' criticalnotification'; + } + } if ($content->is_hidden()) { $class .= ' hidden'; } diff --git a/install.php b/install.php index 20eca65ff1..37b38225d3 100644 --- a/install.php +++ b/install.php @@ -272,8 +272,12 @@ if ($INSTALL['stage'] == DIRECTORY) { /// check dataroot $CFG->dataroot = $INSTALL['dataroot']; + $CFG->wwwroot = $INSTALL['wwwroot']; if (make_upload_directory('sessions', false) === false) { $errormsg .= get_string('datarooterror', 'install').'
'; + + } else if (is_dataroot_insecure(true) == INSECURE_DATAROOT_ERROR) { + $errormsg .= get_string('datarootpublicerror', 'install').'
'; } if (!empty($errormsg)) { diff --git a/install/stringnames.txt b/install/stringnames.txt index 5656ad6af6..4bdd85a46d 100644 --- a/install/stringnames.txt +++ b/install/stringnames.txt @@ -74,6 +74,7 @@ databasetype databaseuser dataroot datarooterror +datarootpublicerror dbconnectionerror dbcreationerror dbhost diff --git a/lang/en_utf8/admin.php b/lang/en_utf8/admin.php index 3f4eb35e6e..b7ddb42596 100644 --- a/lang/en_utf8/admin.php +++ b/lang/en_utf8/admin.php @@ -274,6 +274,8 @@ $string['csvdelimiter'] = 'CSV delimiter'; $string['curlrecommended'] = 'Installing the optional cURL library is highly recommended in order to enable Moodle Networking functionality.'; $string['curlrequired'] = 'The cURL PHP extension is now required by Moodle, in order to commnunicate with Moodle repositories.'; $string['customcheck'] = 'Other Checks'; +$string['datarootsecurityerror'] = '

SECURITY WARNING!

Your dataroot directory is in the wrong location and is exposed to the web. This means that all your private files are available to anyone in the world, and some of them could be used by a cracker to obtain unauthorised administrative access to your site!

+

You must move dataroot directory ($a) to a new location that is not within your public web directory, and update the \$CFG->dataroot setting in your config.php accordingly.

'; $string['datarootsecuritywarning'] = 'Your site configuration might not be secure. Please make sure that your dataroot directory ($a) is not directly accessible via web.'; $string['dbmigrate'] = 'Moodle Database Migration'; $string['dbmigrateconnecerror'] = 'Could not connect to the database specified.'; @@ -386,7 +388,7 @@ $string['gdversion'] = 'GD version'; $string['generalsettings'] = 'General settings'; $string['geoipfile'] = 'GeoIP City data file'; $string['globalsquoteswarning'] = '

Security Warning: to operate properly, Moodle requires
that you make certain changes to your current PHP settings.

You must set register_globals=off and/or magic_quotes_gpc=on.
If possible, you should set register_globals=off to improve general
server security, setting magic_quotes_gpc=on is also recommended.

These settings are controlled by editing your php.ini, Apache/IIS
configuration or .htaccess file.

'; -$string['globalswarning'] = '

Security Warning: to operate properly, Moodle requires
that you make certain changes to your current PHP settings.

You must set register_globals=off.

This setting is controlled by editing your php.ini, Apache/IIS
configuration or .htaccess file.

'; +$string['globalswarning'] = '

SECURITY WARNING!

To operate properly, Moodle requires
that you make certain changes to your current PHP settings.

You must set register_globals=off.

This setting is controlled by editing your php.ini, Apache/IIS
configuration or .htaccess file.

'; $string['googlemapkey'] = 'Google Maps API key'; $string['gotofirst'] = 'Go to first missing string'; $string['gradebook'] = 'Gradebook'; diff --git a/lang/en_utf8/install.php b/lang/en_utf8/install.php index caf26ca903..e01c98f5db 100644 --- a/lang/en_utf8/install.php +++ b/lang/en_utf8/install.php @@ -141,6 +141,7 @@ $string['databasetype']='Database type :'; $string['databaseuser']='Database user :'; $string['dataroot'] = 'Data Directory'; $string['datarooterror'] = 'The \'Data Directory\' you specified could not be found or created. Either correct the path or create that directory manually.'; +$string['datarootpublicerror'] = 'The \'Data Directory\' you specified is directly accessible via web, you must use different directory.'; $string['dbconnectionerror'] = 'We could not connect to the database you specified. Please check your database settings.'; $string['dbcreationerror'] = 'Database creation error. Could not create the given database name with the settings provided'; $string['dbhost'] = 'Host Server'; @@ -184,8 +185,8 @@ Make sure the upper/lower case is correct.
Data Directory: You need a place where Moodle can save uploaded files. This -directory should be readable AND WRITEABLE by the web server user -(usually \'nobody\' or \'apache\'), but it should not be accessible +directory must be readable AND WRITEABLE by the web server user +(usually \'nobody\' or \'apache\'), but it must not be accessible directly via the web.'; $string['dirroot'] = 'Moodle Directory'; $string['dirrooterror'] = 'The \'Moodle Directory\' setting seems to be incorrect - we can\'t find a Moodle installation there. The value below has been reset.'; diff --git a/lib/adminlib.php b/lib/adminlib.php index e67fc5f3a1..97f153aa53 100644 --- a/lib/adminlib.php +++ b/lib/adminlib.php @@ -31,6 +31,8 @@ global $upgradeloghandle, $upgradelogbuffer; $upgradeloghandle = false; $upgradelogbuffer = ''; +define('INSECURE_DATAROOT_WARNING', 1); +define('INSECURE_DATAROOT_ERROR', 2); /** * Upgrade savepoint, marks end of each upgrade block. @@ -985,13 +987,38 @@ function upgrade_log_callback($string) { return $string; } +/** + * Test if and critical warnings are present + * @return bool + */ +function admin_critical_warnings_present() { + global $SESSION; + + if (!has_capability('moodle/site:config', get_context_instance(CONTEXT_SYSTEM))) { + return 0; + } + + if (!isset($SESSION->admin_critical_warning)) { + $SESSION->admin_critical_warning = 0; + if (ini_get_bool('register_globals')) { + $SESSION->admin_critical_warning = 1; + } else if (is_dataroot_insecure(true) === INSECURE_DATAROOT_ERROR) { + $SESSION->admin_critical_warning = 1; + } + } + + return $SESSION->admin_critical_warning; +} + /** * Try to verify that dataroot is not accessible from web. * It is not 100% correct but might help to reduce number of vulnerable sites. * * Protection from httpd.conf and .htaccess is not detected properly. + * @param bool $fetchtest try to test public access by fetching file + * @return mixed empty means secure, INSECURE_DATAROOT_ERROR found a critical problem, INSECURE_DATAROOT_WARNING migth be problematic */ -function is_dataroot_insecure() { +function is_dataroot_insecure($fetchtest=false) { global $CFG; $siteroot = str_replace('\\', '/', strrev($CFG->dirroot.'/')); // win32 backslash workaround @@ -1010,10 +1037,83 @@ function is_dataroot_insecure() { $siteroot = strrev($siteroot); $dataroot = str_replace('\\', '/', $CFG->dataroot.'/'); - if (strpos($dataroot, $siteroot) === 0) { - return true; + if (strpos($dataroot, $siteroot) !== 0) { + return false; } - return false; + + if (!$fetchtest) { + return INSECURE_DATAROOT_WARNING; + } + + // now try all methods to fetch a test file using http protocol + + $httpdocroot = str_replace('\\', '/', strrev($CFG->dirroot.'/')); + preg_match('|(https?://[^/]+)|i', $CFG->wwwroot, $matches); + $httpdocroot = $matches[1]; + $datarooturl = $httpdocroot.'/'. substr($dataroot, strlen($siteroot)); + if (make_upload_directory('diag', false) === false) { + return INSECURE_DATAROOT_WARNING; + } + $testfile = $CFG->dataroot.'/diag/public.txt'; + if (!file_exists($testfile)) { + file_put_contents($testfile, 'test file, do not delete'); + } + $teststr = trim(file_get_contents($testfile)); + if (empty($teststr)) { + // hmm, strange + return INSECURE_DATAROOT_WARNING; + } + + $testurl = $datarooturl.'/diag/public.txt'; + + if (extension_loaded('curl') and ($ch = @curl_init($testurl)) !== false) { + curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); + curl_setopt($ch, CURLOPT_HEADER, false); + $data = curl_exec($ch); + if (!curl_errno($ch)) { + $data = trim($data); + if ($data === $teststr) { + curl_close($ch); + return INSECURE_DATAROOT_ERROR; + } + } + curl_close($ch); + } + + if ($data = @file_get_contents($testurl)) { + $data = trim($data); + if ($data === $teststr) { + return INSECURE_DATAROOT_ERROR; + } + } + + preg_match('|https?://([^/]+)|i', $testurl, $matches); + $sitename = $matches[1]; + $error = 0; + if ($fp = @fsockopen($sitename, 80, $error)) { + preg_match('|https?://[^/]+(.*)|i', $testurl, $matches); + $localurl = $matches[1]; + $out = "GET $localurl HTTP/1.1\r\n"; + $out .= "Host: $sitename\r\n"; + $out .= "Connection: Close\r\n\r\n"; + fwrite($fp, $out); + $data = ''; + $incoming = false; + while (!feof($fp)) { + if ($incoming) { + $data .= fgets($fp, 1024); + } else if (@fgets($fp, 1024) === "\r\n") { + $incoming = true; + } + } + fclose($fp); + $data = trim($data); + if ($data === $teststr) { + return INSECURE_DATAROOT_ERROR; + } + } + + return INSECURE_DATAROOT_WARNING; } /// ============================================================================================================= diff --git a/theme/standard/styles_color.css b/theme/standard/styles_color.css index 84c13a41ea..2ede663ad1 100644 --- a/theme/standard/styles_color.css +++ b/theme/standard/styles_color.css @@ -264,6 +264,10 @@ table.formtable tbody th { background-color:#FFFFFF; } +#admin-index .adminerror { + background-color:#ff6666; +} + body#admin-index .c0 { background-color: #FAFAFA; } @@ -375,6 +379,10 @@ table.flexible .r1 { background-color:#EEEEEE; } +.block_admin_tree.sideblock .link.criticalnotification { + background-color:#ff6666; +} + .block_admin_tree.sideblock .link.hidden { color:#999999; } diff --git a/theme/standard/styles_layout.css b/theme/standard/styles_layout.css index f6916f0a04..62a513219d 100644 --- a/theme/standard/styles_layout.css +++ b/theme/standard/styles_layout.css @@ -1017,6 +1017,7 @@ body#admin-modules table.generaltable td.c0 margin:auto; } +#admin-index .adminerror, #admin-index .adminwarning { text-align:center; border-width: 1px; @@ -1024,6 +1025,7 @@ body#admin-modules table.generaltable td.c0 margin:20px; } +#admin-index .adminerror .singlebutton, #admin-index .adminwarning .singlebutton, #admin-index #layout-table .singlebutton { text-align:center;