MDL Shield

Tiny Font Color

tiny_fontcolor

Print Report
Plugin Information

TinyMCE (Tiny) editor subplugin that adds text foreground and background colour controls to the Moodle editor. Administrators define named colour palettes (with optional alpha channel), can enable a custom colour picker, can switch to a CSS-class output mode that writes colour classes into every theme's SCSS setting, and can feed the palette into the TinyMCE table plugin's border and cell-background colour pickers. Colour values are validated as hex codes and colour names support Moodle multilang markup.

Version:2026071200
Release:1.4
Reviewed for:5.2
Privacy API
Unit Tests
Behat Tests
Reviewed:2026-07-15
36 files·6,722 lines
Grade Justification

The plugin is well engineered and follows Moodle security conventions throughout. All persistence goes through the configuration API (get_config/set_config/config_read/config_write) — there is no raw SQL, no direct database or filesystem access, and no outbound HTTP. Colour values are validated against a strict hex regular expression before they are stored or emitted into CSS, and colour names (which may contain multilang HTML) are consistently passed through format_string() with escaping enabled, through Mustache auto-escaping, or through strip_tags() plus an [a-z] filter when derived into CSS class names. The tiny/fontcolor:use capability is declared in db/access.php and enforced by the editor_tiny\plugin base class, the admin settings save path is CSRF-protected by the framework's confirm_sesskey(), and a null_provider Privacy implementation is correctly present because the plugin stores only site configuration. The bundled jscolor library is properly declared in thirdpartylibs.xml. No critical, high, or medium issues were identified. The findings are all low-severity code-quality and best-practice items confined to the admin-only settings flow or to the user's own browser session: direct $_REQUEST access in the custom admin setting, two no-op statements (one silently swallowing a config_write() failure), a self-XSS-only innerHTML sink in a colour-picker error message, a root-relative <script> include that breaks on subdirectory installs, and the absence of uninstall cleanup for CSS written into theme SCSS. None of these is exploitable to harm another user, so the plugin sits at the top of the quality range with only minor technical-debt items to address.

AI Summary

Overview

tiny_fontcolor is a TinyMCE editor subplugin that adds text colour and text-background colour selection to the Moodle editor. Administrators configure named colour palettes (optionally with an alpha channel), can enable a custom colour picker, can switch to a CSS-class output mode that injects colour classes into every theme's SCSS, and can feed the palette into the TinyMCE table plugin's border/cell-background colour pickers.

Security posture

The plugin is soundly constructed:

  • No dangerous sinks. All persistence uses the config API (get_config/set_config/config_read/config_write). There is no raw SQL, no direct $DB/filesystem/file-store access, no curl/file_get_contents on URLs, and no eval/exec.
  • Input validation. Colour values are validated against /^#?[0-9a-f]{6}([0-9a-f]{2})?$/i before storage and before being written into CSS, so no arbitrary content reaches a style/CSS context.
  • Output encoding. Colour names (which may contain multilang HTML) are emitted through format_string() with escaping enabled, through Mustache auto-escaping in the settings template, and through strip_tags() + [a-z] filtering when converted to CSS class names.
  • Access control. The tiny/fontcolor:use capability is declared and enforced by the editor_tiny\plugin base class; the settings page is gated by moodle/site:config and the save is protected by the admin framework's confirm_sesskey().
  • Privacy. A null_provider is implemented, which is correct — only site configuration is stored.
  • Third-party code. The bundled jscolor 2.5.2 picker is declared in thirdpartylibs.xml; Moodle core does not ship jscolor, so there is no duplication.

Findings

No critical, high, or medium issues were found. Five low-severity items were identified:

  1. Direct $_REQUEST access instead of Moodle parameter helpers in the custom admin setting.
  2. Two no-op statements, one of which silently discards a config_write() failure.
  3. A self-XSS-only innerHTML assignment in the colour-picker error handler.
  4. Missing db/uninstall.php to remove CSS injected into theme SCSS settings.
  5. A root-relative <script> include for jscolor that ignores a wwwroot subdirectory.

The plugin ships PHPUnit and Behat tests and is otherwise a clean, idiomatic Moodle editor subplugin.

Findings

code qualityLow
Direct `$_REQUEST` superglobal access instead of Moodle parameter APIs

