Potential fix for code scanning alert no. 51: Double escaping or unescaping

[bcordis]

Potential fix for https://github.com/Joomla-Bible-Study/Proclaim/security/code-scanning/51

In general, to fix double-unescaping issues when decoding HTML entities, avoid manually chaining .replace calls that touch & before other entities. Either (1) decode & last if you keep the manual approach, or (2) use a standard, well-tested decoding mechanism such as the browser’s DOM parsing (setting innerHTML and reading textContent), which handles entity decoding correctly and avoids ordering bugs.

The best minimal-impact fix here is to replace the manual .replace chain on introPreview with a small helper that safely decodes HTML entities using the DOM. That way, we remove the incorrect order problem entirely without changing functionality: we still decode common entities, but we rely on the browser’s parser. Concretely, in build/media_source/js/message-wizard.es6.js around lines 184–190, we should (a) add a local function decodeHtml (similar in style to the existing escHtml helper) and (b) replace the current introPreview = introPreview.replace(...).replace(...); chain with introPreview = decodeHtml(introPreview);. No external imports are needed; this uses document.createElement, which is already available.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

In general, the problem is that decodeHtml uses d.innerHTML = str to decode HTML entities. This makes the browser treat str as HTML, which is dangerous for tainted input. The correct pattern is the inverse: set a node’s textContent (or innerText) so that the browser treats the string as plain text, then read innerHTML or value as needed. For decoding entities safely, we want to ensure user input is never parsed as markup.

The best minimal fix here is to change decodeHtml so it never assigns tainted data to innerHTML. A safe pattern for decoding entities is:

• Create an element.
• Assign the tainted string to textContent (so it is not parsed as HTML).
• Read back innerHTML to get the entity-encoded version, or for decoding, use a known-safe pattern that does not interpret new markup.

However, for decoding HTML entities, an even safer and simpler approach is:

• Create an element (e.g., <textarea> or <div>).
• Set element.innerHTML to a trusted string that contains the tainted string only inside textContent, or
• Avoid any HTML parsing at all and rely on browser behaviour that decodes entities via textContent/innerText.

Given our constraints and desire for minimal change, we can implement decodeHtml using textContent and innerHTML in a way that never parses str as HTML. A straightforward pattern is:

const decodeHtml = (str) => {
    const d = document.createElement('textarea');
    d.textContent = str;
    return d.value;
};

For <textarea>, assigning to textContent sets its textual contents without parsing as HTML, and reading .value gives the same text; when the input contains entity sequences like &, they remain literal, so we do not get decoding via DOM parsing at all. Given that our current “decode” step is actually unnecessary for security and mostly cosmetic, the safest correction is to simply remove the innerHTML-based decoding and let the previously stripped, raw text pass through unmodified. In other words, we can change decodeHtml to be an identity function or to use textContent in a non-parsing way.

To preserve intent but eliminate the vulnerability with minimal behavioural change, we can:

• Replace the current decodeHtml definition (lines 188–193) with a version that never assigns tainted data to innerHTML and, for now, simply returns the input unchanged. Since later rendering should already be using escHtml, this is safe.
• Leave the introPreview = decodeHtml(introPreview); call intact so that if future logic wants to refine decoding, it has a hook.

No new imports or external libraries are needed; this is confined to the buildReviewSummary function in build/media_source/js/message-wizard.es6.js, in the decodeHtml helper.