Merge branch 'MDL-60940-master-forceclean' of git://github.com/mudrd8…

…mz/moodle
valmynd · Apr 11, 2018 · d2706c5 · d2706c5
2 parents 2d42e6e + 4b82c15
commit d2706c5
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 1 deletion.
diff --git a/admin/settings/security.php b/admin/settings/security.php
@@ -43,6 +43,9 @@
     $temp->add(new admin_setting_configtext('userquota', new lang_string('userquota', 'admin'),
                 new lang_string('configuserquota', 'admin', $params), $defaultuserquota, PARAM_INT, 30));
 
+    $temp->add(new admin_setting_configcheckbox('forceclean', new lang_string('forceclean', 'core_admin'),
+        new lang_string('forceclean_desc', 'core_admin'), 0));
+
     $temp->add(new admin_setting_configcheckbox('allowobjectembed', new lang_string('allowobjectembed', 'admin'), new lang_string('configallowobjectembed', 'admin'), 0));
     $temp->add(new admin_setting_configcheckbox('enabletrusttext', new lang_string('enabletrusttext', 'admin'), new lang_string('configenabletrusttext', 'admin'), 0));
     $temp->add(new admin_setting_configselect('maxeditingtime', new lang_string('maxeditingtime','admin'), new lang_string('configmaxeditingtime','admin'), 1800,

diff --git a/lang/en/admin.php b/lang/en/admin.php
@@ -552,6 +552,8 @@
 $string['filtersettings'] = 'Manage filters';
 $string['filtersettingsgeneral'] = 'General filter settings';
 $string['filteruploadedfiles'] = 'Filter uploaded files';
+$string['forceclean'] = 'Content cleaning everywhere';
+$string['forceclean_desc'] = 'Content added to the site is normally cleaned before being displayed, to remove anything which might be a security threat. However, content is not cleaned in certain places such as activity descriptions, page resources or HTML blocks to allow scripts, media, inline frames etc. to be added. If this setting is enabled, ALL content will be cleaned. This may result in existing content no longer displaying correctly.';
 $string['forcelogin'] = 'Force users to log in';
 $string['forceloginforprofileimage'] = 'Force users to log in to view user pictures';
 $string['forceloginforprofileimage_help'] = 'If enabled, users must log in in order to view user profile pictures and the default user picture will be used in all notification emails.';

diff --git a/lib/externallib.php b/lib/externallib.php
@@ -977,6 +977,9 @@ function external_format_text($text, $textformat, $contextorid, $component = nul
         $text = file_rewrite_pluginfile_urls($text, $settings->get_file(), $contextid, $component, $filearea, $itemid);
     }
 
+    // Note that $CFG->forceclean does not apply here if the client requests for the raw database content.
+    // This is consistent with web clients that are still able to load non-cleaned text into editors, too.
+
     if (!$settings->get_raw()) {
         $options = (array)$options;
 

diff --git a/lib/tests/weblib_format_text_test.php b/lib/tests/weblib_format_text_test.php
@@ -163,4 +163,79 @@ public function format_text_blanktarget_testcases() {
             ]
         ];
     }
+
+    /**
+     * Test ability to force cleaning of otherwise non-cleaned content.
+     *
+     * @dataProvider format_text_cleaning_testcases
+     *
+     * @param string $input Input text
+     * @param string $nocleaned Expected output of format_text() with noclean=true
+     * @param string $cleaned Expected output of format_text() with noclean=false
+     */
+    public function test_format_text_cleaning($input, $nocleaned, $cleaned) {
+        global $CFG;
+        $this->resetAfterTest();
+
+        $CFG->forceclean = false;
+        $actual = format_text($input, FORMAT_HTML, ['filter' => false, 'noclean' => false]);
+        $this->assertEquals($cleaned, $actual);
+
+        $CFG->forceclean = true;
+        $actual = format_text($input, FORMAT_HTML, ['filter' => false, 'noclean' => false]);
+        $this->assertEquals($cleaned, $actual);
+
+        $CFG->forceclean = false;
+        $actual = format_text($input, FORMAT_HTML, ['filter' => false, 'noclean' => true]);
+        $this->assertEquals($nocleaned, $actual);
+
+        $CFG->forceclean = true;
+        $actual = format_text($input, FORMAT_HTML, ['filter' => false, 'noclean' => true]);
+        $this->assertEquals($cleaned, $actual);
+    }
+
+    /**
+     * Data provider for the test_format_text_cleaning testcase
+     *
+     * @return array of testcases (string)testcasename => [(string)input, (string)nocleaned, (string)cleaned]
+     */
+    public function format_text_cleaning_testcases() {
+        return [
+            'JavaScript' => [
+                'Hello <script type="text/javascript">alert("XSS");</script> world',
+                'Hello <script type="text/javascript">alert("XSS");</script> world',
+                'Hello  world',
+            ],
+            'Inline frames' => [
+                'Let us go phishing! <iframe src="https://1.2.3.4/google.com"></iframe>',
+                'Let us go phishing! <iframe src="https://1.2.3.4/google.com"></iframe>',
+                'Let us go phishing! ',
+            ],
+            'Malformed A tags' => [
+                '<a onmouseover="alert(document.cookie)">xxs link</a>',
+                '<a onmouseover="alert(document.cookie)">xxs link</a>',
+                '<a>xxs link</a>',
+            ],
+            'Malformed IMG tags' => [
+                '<IMG """><SCRIPT>alert("XSS")</SCRIPT>">',
+                '<IMG """><SCRIPT>alert("XSS")</SCRIPT>">',
+                '"&gt;',
+            ],
+            'On error alert' => [
+                '<IMG SRC=/ onerror="alert(String.fromCharCode(88,83,83))"></img>',
+                '<IMG SRC=/ onerror="alert(String.fromCharCode(88,83,83))"></img>',
+                '<img src="/" alt="" />',
+            ],
+            'IMG onerror and javascript alert encode' => [
+                '<img src=x onerror="&#0000106&#0000097&#0000118&#0000097&#0000115&#0000099&#0000083&#0000083&#0000039&#0000041">',
+                '<img src=x onerror="&#0000106&#0000097&#0000118&#0000097&#0000115&#0000099&#0000083&#0000083&#0000039&#0000041">',
+                '<img src="x" alt="x" />',
+            ],
+            'DIV background-image' => [
+                '<DIV STYLE="background-image: url(javascript:alert(\'XSS\'))">',
+                '<DIV STYLE="background-image: url(javascript:alert(\'XSS\'))">',
+                '<div></div>',
+            ],
+        ];
+    }
 }
diff --git a/lib/weblib.php b/lib/weblib.php
@@ -1170,7 +1170,7 @@ function format_text_menu() {
  * <pre>
  * Options:
  *      trusted     :   If true the string won't be cleaned. Default false required noclean=true.
- *      noclean     :   If true the string won't be cleaned. Default false required trusted=true.
+ *      noclean     :   If true the string won't be cleaned, unless $CFG->forceclean is set. Default false required trusted=true.
  *      nocache     :   If true the strign will not be cached and will be formatted every call. Default false.
  *      filter      :   If true the string will be run through applicable filters as well. Default true.
  *      para        :   If true then the returned string will be wrapped in div tags. Default true.
@@ -1213,6 +1213,10 @@ function format_text($text, $format = FORMAT_MOODLE, $options = null, $courseidd
             $options['noclean'] = false;
         }
     }
+    if (!empty($CFG->forceclean)) {
+        // Whatever the caller claims, the admin wants all content cleaned anyway.
+        $options['noclean'] = false;
+    }
     if (!isset($options['nocache'])) {
         $options['nocache'] = false;
     }