From 26663756fbf9efed48d70ddc61992edeb4567fd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Tue, 29 Aug 2017 19:01:40 +0200 Subject: [PATCH] MDL-59969 admin: Warn admins if a development libs directory exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can't really control the direct web access to directories in dirroot, that is part of the server setup. So we at least warn admins as they may not realize the risks of having directories like vendor or node_modules exposed. Credit goes to Petr Škoda for mentioning the PHPUnit issue CVE-2017-9841 to me. --- admin/index.php | 9 ++- admin/renderer.php | 24 +++++++- lang/en/admin.php | 1 + report/security/lang/en/report_security.php | 6 ++ report/security/locallib.php | 66 ++++++++++++++++++++- 5 files changed, 103 insertions(+), 3 deletions(-) diff --git a/admin/index.php b/admin/index.php index d9d76b63480..abf05df8848 100644 --- a/admin/index.php +++ b/admin/index.php @@ -852,10 +852,17 @@ $cachewarnings = cache_helper::warnings(); // Check if there are events 1 API handlers. $eventshandlers = $DB->get_records_sql('SELECT DISTINCT component FROM {events_handlers}'); +// Check if a directory with development libraries exists. +if (is_dir($CFG->dirroot.'/vendor') || is_dir($CFG->dirroot.'/node_modules')) { + $devlibdir = true; +} else { + $devlibdir = false; +} + admin_externalpage_setup('adminnotifications'); $output = $PAGE->get_renderer('core', 'admin'); echo $output->admin_notifications_page($maturity, $insecuredataroot, $errorsdisplayed, $cronoverdue, $dbproblems, $maintenancemode, $availableupdates, $availableupdatesfetch, $buggyiconvnomb, - $registered, $cachewarnings, $eventshandlers); + $registered, $cachewarnings, $eventshandlers, null, $devlibdir); diff --git a/admin/renderer.php b/admin/renderer.php index b17c05745fe..8ac442f27dd 100644 --- a/admin/renderer.php +++ b/admin/renderer.php @@ -276,12 +276,15 @@ class core_admin_renderer extends plugin_renderer_base { * @param int|null $availableupdatesfetch timestamp of the most recent updates fetch or null (unknown) * @param string[] $cachewarnings An array containing warnings from the Cache API. * @param array $eventshandlers Events 1 API handlers. + * @param bool $ignored Forward-compatible placeholder for the themedesignermode. + * @param bool $devlibdir Warn about development libs directory presence. * * @return string HTML to output. */ public function admin_notifications_page($maturity, $insecuredataroot, $errorsdisplayed, $cronoverdue, $dbproblems, $maintenancemode, $availableupdates, $availableupdatesfetch, - $buggyiconvnomb, $registered, array $cachewarnings = array(), $eventshandlers = 0) { + $buggyiconvnomb, $registered, array $cachewarnings = array(), $eventshandlers = 0, + $ignored = false, $devlibdir = false) { global $CFG; $output = ''; @@ -290,6 +293,7 @@ class core_admin_renderer extends plugin_renderer_base { $output .= $this->legacy_log_store_writing_error(); $output .= empty($CFG->disableupdatenotifications) ? $this->available_updates($availableupdates, $availableupdatesfetch) : ''; $output .= $this->insecure_dataroot_warning($insecuredataroot); + $output .= $this->development_libs_directories_warning($devlibdir); $output .= $this->display_errors_warning($errorsdisplayed); $output .= $this->buggy_iconv_warning($buggyiconvnomb); $output .= $this->cron_overdue_warning($cronoverdue); @@ -519,6 +523,24 @@ class core_admin_renderer extends plugin_renderer_base { } } + /** + * Render a warning that a directory with development libs is present. + * + * @param bool $devlibdir True if the warning should be displayed. + * @return string + */ + protected function development_libs_directories_warning($devlibdir) { + + if ($devlibdir) { + $moreinfo = new moodle_url('/report/security/index.php'); + $warning = get_string('devlibdirpresent', 'core_admin', ['moreinfourl' => $moreinfo->out()]); + return $this->warning($warning, 'error'); + + } else { + return ''; + } + } + /** * Render an appropriate message if dataroot is insecure. * @param bool $errorsdisplayed diff --git a/lang/en/admin.php b/lang/en/admin.php index 2c758cb37a1..314b79397ff 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -416,6 +416,7 @@ $string['deleteunconfirmed'] = 'Delete not fully setup users after'; $string['deleteuser'] = 'Delete user'; $string['density'] = 'Density'; $string['denyemailaddresses'] = 'Denied email domains'; +$string['devlibdirpresent'] = 'Directories with development libraries such as vendor or node_modules should not be present on public sites. See the security overview report for more details.'; $string['development'] = 'Development'; $string['devicedetectregex'] = 'Device detection regular expressions'; $string['devicedetectregex_desc'] = '

