MDL-87066 core: add clean_string() to solve double encoding in Mustache

The idea is to prevent double escaping in s()
and mustache.escape() by using numeric html
entities to sanitise result of format_string(),
get_string() and similar.

lib/amd/build/local/templates/renderer.min.js

@@ -1,4 +1,4 @@
-define("core/local/templates/renderer",["exports","core/log","core/truncate","core/user_date","core/pending","core/str","core/icon_system","core/config","core/mustache","./loader","core/utils"],(function(_exports,Log,Truncate,UserDate,_pending,_str,_icon_system,_config,_mustache,_loader,_utils){function _interopRequireDefault(obj){return obj&&obj.__esModule?obj:{default:obj}}function _getRequireWildcardCache(nodeInterop){if("function"!=typeof WeakMap)return null;var cacheBabelInterop=new WeakMap,cacheNodeInterop=new WeakMap;return(_getRequireWildcardCache=function(nodeInterop){return nodeInterop?cacheNodeInterop:cacheBabelInterop})(nodeInterop)}function _interopRequireWildcard(obj,nodeInterop){if(!nodeInterop&&obj&&obj.__esModule)return obj;if(null===obj||"object"!=typeof obj&&"function"!=typeof obj)return{default:obj};var cache=_getRequireWildcardCache(nodeInterop);if(cache&&cache.has(obj))return cache.get(obj);var newObj={},hasPropertyDescriptor=Object.defineProperty&&Object.getOwnPropertyDescriptor;for(var key in obj)if("default"!==key&&Object.prototype.hasOwnProperty.call(obj,key)){var desc=hasPropertyDescriptor?Object.getOwnPropertyDescriptor(obj,key):null;desc&&(desc.get||desc.set)?Object.defineProperty(newObj,key,desc):newObj[key]=obj[key]}return newObj.default=obj,cache&&cache.set(obj,newObj),newObj}function _defineProperty(obj,key,value){return key in obj?Object.defineProperty(obj,key,{value:value,enumerable:!0,configurable:!0,writable:!0}):obj[key]=value,obj}Object.defineProperty(_exports,"__esModule",{value:!0}),_exports.default=void 0,Log=_interopRequireWildcard(Log),Truncate=_interopRequireWildcard(Truncate),UserDate=_interopRequireWildcard(UserDate),_pending=_interopRequireDefault(_pending),_icon_system=_interopRequireDefault(_icon_system),_config=_interopRequireDefault(_config),_mustache=_interopRequireDefault(_mustache),_loader=_interopRequireDefault(_loader);
+define("core/local/templates/renderer",["exports","core/log","core/truncate","core/user_date","core/pending","core/str","core/icon_system","core/config","core/mustache","./loader","core/utils"],(function(_exports,Log,Truncate,UserDate,_pending,_str,_icon_system,_config,_mustache,_loader,_utils){function _interopRequireDefault(obj){return obj&&obj.__esModule?obj:{default:obj}}function _getRequireWildcardCache(nodeInterop){if("function"!=typeof WeakMap)return null;var cacheBabelInterop=new WeakMap,cacheNodeInterop=new WeakMap;return(_getRequireWildcardCache=function(nodeInterop){return nodeInterop?cacheNodeInterop:cacheBabelInterop})(nodeInterop)}function _interopRequireWildcard(obj,nodeInterop){if(!nodeInterop&&obj&&obj.__esModule)return obj;if(null===obj||"object"!=typeof obj&&"function"!=typeof obj)return{default:obj};var cache=_getRequireWildcardCache(nodeInterop);if(cache&&cache.has(obj))return cache.get(obj);var newObj={},hasPropertyDescriptor=Object.defineProperty&&Object.getOwnPropertyDescriptor;for(var key in obj)if("default"!==key&&Object.prototype.hasOwnProperty.call(obj,key)){var desc=hasPropertyDescriptor?Object.getOwnPropertyDescriptor(obj,key):null;desc&&(desc.get||desc.set)?Object.defineProperty(newObj,key,desc):newObj[key]=obj[key]}return newObj.default=obj,cache&&cache.set(obj,newObj),newObj}function _defineProperty(obj,key,value){return key in obj?Object.defineProperty(obj,key,{value:value,enumerable:!0,configurable:!0,writable:!0}):obj[key]=value,obj}Object.defineProperty(_exports,"__esModule",{value:!0}),_exports.default=void 0,Log=_interopRequireWildcard(Log),Truncate=_interopRequireWildcard(Truncate),UserDate=_interopRequireWildcard(UserDate),_pending=_interopRequireDefault(_pending),_icon_system=_interopRequireDefault(_icon_system),_config=_interopRequireDefault(_config),_mustache=_interopRequireDefault(_mustache),_loader=_interopRequireDefault(_loader);const originalMustacheEscape=_mustache.default.escape;_mustache.default.escape=function(string){return string=(string=originalMustacheEscape(string)).replace(/&#([0-9]+|x[0-9a-fA-F]+);/g,"&#$1;")};
 /**
    * Template Renderer Class.
    *

lib/amd/src/local/templates/renderer.js

@@ -30,6 +30,16 @@ const placeholderString = 's';
 /** @var {string} The placeholder character used for cleaned strings */
 const placeholderCleanedString = 'c';
 
+/** @var {Function} originalMustacheEscape */
+const originalMustacheEscape = mustache.escape;
+
+// Replicate escaping logic of PHP s() function.
+mustache.escape = function(string) {
+    string = originalMustacheEscape(string);
+    string = string.replace(/&#([0-9]+|x[0-9a-fA-F]+);/g, '&#$1;');
+    return string;
+};
+
 /**
  * Template Renderer Class.
  *

lib/classes/text.php

@@ -461,6 +461,35 @@ class core_text {
         return $translationtable;
     }
 
+    /**
+     * Returns transliteration table for conversion of named
+     * html entities to numeric html entities.
+     * @return array
+     */
+    protected static function get_named_entities_table(): array {
+        static $translationtable = null;
+
+        if (!isset($translationtable)) {
+            $translationtable = [];
+            // NOTE: do not use ENT_HTML5 here because it adds way too many items.
+            $entities = get_html_translation_table(HTML_ENTITIES, ENT_COMPAT | ENT_HTML401, 'UTF-8');
+            foreach ($entities as $char => $entity) {
+                $translationtable[$entity] = '&#' . IntlChar::ord($char) . ';';
+            }
+        }
+
+        return $translationtable;
+    }
+
+    /**
+     * Converts all named html entities " to numeric entities &#nnnn;
+     * @param string $str input string
+     * @return string
+     */
+    public static function entities_named_to_numeric(string $str): string {
+        return strtr($str, self::get_named_entities_table());
+    }
+
     /**
      * Converts all the numeric entities &#nnnn; or &#xnnn; to UTF-8
      * Original from laurynas dot butkus at gmail at:

lib/tests/output/mustache_escape_test.php

@@ -0,0 +1,148 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+namespace core\output;
+
+/**
+ * Test escaping of Mustache template placeholders.
+ *
+ * @package   core
+ * @copyright 2025 Petr Skoda
+ * @license   https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+final class mustache_escape_test extends \advanced_testcase {
+    /**
+     * Test escaping of characters in {{ }} placeholders.
+     *
+     * @covers \renderer_base::render_from_template
+     */
+    public function test_escape(): void {
+        $page = new \moodle_page();
+        $page->set_url('/');
+        $page->set_context(null);
+        $renderer = new renderer_base($page, RENDERER_TARGET_GENERAL);
+
+        // Get the mustache engine from the renderer.
+        $reflection = new \ReflectionMethod($renderer, 'get_mustache');
+        /** @var \Mustache_Engine $engine */
+        $engine = $reflection->invoke($renderer);
+
+        // Swap to custom loader.
+        $loader = new \Mustache_Loader_ArrayLoader([
+            'core/test' => '<a href="#" title="{{title}}">test</a>',
+        ]);
+        $engine->setLoader($loader);
+
+        $teststring = 'Test title < > " \' & &lt;';
+
+        $result = $renderer->render_from_template(
+            'core/test',
+            ['title' => $teststring],
+        );
+        $this->assertSame(
+            '<a href="#" title="Test title &lt; &gt; &quot; &#039; &amp; &amp;lt;">test</a>',
+            $result
+        );
+
+        $result = $renderer->render_from_template(
+            'core/test',
+            ['title' => s($teststring)],
+        );
+        $this->assertSame(
+            '<a href="#" title="Test title &amp;lt; &amp;gt; &amp;quot; &#039; &amp;amp; &amp;amp;lt;">test</a>',
+            $result
+        );
+
+        $result = $renderer->render_from_template(
+            'core/test',
+            ['title' => clean_string($teststring)],
+        );
+        $this->assertSame(
+            '<a href="#" title="Test title &#60; &#62; &#34; &#39; &#38; &#60;">test</a>',
+            $result
+        );
+
+        $result = $renderer->render_from_template(
+            'core/test',
+            ['title' => clean_string(clean_string($teststring))],
+        );
+        $this->assertSame(
+            '<a href="#" title="Test title &#60; &#62; &#34; &#39; &#38; &#60;">test</a>',
+            $result
+        );
+
+        $result = $renderer->render_from_template(
+            'core/test',
+            ['title' => s(clean_string($teststring))],
+        );
+        $this->assertSame(
+            '<a href="#" title="Test title &#60; &#62; &#34; &#39; &#38; &#60;">test</a>',
+            $result
+        );
+
+        $result = $renderer->render_from_template(
+            'core/test',
+            ['title' => format_string(clean_string($teststring))],
+        );
+        $this->assertSame(
+            '<a href="#" title="Test title &#60; &#62; &#34; &#39; &#38; &#60;">test</a>',
+            $result
+        );
+    }
+
+    /**
+     * Test that there is no escaping of characters in {{{ }}} placeholders.
+     *
+     * @covers \renderer_base::render_from_template
+     */
+    public function test_no_escape(): void {
+        $page = new \moodle_page();
+        $page->set_url('/');
+        $page->set_context(null);
+        $renderer = new renderer_base($page, RENDERER_TARGET_GENERAL);
+
+        // Get the mustache engine from the renderer.
+        $reflection = new \ReflectionMethod($renderer, 'get_mustache');
+        /** @var \Mustache_Engine $engine */
+        $engine = $reflection->invoke($renderer);
+
+        // Swap to custom loader.
+        $loader = new \Mustache_Loader_ArrayLoader([
+            'core/test' => 'Some {{{html}}} test',
+        ]);
+        $engine->setLoader($loader);
+
+        $teststring = 'Test title < > " \' & &lt;';
+
+        $result = $renderer->render_from_template(
+            'core/test',
+            ['html' => $teststring],
+        );
+        $this->assertSame(
+            'Some Test title < > " \' & &lt; test',
+            $result
+        );
+
+        $result = $renderer->render_from_template(
+            'core/test',
+            ['html' => clean_string($teststring)],
+        );
+        $this->assertSame(
+            'Some Test title &#60; &#62; &#34; &#39; &#38; &#60; test',
+            $result
+        );
+    }
+}

lib/tests/text_test.php

@@ -465,6 +465,38 @@ final class text_test extends advanced_testcase {
         $this->assertSame('', core_text::encode_mimeheader(null));
     }
 
+    /**
+     * Tests conversion of named to numeric html entities.
+     *
+     * @covers ::entities_named_to_numeric()
+     */
+    public function test_entities_named_to_numeric(): void {
+        $str = '& " < >';
+        $this->assertSame('& " < >', core_text::entities_named_to_numeric($str));
+
+        $str = 'Žluťoučký koníček testing Ι';
+        $this->assertSame('Žluťoučký koníček testing Ι', core_text::entities_named_to_numeric($str));
+
+        $str = "Žluťoučký koníček©"&<>§«";
+        $this->assertSame(
+            "Žluťoučký koníček©"&<>§«",
+            core_text::entities_named_to_numeric($str)
+        );
+
+        $this->assertSame(
+            core_text::entities_to_utf8($str),
+            core_text::entities_to_utf8(core_text::entities_named_to_numeric($str))
+        );
+
+        $table = get_html_translation_table(HTML_ENTITIES, ENT_COMPAT | ENT_HTML401, 'UTF-8');
+        $entities = implode(' ', $table);
+        $chars = implode(' ', array_keys($table));
+        $this->assertSame(
+            core_text::entities_to_utf8($chars),
+            core_text::entities_to_utf8(core_text::entities_named_to_numeric($entities))
+        );
+    }
+
     /**
      * Tests the static entities_to_utf8 method.
      *

lib/tests/weblib_test.php

@@ -42,9 +42,8 @@ final class weblib_test extends advanced_testcase {
         $this->assertSame('Café', s('Café'));
         $this->assertSame('一, 二, 三', s('一, 二, 三'));
 
-        // Don't escape already-escaped numeric entities. (Note, this behaviour
-        // may not be desirable. Perhaps we should remove these tests and that
-        // functionality, but we can only do that if we understand why it was added.)
+        // Don't escape already-escaped numeric entities. This is a feature
+        // necessary to prevent double-escaping in Mustache templates via clean_string().
         $this->assertSame('An entity: ৿.', s('An entity: ৿.'));
         $this->assertSame('An entity: б.', s('An entity: б.'));
         $this->assertSame('An entity: &.', s('An entity: &.'));
@@ -184,6 +183,36 @@ final class weblib_test extends advanced_testcase {
         );
     }
 
+    /**
+     * Test conversion of dangerous characters and named entities to numeric entities.
+     *
+     * @covers ::clean_string
+     */
+    public function test_clean_string(): void {
+        $string = 'Žluťoučký koníček <tag> "test" \'example\' & escaped &amp; &lt; &gt; &quot; ';
+        $cleaned = clean_string($string);
+
+        $this->assertSame(
+            'Žluťoučký koníček &#60;tag&#62; &#34;test&#34; &#39;example&#39; &#38; escaped &#38; &#60; &#62; &#34; ',
+            $cleaned
+        );
+
+        // Repeated cleaning does not change result.
+        $this->assertSame($cleaned, clean_string($cleaned));
+
+        // Function s() does not modify it.
+        $this->assertSame($cleaned, s($cleaned));
+
+        // Function format_string() does not modify it.
+        $this->assertSame($cleaned, format_string($cleaned));
+
+        // Function clean_text() does not remove data.
+        $this->assertSame($cleaned, clean_string(clean_text($cleaned)));
+
+        // It can be converted back to raw UTF-8 characters.
+        $this->assertSame(core_text::entities_to_utf8($string), core_text::entities_to_utf8($cleaned));
+    }
+
     /**
      * @covers ::format_text_email
      */

lib/weblib.php

@@ -760,6 +760,35 @@ function format_string($string, $striplinks = true, $options = null) {
     );
 }
 
+/**
+ * Encode all dangerous characters and named html entities as
+ * numeric html entities.
+ *
+ * The result of this function can be used safely in both {{ }} and {{{ }}} tags in Mustache templates
+ * because it is not modified by s() function and it is equivalent to htmlentities() escaping.
+ *
+ * @param string|null $string
+ * @return string|null html string without any tags or dangerous characters
+ */
+function clean_string(?string $string): ?string {
+    if ($string === null || $string === '') {
+        return $string;
+    }
+
+    $replace = [
+        '"' => '"',
+        '\'' => ''',
+        '<' => '&#60;',
+        '>' => '&#62;',
+    ];
+    $string = strtr($string, $replace);
+    $string = preg_replace('/&(?![a-zA-Z0-9#]{1,8};)/', '&#38;', $string);
+
+    $string = core_text::entities_named_to_numeric($string);
+
+    return $string;
+}
+
 /**
  * Given a string, performs a negative lookahead looking for any ampersand character
  * that is not followed by a proper HTML entity. If any is found, it is replaced