From 972340b678e2daa96da58af507735aefc70fef4f Mon Sep 17 00:00:00 2001 From: =?utf8?q?David=20Mudr=C3=A1k?= Date: Wed, 16 Sep 2020 13:17:04 +0200 Subject: [PATCH] MDL-69050 lang: Stop using the term blacklist in mustache output engine --- lib/amd/build/templates.min.js.map | Bin 76264 -> 76255 bytes lib/amd/src/templates.js | 14 +-- lib/classes/output/mustache_engine.php | 14 ++- .../output/mustache_helper_collection.php | 60 ++++++---- lib/outputrenderers.php | 2 +- ...output_mustache_helper_collection_test.php | 106 ++++++++++-------- lib/upgrade.txt | 2 + 7 files changed, 117 insertions(+), 81 deletions(-) diff --git a/lib/amd/build/templates.min.js.map b/lib/amd/build/templates.min.js.map index b75e37fda87b1b6fae7f8f1ff022bba3bc40532d..028dd3411dd16b80ee45dfbb05bd2e48a8e1df36 100644 GIT binary patch delta 231 zcmaEHndSavmJMEv94VQ_i8(p><(q>UUvn{*PPXR}Wz52nSL&qW?C9v6?xExB z=vXniPEBm`G7}NTvdOzmR_H(_OPzK6fs$aA>CQUAj*h`5t{`n8AfgZ??wqx`-}Hbf z+@{G9*%5Hg=AGF`!w|goLkpP@Jj0_6NDka}j17r*^0)?qH~H5IRW6uMQd1@yR*G)6 IKe;ji0J3CN-2eap delta 253 zcmcbAndQZ0mJMEvoJl!}$=NxX#U-1A7+-UVmpkb=J36|92xlEnM@JVRtI8>7a|n;F zhA2eRIo(6Y*AXZRQk3pId76nbW5wiMCM$HH@} ['js'] + * 'disallowednestedhelpers' => ['js'] * ]; * @param array $options [description] */ public function __construct(array $options = []) { + if (isset($options['blacklistednestedhelpers'])) { - $this->blacklistednestedhelpers = $options['blacklistednestedhelpers']; + debugging('blacklistednestedhelpers option is deprecated. Use disallowednestedhelpers instead.', DEBUG_DEVELOPER); + $this->disallowednestedhelpers = $options['blacklistednestedhelpers']; + } + + if (isset($options['disallowednestedhelpers'])) { + $this->disallowednestedhelpers = $options['disallowednestedhelpers']; } parent::__construct($options); @@ -69,7 +75,7 @@ class mustache_engine extends \Mustache_Engine { public function getHelpers() { if (!isset($this->helpers)) { - $this->helpers = new mustache_helper_collection(null, $this->blacklistednestedhelpers); + $this->helpers = new mustache_helper_collection(null, $this->disallowednestedhelpers); } return $this->helpers; diff --git a/lib/classes/output/mustache_helper_collection.php b/lib/classes/output/mustache_helper_collection.php index 233d7412380..0c72ff60b62 100644 --- a/lib/classes/output/mustache_helper_collection.php +++ b/lib/classes/output/mustache_helper_collection.php @@ -34,7 +34,7 @@ class mustache_helper_collection extends \Mustache_HelperCollection { /** * @var string[] Names of helpers that aren't allowed to be called within other helpers. */ - private $blacklistednestedhelpers = []; + private $disallowednestedhelpers = []; /** * Helper Collection constructor. @@ -44,43 +44,43 @@ class mustache_helper_collection extends \Mustache_HelperCollection { * @throws \Mustache_Exception_InvalidArgumentException if the $helpers argument isn't an array or Traversable * * @param array|\Traversable $helpers (default: null) - * @param string[] $blacklistednestedhelpers Names of helpers that aren't allowed to be called within other helpers. + * @param string[] $disallowednestedhelpers Names of helpers that aren't allowed to be called within other helpers. */ - public function __construct($helpers = null, array $blacklistednestedhelpers = []) { - $this->blacklistednestedhelpers = $blacklistednestedhelpers; + public function __construct($helpers = null, array $disallowednestedhelpers = []) { + $this->disallowednestedhelpers = $disallowednestedhelpers; parent::__construct($helpers); } /** * Add a helper to this collection. * - * This function has overridden the parent implementation to provide blacklist + * This function has overridden the parent implementation to provide disallowing * functionality for certain helpers to prevent them being called from within * other helpers. This is because the JavaScript helper can be used in a * security exploit if it can be nested. * * The function will wrap callable helpers in an anonymous function that strips - * out the blacklisted helpers from the source string before giving it to the - * helper function. This prevents the blacklisted helper functions from being + * out the disallowed helpers from the source string before giving it to the + * helper function. This prevents the disallowed helper functions from being * called by nested render functions from within other helpers. * * @see \Mustache_HelperCollection::add() * @param string $name * @param mixed $helper */ - public function add($name, $helper) - { - $blacklist = $this->blacklistednestedhelpers; + public function add($name, $helper) { - if (is_callable($helper) && !empty($blacklist)) { - $helper = function($source, \Mustache_LambdaHelper $lambdahelper) use ($helper, $blacklist) { + $disallowedlist = $this->disallowednestedhelpers; - // Temporarily override the blacklisted helpers to return nothing + if (is_callable($helper) && !empty($disallowedlist)) { + $helper = function($source, \Mustache_LambdaHelper $lambdahelper) use ($helper, $disallowedlist) { + + // Temporarily override the disallowed helpers to return nothing // so that they can't be executed from within other helpers. - $disabledhelpers = $this->disable_helpers($blacklist); + $disabledhelpers = $this->disable_helpers($disallowedlist); // Call the original function with the modified sources. $result = call_user_func($helper, $source, $lambdahelper); - // Restore the original blacklisted helper implementations now + // Restore the original disallowed helper implementations now // that this helper has finished executing so that the rest of // the rendering process continues to work correctly. $this->restore_helpers($disabledhelpers); @@ -89,7 +89,7 @@ class mustache_helper_collection extends \Mustache_HelperCollection { // This is done because a secondary render is called on the result // of a helper function if it still includes mustache tags. See // the section function of Mustache_Compiler for details. - return $this->strip_blacklisted_helpers($blacklist, $result); + return $this->strip_disallowed_helpers($disallowedlist, $result); }; } @@ -137,18 +137,18 @@ class mustache_helper_collection extends \Mustache_HelperCollection { } /** - * Parse the given string and remove any reference to blacklisted helpers. + * Parse the given string and remove any reference to disallowed helpers. * * E.g. - * $blacklist = ['js']; + * $disallowedlist = ['js']; * $string = "core, move, {{#js}} some nasty JS hack {{/js}}" * result: "core, move, {{}}" * - * @param string[] $blacklist List of helper names to strip + * @param string[] $disallowedlist List of helper names to strip * @param string $string String to parse * @return string Parsed string */ - public function strip_blacklisted_helpers($blacklist, $string) { + public function strip_disallowed_helpers($disallowedlist, $string) { $starttoken = \Mustache_Tokenizer::T_SECTION; $endtoken = \Mustache_Tokenizer::T_END_SECTION; if ($endtoken == '/') { @@ -160,7 +160,7 @@ class mustache_helper_collection extends \Mustache_HelperCollection { // the user is able to change the delimeters on a per template // basis so they may not be curly braces. return '/\s*' . $starttoken . '\s*'. $name . '\W+.*' . $endtoken . '\s*' . $name . '\s*/'; - }, $blacklist); + }, $disallowedlist); // This will strip out unwanted helpers from the $source string // before providing it to the original helper function. @@ -168,9 +168,25 @@ class mustache_helper_collection extends \Mustache_HelperCollection { // Before: // "core, move, {{#js}} some nasty JS hack {{/js}}" // After: - // "core, move, {{}}" + // "core, move, {{}}". return preg_replace_callback($regexes, function() { return ''; }, $string); } + + /** + * Parse the given string and remove any reference to disallowed helpers. + * + * @deprecated Deprecated since Moodle 3.10 (MDL-69050) - use {@see self::strip_disallowed_helpers()} + * @param string[] $disallowedlist List of helper names to strip + * @param string $string String to parse + * @return string Parsed string + */ + public function strip_blacklisted_helpers($disallowedlist, $string) { + + debugging('mustache_helper_collection::strip_blacklisted_helpers() is deprecated. ' . + 'Please use mustache_helper_collection::strip_disallowed_helpers() instead.', DEBUG_DEVELOPER); + + return $this->strip_disallowed_helpers($disallowedlist, $string); + } } diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php index dd9bd247977..d81e43a10df 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -129,7 +129,7 @@ class renderer_base { // Don't allow the JavaScript helper to be executed from within another // helper. If it's allowed it can be used by users to inject malicious // JS into the page. - 'blacklistednestedhelpers' => ['js'])); + 'disallowednestedhelpers' => ['js'])); } diff --git a/lib/tests/output_mustache_helper_collection_test.php b/lib/tests/output_mustache_helper_collection_test.php index aaac1e989f1..52e39764dfa 100644 --- a/lib/tests/output_mustache_helper_collection_test.php +++ b/lib/tests/output_mustache_helper_collection_test.php @@ -30,94 +30,94 @@ use core\output\mustache_helper_collection; */ class core_output_mustache_helper_collection_testcase extends advanced_testcase { /** - * Test cases to confirm that blacklisted helpers are stripped from the source + * Test cases to confirm that disallowed helpers are stripped from the source * text by the helper before being passed to other another helper. This prevents * nested calls to helpers. */ - public function get_strip_blacklisted_helpers_testcases() { + public function get_strip_disallowed_helpers_testcases() { return [ - 'no blacklist' => [ - 'blacklist' => [], + 'no disallowed' => [ + 'disallowed' => [], 'input' => 'core, move, {{#js}} some nasty JS {{/js}}', 'expected' => 'core, move, {{#js}} some nasty JS {{/js}}' ], - 'blacklist no match' => [ - 'blacklist' => ['foo'], + 'disallowed no match' => [ + 'disallowed' => ['foo'], 'input' => 'core, move, {{#js}} some nasty JS {{/js}}', 'expected' => 'core, move, {{#js}} some nasty JS {{/js}}' ], - 'blacklist partial match 1' => [ - 'blacklist' => ['js'], + 'disallowed partial match 1' => [ + 'disallowed' => ['js'], 'input' => 'core, move, {{#json}} some nasty JS {{/json}}', 'expected' => 'core, move, {{#json}} some nasty JS {{/json}}' ], - 'blacklist partial match 2' => [ - 'blacklist' => ['js'], + 'disallowed partial match 2' => [ + 'disallowed' => ['js'], 'input' => 'core, move, {{#onjs}} some nasty JS {{/onjs}}', 'expected' => 'core, move, {{#onjs}} some nasty JS {{/onjs}}' ], - 'single blacklist 1' => [ - 'blacklist' => ['js'], + 'single disallowed 1' => [ + 'disallowed' => ['js'], 'input' => 'core, move, {{#js}} some nasty JS {{/js}}', 'expected' => 'core, move, {{}}' ], - 'single blacklist 2' => [ - 'blacklist' => ['js'], + 'single disallowed 2' => [ + 'disallowed' => ['js'], 'input' => 'core, move, {{ # js }} some nasty JS {{ / js }}', 'expected' => 'core, move, {{}}' ], - 'single blacklist 3' => [ - 'blacklist' => ['js'], + 'single disallowed 3' => [ + 'disallowed' => ['js'], 'input' => 'core, {{#js}} some nasty JS {{/js}}, test', 'expected' => 'core, {{}}, test' ], - 'single blacklist 3' => [ - 'blacklist' => ['js'], + 'single disallowed 3' => [ + 'disallowed' => ['js'], 'input' => 'core, {{#ok}} this is ok {{/ok}}, {{#js}} some nasty JS {{/js}}', 'expected' => 'core, {{#ok}} this is ok {{/ok}}, {{}}' ], - 'single blacklist multiple matches 1' => [ - 'blacklist' => ['js'], + 'single disallowed multiple matches 1' => [ + 'disallowed' => ['js'], 'input' => 'core, {{#js}} some nasty JS {{/js}}, {{#js}} some nasty JS {{/js}}', 'expected' => 'core, {{}}' ], - 'single blacklist multiple matches 2' => [ - 'blacklist' => ['js'], + 'single disallowed multiple matches 2' => [ + 'disallowed' => ['js'], 'input' => 'core, {{ # js }} some nasty JS {{ / js }}, {{ # js }} some nasty JS {{ / js }}', 'expected' => 'core, {{}}' ], - 'single blacklist multiple matches nested 1' => [ - 'blacklist' => ['js'], + 'single disallowed multiple matches nested 1' => [ + 'disallowed' => ['js'], 'input' => 'core, move, {{#js}} some nasty JS {{#js}} some nasty JS {{/js}} {{/js}}', 'expected' => 'core, move, {{}}' ], - 'single blacklist multiple matches nested 2' => [ - 'blacklist' => ['js'], + 'single disallowed multiple matches nested 2' => [ + 'disallowed' => ['js'], 'input' => 'core, move, {{ # js }} some nasty JS {{ # js }} some nasty JS {{ / js }}{{ / js }}', 'expected' => 'core, move, {{}}' ], - 'multiple blacklist 1' => [ - 'blacklist' => ['js', 'foo'], + 'multiple disallowed 1' => [ + 'disallowed' => ['js', 'foo'], 'input' => 'core, move, {{#js}} some nasty JS {{/js}}', 'expected' => 'core, move, {{}}' ], - 'multiple blacklist 2' => [ - 'blacklist' => ['js', 'foo'], + 'multiple disallowed 2' => [ + 'disallowed' => ['js', 'foo'], 'input' => 'core, {{#foo}} blah {{/foo}}, {{#js}} js {{/js}}', 'expected' => 'core, {{}}, {{}}' ], - 'multiple blacklist 3' => [ - 'blacklist' => ['js', 'foo'], + 'multiple disallowed 3' => [ + 'disallowed' => ['js', 'foo'], 'input' => '{{#foo}} blah {{/foo}}, {{#foo}} blah {{/foo}}, {{#js}} js {{/js}}', 'expected' => '{{}}, {{}}' ], - 'multiple blacklist 4' => [ - 'blacklist' => ['js', 'foo'], + 'multiple disallowed 4' => [ + 'disallowed' => ['js', 'foo'], 'input' => '{{#foo}} blah {{/foo}}, {{#js}} js {{/js}}, {{#foo}} blah {{/foo}}', 'expected' => '{{}}' ], - 'multiple blacklist 4' => [ - 'blacklist' => ['js', 'foo'], + 'multiple disallowed 4' => [ + 'disallowed' => ['js', 'foo'], 'input' => 'core, move, {{#js}} JS {{#foo}} blah {{/foo}} {{/js}}', 'expected' => 'core, move, {{}}' ], @@ -126,29 +126,29 @@ class core_output_mustache_helper_collection_testcase extends advanced_testcase /** * Test that the mustache_helper_collection class correctly strips - * @dataProvider get_strip_blacklisted_helpers_testcases() - * @param string[] $blacklist The list of helpers to strip + * @dataProvider get_strip_disallowed_helpers_testcases() + * @param string[] $disallowed The list of helpers to strip * @param string $input The input string for the helper - * @param string $expected The expected output of the string after blacklist strip + * @param string $expected The expected output of the string after disallowed strip */ - public function test_strip_blacklisted_helpers($blacklist, $input, $expected) { - $collection = new mustache_helper_collection(null, $blacklist); - $this->assertEquals($expected, $collection->strip_blacklisted_helpers($blacklist, $input)); + public function test_strip_disallowed_helpers($disallowed, $input, $expected) { + $collection = new mustache_helper_collection(null, $disallowed); + $this->assertEquals($expected, $collection->strip_disallowed_helpers($disallowed, $input)); } /** - * Test that the blacklisted helpers are disabled during the execution of other + * Test that the disallowed helpers are disabled during the execution of other * helpers. * - * Any non-blacklisted helper should still be available to call during the + * Any allowed helper should still be available to call during the * execution of a helper. */ - public function test_blacklisted_helpers_disabled_during_execution() { + public function test_disallowed_helpers_disabled_during_execution() { $engine = new \Mustache_Engine(); $context = new \Mustache_Context(); $lambdahelper = new \Mustache_LambdaHelper($engine, $context); - $blacklist = ['bad']; - $collection = new mustache_helper_collection(null, $blacklist); + $disallowed = ['bad']; + $collection = new mustache_helper_collection(null, $disallowed); $badcalled = false; $goodcalled = false; @@ -174,4 +174,16 @@ class core_output_mustache_helper_collection_testcase extends advanced_testcase $this->assertTrue($goodcalled); $this->assertFalse($badcalled); } + + /** + * Test that calling deprecated method strip_blacklisted_helpers() still works and shows developer debugging. + */ + public function test_deprecated_strip_blacklisted_helpers() { + + $collection = new mustache_helper_collection(null, ['js']); + $stripped = $collection->strip_blacklisted_helpers(['js'], '{{#js}} JS {{/js}}'); + $this->assertEquals('{{}}', $stripped); + $this->assertDebuggingCalled('mustache_helper_collection::strip_blacklisted_helpers() is deprecated. ' . + 'Please use mustache_helper_collection::strip_disallowed_helpers() instead.', DEBUG_DEVELOPER); + } } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 699f5c6b6b3..c5fe41f75e1 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -50,6 +50,8 @@ information provided here is intended especially for developers. * The http-message library has been added to Moodle core in /lib/http-message. * Methods `filetypes_util::is_whitelisted()` and `filetypes_util::get_not_whitelisted()` have been deprecated and renamed to `is_listed()` and `get_not_listed()` respectively. +* Method `mustache_helper_collection::strip_blacklisted_helpers()` has been deprecated and renamed to + `strip_disallowed_helpers()`. === 3.9 === * Following function has been deprecated, please use \core\task\manager::run_from_cli(). -- 2.17.1