By default, Moodle can detect devices of the type default (desktop PCs, laptops, etc), mobile (phones and small hand held devices), tablet (iPads, Android tablets) and legacy (Internet Explorer 6 users). The theme selector can be used to apply separate themes to all of these. This setting allows regular expressions that allow the detection of extra device types (these take precedence over the default types).

diff --git a/report/security/lang/en/report_security.php b/report/security/lang/en/report_security.php index 7d9c0a6baa0..e5e006cc08b 100644 --- a/report/security/lang/en/report_security.php +++ b/report/security/lang/en/report_security.php @@ -80,6 +80,9 @@ $string['check_noauth_details'] = '

The No authentication plugin is n $string['check_noauth_error'] = 'The No authentication plugin cannot be used on production sites.'; $string['check_noauth_name'] = 'No authentication'; $string['check_noauth_ok'] = 'No authentication plugin is disabled.'; +$string['check_nodemodules_details'] = '

The directory {$a->path} contains Node.js modules and their dependencies, typically installed by the NPM utility. These modules may be required for Moodle development. They are not needed to run a Moodle site and they can contain potentially dangerous code exposing your site to remote attacks.

It is strongly recommended to remove the directory if the site is available via a public URL, or at least prohibit web access to it.

'; +$string['check_nodemodules_info'] = 'The node_modules directory should not be present on public sites.'; +$string['check_nodemodules_name'] = 'Node.js modules directory'; $string['check_openprofiles_details'] = 'Open user profiles can be abused by spammers. It is recommended that either Force users to log in for profiles or Force users to log in are enabled.'; $string['check_openprofiles_error'] = 'Anyone can may view user profiles without logging in.'; $string['check_openprofiles_name'] = 'Open user profiles'; @@ -116,6 +119,9 @@ $string['check_unsecuredataroot_error'] = 'Your dataroot directory {$a} 'report_security_check_vendordir', + 'name' => get_string('check_vendordir_name', 'report_security'), + 'info' => get_string('check_vendordir_info', 'report_security'), + 'details' => null, + 'status' => null, + 'link' => null, + ]; + + if (is_dir($CFG->dirroot.'/vendor')) { + $result->status = REPORT_SECURITY_WARNING; + } else { + $result->status = REPORT_SECURITY_OK; + } + + if ($detailed) { + $result->details = get_string('check_vendordir_details', 'report_security', ['path' => $CFG->dirroot.'/vendor']); + } + + return $result; +} + +/** + * Check the presence of the node_modules directory. + * + * @param bool $detailed Return detailed info. + * @return object Result data. + */ +function report_security_check_nodemodules($detailed = false) { + global $CFG; + + $result = (object)[ + 'issue' => 'report_security_check_nodemodules', + 'name' => get_string('check_nodemodules_name', 'report_security'), + 'info' => get_string('check_nodemodules_info', 'report_security'), + 'details' => null, + 'status' => null, + 'link' => null, + ]; + + if (is_dir($CFG->dirroot.'/node_modules')) { + $result->status = REPORT_SECURITY_WARNING; + } else { + $result->status = REPORT_SECURITY_OK; + } + + if ($detailed) { + $result->details = get_string('check_nodemodules_details', 'report_security', ['path' => $CFG->dirroot.'/node_modules']); + } + + return $result; +}