The custom admin_setting_colorlist reads submitted data directly from the $_REQUEST superglobal rather than using Moodle's optional_param()/required_param() (or data_submitted()) helpers with a declared PARAM_* type.

Moodle discourages direct superglobal access because the parameter helpers apply type cleaning and are the project-wide convention. Here the setting iterates every request key to collect the dynamically-named colour name/value fields, and separately reads the usecssclassnames toggle from $_REQUEST.

The practical risk is limited: this code is only reachable on the site administration settings page (which requires the moodle/site:config capability), the save is CSRF-protected by the admin framework's confirm_sesskey(), and every collected value is subsequently validated against a strict hex regex (values) or an empty check (names) before being persisted. It is therefore a code-quality / robustness issue rather than an exploitable vulnerability.

Risk Assessment

Low risk. The blast radius is limited to site administrators, the entry point is CSRF-protected, and all values are validated before storage, so there is no injection or privilege-escalation path. This is flagged for maintainability and consistency with Moodle coding standards rather than for a concrete exploit.

Context

admin_setting_colorlist extends the core admin_setting class and renders a variable-length list of colour name/value input pairs. Because the number of colours (and thus the field names such as s_tiny_fontcolor_textcolors_name_1) is dynamic, the setting scans $_REQUEST to reassemble the list on save (get_setting_val_from_request()), and reads the sibling usecssclassnames checkbox to decide whether to also write theme CSS (use_css_classnames()). The data reaches this code only after the admin settings controller has already enforced the session key and the moodle/site:config capability.

Identified Code
            foreach ($_REQUEST as $key => $val) {
                if (strpos($key, $this->name . '_name_') !== false) {
                    $names[$key] = trim($val);
                } else if (strpos($key, $this->name . '_value_') !== false) {
                    $values[$key] = trim($val);
                }
            }
Suggested Fix

Use data_submitted() to obtain the posted object and iterate its properties, or clean each collected value with clean_param($val, PARAM_RAW_TRIMMED) (names) / a hex check (values). The dynamic field names make optional_param() awkward, but the raw superglobal should still be routed through Moodle's cleaning helpers rather than read directly.

Identified Code
        $name = substr($this->get_full_name(), 0, strrpos($this->get_full_name(), '_')) . '_usecssclassnames';
        if (isset($_REQUEST) && isset($_REQUEST[$name])) {
            return (bool)$_REQUEST[$name];
        }
Suggested Fix
        $value = optional_param($name, null, PARAM_BOOL);
        if ($value !== null) {
            return (bool)$value;
        }
code qualityLow
No-op statements discard return values, silently swallowing a `config_write()` failure

Two locations contain expression statements that evaluate a variable but do nothing with the result — they appear to be missing a return.

In write_setting(), when config_write() fails, the error message is assigned to $res and then the bare statement $res; is executed inside the if block. This is a no-op: the computed error string is discarded, so a failed write of the colour list is silently ignored and the method continues as though it succeeded.

In color_list::get_css_class_list(), the bare statement $list; inside the if ($this->colors === null) branch is also a no-op (and the branch is effectively dead because $colors is initialised to an array and never assigned null).

Both are minor logic defects. The first has a small functional consequence (a storage error is not surfaced to the administrator); the second is dead code.

Risk Assessment

Low risk. No security impact. The only observable effect is that a rare config_write() failure would not be reported back to the administrator (the setting page would appear to save successfully). The dead branch in get_css_class_list() has no effect at all. These indicate minor carelessness rather than a functional defect in normal operation.

Context

write_setting() is the persistence entry point invoked by the admin settings framework when the colour list is saved; it validates the submitted colours and then writes them as JSON. get_css_class_list() derives CSS class names from colour names and is used both for the editor configuration and for generating the theme SCSS block.

Identified Code
        $res = ($this->config_write($this->name, $data->to_json()) ? '' : get_string('errorsetting', 'admin'));
        if (!empty($res)) {
            $res;
        }
Suggested Fix
        $res = ($this->config_write($this->name, $data->to_json()) ? '' : get_string('errorsetting', 'admin'));
        if (!empty($res)) {
            return $res;
        }
Identified Code
        $list = [];
        if ($this->colors === null) {
            $list;
        }
Suggested Fix

