MDL-15716 Tightened dataroot security checks and and 'loud' administr…

…ator warning

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')) {

blocks/admin_tree/block_admin_tree.php

@@ -70,6 +70,11 @@ function build_tree (&$content) {
             // 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';
                 }

install.php

@@ -272,8 +272,12 @@
 
     /// check dataroot
     $CFG->dataroot = $INSTALL['dataroot'];
+    $CFG->wwwroot  = $INSTALL['wwwroot'];
     if (make_upload_directory('sessions', false) === false) {
         $errormsg .= get_string('datarooterror', 'install').'<br />';
+
+    } else if (is_dataroot_insecure(true) == INSECURE_DATAROOT_ERROR) {
+        $errormsg .= get_string('datarootpublicerror', 'install').'<br />';
     }
 
     if (!empty($errormsg)) {

install/stringnames.txt

@@ -74,6 +74,7 @@ databasetype
 databaseuser
 dataroot
 datarooterror
+datarootpublicerror
 dbconnectionerror
 dbcreationerror
 dbhost

lang/en_utf8/admin.php

@@ -274,6 +274,8 @@
 $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'] = '<p><strong>SECURITY WARNING!</strong></p><p>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!</p>
+<p>You <em>must</em> move dataroot directory ($a) to a new location that is not within your public web directory, and update the <code>\$CFG->dataroot</code> setting in your config.php accordingly.</p>';
 $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['generalsettings'] = 'General settings';
 $string['geoipfile'] = 'GeoIP City data file';
 $string['globalsquoteswarning'] = '<p><strong>Security Warning</strong>: to operate properly, Moodle requires <br />that you make certain changes to your current PHP settings.<p/><p>You <em>must</em> set <code>register_globals=off</code> and/or <code>magic_quotes_gpc=on</code>. <br />If possible, you should set <code>register_globals=off</code> to improve general <br /> server security, setting <code>magic_quotes_gpc=on</code> is also recommended.<p/><p>These settings are controlled by editing your <code>php.ini</code>, Apache/IIS <br />configuration or <code>.htaccess</code> file.</p>';
-$string['globalswarning'] = '<p><strong>Security Warning</strong>: to operate properly, Moodle requires <br />that you make certain changes to your current PHP settings.<p/><p>You <em>must</em> set <code>register_globals=off</code>.<p>This setting is controlled by editing your <code>php.ini</code>, Apache/IIS <br />configuration or <code>.htaccess</code> file.</p>';
+$string['globalswarning'] = '<p><strong>SECURITY WARNING!</strong></p><p> To operate properly, Moodle requires <br />that you make certain changes to your current PHP settings.</p><p>You <em>must</em> set <code>register_globals=off</code>.</p><p>This setting is controlled by editing your <code>php.ini</code>, Apache/IIS <br />configuration or <code>.htaccess</code> file.</p>';
 $string['googlemapkey'] = 'Google Maps API key';
 $string['gotofirst'] = 'Go to first missing string';
 $string['gradebook'] = 'Gradebook';

lang/en_utf8/install.php

@@ -141,6 +141,7 @@
 $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 @@
 <br />
 <b>Data Directory:</b>
 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.';

lib/adminlib.php

@@ -31,6 +31,8 @@
 $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;
 }
 
 /// =============================================================================================================

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;
 }

theme/standard/styles_layout.css

@@ -1017,13 +1017,15 @@ body#admin-modules table.generaltable td.c0
   margin:auto;
 }
 
+#admin-index .adminerror,
 #admin-index .adminwarning {
   text-align:center;
   border-width: 1px;
   border-style: solid;
   margin:20px;
 }
 
+#admin-index .adminerror .singlebutton,
 #admin-index .adminwarning .singlebutton,
 #admin-index #layout-table .singlebutton {
   text-align:center;