mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-04-20 07:56:06 +02:00
Code Issues Projects Releases Wiki Activity

MDL-69050 lang: Stop using the term blacklist in mustache output engine

Browse Source
This commit is contained in:
David Mudrák 2020-09-16 13:17:04 +02:00
parent 7a079a890b
commit 9ce768008c
7 changed files with 118 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/amd/build/templates.min.js.map
View File

File diff suppressed because one or more lines are too long

14
lib/amd/src/templates.js
View File

@ -65,8 +65,8 @@ define([
/** @var {Bool} isLoadingTemplates - Whether templates are currently being loaded */
var isLoadingTemplates = false;
/** @var {Array} blacklistedNestedHelpers - List of helpers that can't be called within other helpers */
var blacklistedNestedHelpers = ['js'];
/** @var {Array} disallowedNestedHelpers - List of helpers that can't be called within other helpers */
var disallowedNestedHelpers = ['js'];
/**
* Search the various caches for a template promise for the given search key.
@ -609,7 +609,7 @@ define([
* template.
*
* This will parse the provided text before giving it to the helper function
* in order to remove any blacklisted nested helpers to prevent one helper
* in order to remove any disallowed nested helpers to prevent one helper
* from calling another.
*
* In particular to prevent the JS helper from being called from within another
@ -623,12 +623,12 @@ define([
Renderer.prototype.addHelperFunction = function(helperFunction, context) {
return function() {
return function(sectionText, helper) {
// Override the blacklisted helpers in the template context with
// Override the disallowed helpers in the template context with
// a function that returns an empty string for use when executing
// other helpers. This is to prevent these helpers from being
// executed as part of the rendering of another helper in order to
// prevent any potential security issues.
var originalHelpers = blacklistedNestedHelpers.reduce(function(carry, name) {
var originalHelpers = disallowedNestedHelpers.reduce(function(carry, name) {
if (context.hasOwnProperty(name)) {
carry[name] = context[name];
}
@ -636,14 +636,14 @@ define([
return carry;
}, {});
blacklistedNestedHelpers.forEach(function(helperName) {
disallowedNestedHelpers.forEach(function(helperName) {
context[helperName] = function() {
return '';
};
});
// Execute the helper with the modified context that doesn't include
// the blacklisted nested helpers. This prevents the blacklisted
// the disallowed nested helpers. This prevents the disallowed
// helpers from being called from within other helpers.
var result = helperFunction.apply(this, [context, sectionText, helper]);

14
lib/classes/output/mustache_engine.php
View File

@ -38,7 +38,7 @@ class mustache_engine extends \Mustache_Engine {
/**
* @var string[] Names of helpers that aren't allowed to be called within other helpers.
*/
private $blacklistednestedhelpers = [];
private $disallowednestedhelpers = [];
/**
* Mustache engine constructor.
@ -47,13 +47,19 @@ class mustache_engine extends \Mustache_Engine {
* $options = [
* // A list of helpers (by name) to prevent from executing within the rendering
* // of other helpers.
* 'blacklistednestedhelpers' => ['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;

60
lib/classes/output/mustache_helper_collection.php
View File

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

2
lib/outputrenderers.php
View File

@ -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']));
}

106
lib/tests/output_mustache_helper_collection_test.php
View File

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

2
lib/upgrade.txt
View File

@ -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().