Return early with the empty list, or remove the dead branch entirely since $colors is always an array:

        $list = [];
        if ($this->colors === null) {
            return $list;
        }
best practiceLow
Unsanitized input assigned to `innerHTML` in the colour-picker error handler (self-XSS only)

In the custom colour-picker dialog, validation error messages are built by interpolating the user's own field input into a language string and assigning the result to element.innerHTML. The RGB field value (i.value) and the hex value (hex) are inserted without escaping.

Because the values originate from inputs the user is typing into their own editor dialog, and the reflected result is written back into that same user's DOM, this is a classic self-XSS: there is no vector by which one user can cause markup to be injected into another user's page. There is no stored or reflected cross-user path — the erroneous values are never persisted or transmitted to anyone else.

Even so, assigning untrusted input to innerHTML is an unsafe pattern that should be avoided; using textContent (or escaping the interpolated value) removes the sink entirely.

Risk Assessment

Low risk. This is self-XSS: the reflected value is the user's own keystrokes rendered back into their own DOM, with no persistence and no cross-user delivery mechanism, so it does not constitute a real-world vulnerability. Reaching the dialog additionally requires the tiny/fontcolor:use capability (editing teacher and above by default) and the colour-picker option to be enabled. It is reported as a defensive-coding cleanup: prefer textContent over innerHTML for user-derived strings.

Context

colorPickerDialog() opens a TinyMCE dialog containing an RGB/hex colour picker and an error <span> (.dlg-color-picker-error). On submit, onSubmit() reads the picker inputs, and for any value that fails validation it writes a localized error message — with the offending value embedded — into the error span. The inputs are entirely user-supplied and local to the current session.

Identified Code
        if (!i.value.match(/^\d{1,3}$/) || r < 0 || r > 255) {
          err.innerHTML = labels.get('colorPickerErrRgbCode', m[x] + ' = ' + i.value);
          i.focus();
          isValid = false;
        }
Suggested Fix

Build the message text and assign it with textContent, or HTML-escape the interpolated value before insertion:

          err.textContent = labels.get('colorPickerErrRgbCode', m[x] + ' = ' + i.value);

Apply the same change to the hex-code branch.

Identified Code
      } else if (!isHexString('#' + i.value)) {
        err.innerHTML = labels.get('colorPickerErrHexCode', hex);
        i.focus();
        isValid = false;
      }
Suggested Fix
        err.textContent = labels.get('colorPickerErrHexCode', hex);
best practiceLow
No `db/uninstall.php` to remove CSS injected into theme SCSS settings

When the usecssclassnames option is enabled, the plugin writes a block of generated CSS (delimited by /* automatically set by ... */ markers) into the scss configuration of every installed theme via set_config('scss', ..., 'theme_<name>'). This is data stored outside the plugin's own tiny_fontcolor/* configuration namespace.

When the plugin is uninstalled, Moodle automatically removes the plugin's own config entries, but it does not touch the theme_<name>/scss values. The injected CSS blocks therefore remain orphaned in each theme's SCSS after removal.

The project is aware of this — the README states "Deinstalling the plugin will leave the colors because the css is stored in the theme settings" — so it is a documented limitation. A db/uninstall.php that strips the marked blocks from each theme's SCSS would make removal clean.

Risk Assessment

Low risk. No security impact. The consequence is residual configuration left behind on uninstall (marked CSS blocks in theme SCSS), which an administrator would have to remove manually. It only applies to sites that enabled the CSS-class mode. Reported as a housekeeping/best-practice gap; the behaviour is already documented in the README.

Context

save_css_classnames() is called from write_setting() whenever the colour list is saved and usecssclassnames is on. It appends or replaces a marked CSS block in the SCSS setting of all themes returned by core_component::get_plugin_list('theme'). There is no corresponding teardown routine.

