diff --git a/filter/upgrade.txt b/filter/upgrade.txt index 4f28d94b3ef..bd186cec7e1 100644 --- a/filter/upgrade.txt +++ b/filter/upgrade.txt @@ -1,6 +1,12 @@ This file describes API changes in core filter API and plugins, information provided here is intended especially for developers. +=== 4.3 === + +* Filters can be applied at different stages - before text format conversion, before text cleaning or after text cleaning. +It is recommended that add-on filter developers use early formatting stages to prevent security issues caused by +modification of already sanitised text. + === 4.0 === * The Word censorship (filter_censor) filter has been completely removed from core. It has been moved to the plugins database diff --git a/lib/filterlib.php b/lib/filterlib.php index ec80be2d720..58aeaff6d2a 100644 --- a/lib/filterlib.php +++ b/lib/filterlib.php @@ -169,11 +169,23 @@ class filter_manager { */ protected function apply_filter_chain($text, $filterchain, array $options = array(), array $skipfilters = null) { + if (!isset($options['stage'])) { + $filtermethod = 'filter'; + } else if (in_array($options['stage'], ['pre_format', 'pre_clean', 'post_clean', 'string'], true)) { + $filtermethod = 'filter_stage_' . $options['stage']; + } else { + $filtermethod = 'filter'; + debugging('Invalid filter stage specified in options: ' . $options['stage'], DEBUG_DEVELOPER); + } + if ($text === null || $text === '') { + // Nothing to filter. + return ''; + } foreach ($filterchain as $filtername => $filter) { if ($skipfilters !== null && in_array($filtername, $skipfilters)) { continue; } - $text = $filter->filter($text, $options); + $text = $filter->$filtermethod($text, $options); } return $text; } @@ -229,7 +241,7 @@ class filter_manager { * @return string resulting string */ public function filter_string($string, $context) { - return $this->apply_filter_chain($string, $this->get_string_filters($context)); + return $this->apply_filter_chain($string, $this->get_string_filters($context), ['stage' => 'string']); } /** @@ -368,7 +380,9 @@ class performance_measuring_filter_manager extends filter_manager { public function filter_text($text, $context, array $options = array(), array $skipfilters = null) { - $this->textsfiltered++; + if (!isset($options['stage']) || $options['stage'] === 'post_clean') { + $this->textsfiltered++; + } return parent::filter_text($text, $context, $options, $skipfilters); } @@ -450,11 +464,69 @@ abstract class moodle_text_filter { /** * Override this function to actually implement the filtering. * + * Filter developers must make sure that filtering done after text cleaning + * does not introduce security vulnerabilities. + * * @param string $text some HTML content to process. * @param array $options options passed to the filters * @return string the HTML content after the filtering has been applied. */ public abstract function filter($text, array $options = array()); + + /** + * Filter text before changing format to HTML. + * + * @param string $text + * @param array $options + * @return string + */ + public function filter_stage_pre_format(string $text, array $options): string { + // NOTE: override if necessary. + return $text; + } + + /** + * Filter HTML text before sanitising text. + * + * NOTE: this is called even if $options['noclean'] is true and text is not cleaned. + * + * @param string $text + * @param array $options + * @return string + */ + public function filter_stage_pre_clean(string $text, array $options): string { + // NOTE: override if necessary. + return $text; + } + + /** + * Filter HTML text at the very end after text is sanitised. + * + * NOTE: this is called even if $options['noclean'] is true and text is not cleaned. + * + * @param string $text + * @param array $options + * @return string + */ + public function filter_stage_post_clean(string $text, array $options): string { + // NOTE: override if necessary. + return $this->filter($text, $options); + } + + /** + * Filter simple text coming from format_string(). + * + * Note that unless $CFG->formatstringstriptags is disabled + * HTML tags are not expected in returned value. + * + * @param string $text + * @param array $options + * @return string + */ + public function filter_stage_string(string $text, array $options): string { + // NOTE: override if necessary. + return $this->filter($text, $options); + } } diff --git a/lib/weblib.php b/lib/weblib.php index d87c52f755c..89a4b6e3da2 100644 --- a/lib/weblib.php +++ b/lib/weblib.php @@ -1343,9 +1343,15 @@ function format_text($text, $format = FORMAT_MOODLE, $options = null, $courseidd switch ($format) { case FORMAT_HTML: + $filteroptions['stage'] = 'pre_format'; + $text = $filtermanager->filter_text($text, $context, $filteroptions); + // Text is already in HTML format, so just continue to the next filtering stage. + $filteroptions['stage'] = 'pre_clean'; + $text = $filtermanager->filter_text($text, $context, $filteroptions); if (!$options['noclean']) { $text = clean_text($text, FORMAT_HTML, $options); } + $filteroptions['stage'] = 'post_clean'; $text = $filtermanager->filter_text($text, $context, $filteroptions); break; @@ -1365,17 +1371,27 @@ function format_text($text, $format = FORMAT_MOODLE, $options = null, $courseidd break; case FORMAT_MARKDOWN: + $filteroptions['stage'] = 'pre_format'; + $text = $filtermanager->filter_text($text, $context, $filteroptions); $text = markdown_to_html($text); + $filteroptions['stage'] = 'pre_clean'; + $text = $filtermanager->filter_text($text, $context, $filteroptions); // The markdown parser does not strip dangerous html so we need to clean it, even if noclean is set to true. $text = clean_text($text, FORMAT_HTML, $options); + $filteroptions['stage'] = 'post_clean'; $text = $filtermanager->filter_text($text, $context, $filteroptions); break; default: // FORMAT_MOODLE or anything else. + $filteroptions['stage'] = 'pre_format'; + $text = $filtermanager->filter_text($text, $context, $filteroptions); $text = text_to_html($text, null, $options['para'], $options['newlines']); + $filteroptions['stage'] = 'pre_clean'; + $text = $filtermanager->filter_text($text, $context, $filteroptions); if (!$options['noclean']) { $text = clean_text($text, FORMAT_HTML, $options); } + $filteroptions['stage'] = 'post_clean'; $text = $filtermanager->filter_text($text, $context, $filteroptions); break; }