Identified Code
    protected function save_css_classnames(): bool {
        $themes = \core_component::get_plugin_list('theme');
        foreach (\array_keys($themes) as $theme) {
            $key = 'theme_' . $theme;
            $scss = get_config($key, 'scss');
            ...
            set_config('scss', $scss, $key);
Suggested Fix

Add a db/uninstall.php implementing xmldb_tiny_fontcolor_uninstall() that iterates the installed themes and removes the CSS between the get_custom_css_marker('start') and get_custom_css_marker('end') markers from each theme_<name>/scss value, restoring it to its pre-plugin state.

code qualityLow
Bundled jscolor loaded via a root-relative `<script>` path that ignores wwwroot subdirectories

The settings template loads the bundled jscolor picker with a raw <script> tag whose src is built from plugininfo::get_base_dir(). That helper strips $CFG->dirroot from the plugin's absolute filesystem path and returns a path such as /lib/editor/tiny/plugins/fontcolor, which is then used directly as a root-relative URL.

Because the path is not prefixed with $CFG->wwwroot, it resolves against the web-server document root. On a Moodle installed in a subdirectory (for example https://example.org/moodle), the browser requests https://example.org/lib/editor/... instead of https://example.org/moodle/lib/editor/..., so the script fails to load and the jscolor enhancement on the settings inputs is lost.

Separately, loading a script through a hand-assembled <script> tag in a template bypasses Moodle's JavaScript loading mechanisms ($PAGE->requires->js() / AMD), which is not the idiomatic approach.

Risk Assessment

Low risk. No security impact. This is a portability/robustness defect: on subdirectory installs the jscolor picker silently fails to load on the plugin's settings page (admin-only), degrading a convenience feature while the underlying text inputs continue to work. Sites installed at the web root are unaffected.

Context

admin_setting_colorlist::output_html() renders settings_config_color.mustache with plugindir => plugininfo::get_base_dir(). The template uses that value to build a <script src> for the bundled jscolor picker that decorates the colour-value inputs on the admin settings page.

Identified Code
<script src="{{ plugindir }}/js/jscolor/jscolor.min.js"></script>
Suggested Fix

Build the URL from wwwroot (e.g. supply (new moodle_url('/lib/editor/tiny/plugins/fontcolor/js/jscolor/jscolor.min.js'))->out() as the template variable), or load jscolor via $PAGE->requires->js() from within output_html() instead of embedding a <script> tag in the template.

Identified Code
    public static function get_base_dir(): string {
        global $CFG;
        $dir = str_replace($CFG->dirroot, '', realpath(__DIR__));
        return substr($dir, 0, strrpos($dir, DIRECTORY_SEPARATOR));
    }
Suggested Fix

When the returned value is used to construct a browser URL, prepend $CFG->wwwroot (or return a moodle_url). A filesystem-relative fragment must not be emitted as an absolute URL without accounting for a wwwroot subdirectory.

Third-Party Libraries (1)
LibraryVersionLicenseDeclared
jscolor Color Picker
Client-side colour picker that decorates the colour-value input fields on the plugin's admin settings page.
2.5.2GNU GPL v3
Additional AI Notes

Module import style in amd/src/common.js. common.js declares only a default export, yet sibling modules (plugin.js, commands.js, configuration.js, options.js, colorswat.js) import named bindings such as {component, pluginName, forecolor, backcolor} from it. This works in the shipped build because Moodle's AMD/Babel output for a default-only module ends with return _exports.default, so RequireJS uses that object as the module value and the named property accesses resolve. It is functional but non-idiomatic — declaring explicit named exports in common.js would make the contract clear and less fragile against future build-tool changes.

Unused colorscheme.json. The bundled colorscheme.json (a 39-colour default palette) is not referenced by any PHP, JS, or template code. Per the README it exists only as copy-paste material for administrators seeding the textcolors/backgroundcolors settings. It is harmless, but could be documented in-file or removed to avoid confusion.

Broad theme SCSS modification. When usecssclassnames is enabled, save_css_classnames() writes the generated CSS into the scss setting of every installed theme, not just the active one(s). This is intentional and documented, but it is an invasive pattern: it can collide with manual SCSS edits and (combined with the missing uninstall cleanup in finding 4) spreads plugin-owned data across many core-owned config keys. Consider scoping to the site/active themes or clearly documenting the maintenance implications.

Good practices worth acknowledging. The plugin validates colour hex codes with a strict regex before storage and CSS emission, escapes colour names via format_string() and Mustache, relies on the editor_tiny\plugin base class to enforce the tiny/fontcolor:use capability, implements a correct null_provider privacy class, declares its third-party library, and ships both PHPUnit (color_test, color_list_test, plugininfo_test) and Behat coverage.

This review was generated by an AI system and may contain inaccuracies. Findings should be verified by a human reviewer before acting on them.