From a1f744f920ae77d150beee179dfb3503362be434 Mon Sep 17 00:00:00 2001 From: mscherer Date: Wed, 13 May 2026 23:53:44 +0200 Subject: [PATCH] JS DOM-safe rendering: stop string-concat HTML into the toolbar iframe The toolbar iframe is loaded same-origin with the host app, so any HTML injection inside it is effectively XSS against the app being developed. Several panels were composing HTML by string concatenation from data that crosses the toolbar's trust boundary (cross-origin response headers, attacker-influenced URLs, raw composer error bodies). Replace each sink with DOM construction via `document.createElement` / jQuery `.text()` / `.append()`. * HistoryPanel: build each row by parsing the template once, substituting only the (sanitized) UUID `{id}` into the href/data-request attributes, and populating method/status/type/url/time via `.text()`. Replaces the prior pattern that interpolated all six placeholders raw into HTML and parsed the result with `.after(string)`. Also rewrites the `[data-request=...]` selector as a `.filter()` predicate so a runtime request id can't slip into selector syntax. * CachePanel / RequestPanel `addMessage`: switch `$(`

${text}

`)` to `$('

').text(text)`. * PackagesPanel: assemble the success / error output as jQuery-built elements with `.text()`, and `showMessage()` clears the terminal with `.empty().append(...)` rather than `.html(string)`. Fixes a pre-existing bug along the way: the error callback was passing the jQuery `textStatus` argument to `buildErrorMessage` (which expected an XHR object), so every non-2xx response was previously a silent JSON.parse throw. The error builder now takes the jqXHR object directly and falls back to `responseText` / `statusText` / a literal "Request failed". * postMessage handlers in `inject-iframe.js` and `Toolbar.onMessage` now reject any message whose origin is not same-origin and whose source is not the expected counterpart window (the iframe contentWindow on the parent side, `window.parent` on the iframe side). Today only resize and XHR-counter messages flow, so the practical impact of the missing check is low, but the one-line hardening keeps the protocol safe as it grows. ESLint output for the touched files is clean; the remaining lint warnings/errors (Turbo undefined, unnamed function expressions in Toolbar and inject-iframe, Keyboard.js return) are pre-existing on 5.x. --- webroot/js/inject-iframe.js | 8 +++++ webroot/js/modules/Panels/CachePanel.js | 2 +- webroot/js/modules/Panels/HistoryPanel.js | 38 +++++++++++++--------- webroot/js/modules/Panels/PackagesPanel.js | 33 ++++++++++++------- webroot/js/modules/Panels/RequestPanel.js | 2 +- webroot/js/modules/Toolbar.js | 8 +++++ 6 files changed, 62 insertions(+), 29 deletions(-) diff --git a/webroot/js/inject-iframe.js b/webroot/js/inject-iframe.js index f24ca6a12..357fe0d69 100644 --- a/webroot/js/inject-iframe.js +++ b/webroot/js/inject-iframe.js @@ -10,6 +10,14 @@ if (elem) { let bodyOverflow; const onMessage = (event) => { + // The toolbar iframe is same-origin with the host app; reject any + // postMessage that does not originate from the iframe we created. + if (event.origin !== win.location.origin) { + return; + } + if (!iframe || event.source !== iframe.contentWindow) { + return; + } if (event.data === 'collapse') { iframe.height = 40; iframe.width = 40; diff --git a/webroot/js/modules/Panels/CachePanel.js b/webroot/js/modules/Panels/CachePanel.js index 24050762a..bf3fc205e 100644 --- a/webroot/js/modules/Panels/CachePanel.js +++ b/webroot/js/modules/Panels/CachePanel.js @@ -1,6 +1,6 @@ export default (($) => { const addMessage = (text) => { - $(`

${text}

`) + $('

').text(text) .appendTo('.c-cache-panel__messages') .fadeOut(2000); }; diff --git a/webroot/js/modules/Panels/HistoryPanel.js b/webroot/js/modules/Panels/HistoryPanel.js index 856ffd75b..b5969b1bc 100644 --- a/webroot/js/modules/Panels/HistoryPanel.js +++ b/webroot/js/modules/Panels/HistoryPanel.js @@ -11,24 +11,32 @@ export default (($) => { for (let i = 0; i < toolbar.ajaxRequests.length; i++) { const element = toolbar.ajaxRequests[i]; - const params = { - id: element.requestId, - time: (new Date(element.date)).toLocaleString(), - method: element.method, - status: element.status, - url: element.url, - type: element.type, - }; - const content = listItem.replace(/{([^{}]*)}/g, (a, b) => { - const r = params[b]; - return typeof r === 'string' || typeof r === 'number' ? r : a; - }); - $('.c-history-panel__list li:first').after(content); + // Request ID is a server-generated UUID; sanitize defensively before + // substituting into the href/data-request attribute scaffold. + const safeId = String(element.requestId || '').replace(/[^A-Za-z0-9-]/g, ''); + + // Only the {id} placeholder feeds attributes; substitute it now and + // populate the visible text via .text() so attacker-controlled values + // (method/URL/Content-Type from cross-origin responses) cannot inject + // HTML into the toolbar iframe (same-origin with the dev's app). + const $row = $(listItem.replace(/\{id\}/g, safeId)); + $row.find('.c-history-panel__time').text((new Date(element.date)).toLocaleString()); + // bubble[0] is the static "XHR" label; the next three are method/status/type. + const $bubbles = $row.find('.c-history-panel__bubble').not('.c-history-panel__xhr'); + $bubbles.eq(0).text(String(element.method ?? '')); + $bubbles.eq(1).text(String(element.status ?? '')); + $bubbles.eq(2).text(String(element.type ?? '')); + $row.find('.c-history-panel__url').text(String(element.url ?? '')); + + $('.c-history-panel__list li:first').after($row); } const links = $('.c-history-panel__link'); - // Highlight the active request. - links.filter(`[data-request=${toolbar.currentRequest}]`).addClass('is-active'); + // Highlight the active request via attribute comparison rather than an + // unquoted attribute-selector built from a runtime value. + links.filter(function highlightActive() { + return this.getAttribute('data-request') === String(toolbar.currentRequest); + }).addClass('is-active'); links.on('click', function historyLinkClick(e) { const el = $(this); diff --git a/webroot/js/modules/Panels/PackagesPanel.js b/webroot/js/modules/Panels/PackagesPanel.js index 436f4a734..fcd4c5579 100644 --- a/webroot/js/modules/Panels/PackagesPanel.js +++ b/webroot/js/modules/Panels/PackagesPanel.js @@ -1,26 +1,35 @@ export default (($) => { const buildSuccessfulMessage = (response) => { - let html = ''; + const $out = $('

'); if (response.packages.bcBreaks === undefined && response.packages.semverCompatible === undefined) { - return '
All dependencies are up to date
'; + $out.append($('
').addClass('c-packages-panel__up2date').text('All dependencies are up to date'));
+      return $out;
     }
     if (response.packages.bcBreaks !== undefined) {
-      html += '
Update with potential BC break
';
-      html += `
${response.packages.bcBreaks}
`;
+      $out.append($('
').addClass('c-packages-panel__section-header').text('Update with potential BC break'));
+      $out.append($('
').text(String(response.packages.bcBreaks)));
     }
     if (response.packages.semverCompatible !== undefined) {
-      html += '
Update semver compatible
';
-      html += `
${response.packages.semverCompatible}
`;
+      $out.append($('
').addClass('c-packages-panel__section-header').text('Update semver compatible'));
+      $out.append($('
').text(String(response.packages.semverCompatible)));
     }
-    return html;
+    return $out;
   };
 
-  const showMessage = (el, html) => {
-    el.show().html(html);
+  const showMessage = (el, $content) => {
+    el.show().empty().append($content);
     $('.o-loader').removeClass('is-loading');
   };
 
-  const buildErrorMessage = (response) => `
${JSON.parse(response.responseText).message}
`;
+  const buildErrorMessage = (jqXHR) => {
+    let message = '';
+    try {
+      message = String(JSON.parse(jqXHR.responseText).message ?? '');
+    } catch (_e) {
+      message = String(jqXHR.responseText || jqXHR.statusText || 'Request failed');
+    }
+    return $('
').addClass('c-packages-panel__warning-message').text(message);
+  };
 
   const init = () => {
     const $panel = $('.c-packages-panel');
@@ -41,8 +50,8 @@ export default (($) => {
         success(data) {
           showMessage($terminal, buildSuccessfulMessage(data));
         },
-        error(jqXHR, textStatus) {
-          showMessage($terminal, buildErrorMessage(textStatus));
+        error(jqXHR) {
+          showMessage($terminal, buildErrorMessage(jqXHR));
         },
       });
       e.preventDefault();
diff --git a/webroot/js/modules/Panels/RequestPanel.js b/webroot/js/modules/Panels/RequestPanel.js
index 7e5da6145..4802b5281 100644
--- a/webroot/js/modules/Panels/RequestPanel.js
+++ b/webroot/js/modules/Panels/RequestPanel.js
@@ -1,6 +1,6 @@
 export default (($) => {
   const addMessage = (text) => {
-    $(`
${text}
`)
+    $('
').text(text)
       .appendTo('.c-request-panel__messages')
       .fadeOut(2000);
   };
diff --git a/webroot/js/modules/Toolbar.js b/webroot/js/modules/Toolbar.js
index c3d9d9c60..e87d31acd 100644
--- a/webroot/js/modules/Toolbar.js
+++ b/webroot/js/modules/Toolbar.js
@@ -236,6 +236,14 @@ export default class Toolbar {
   // ========== AJAX related functionality ==========
 
   onMessage(event) {
+    // Only accept messages from the parent window that loaded the toolbar
+    // iframe; the toolbar is served same-origin so origin must match too.
+    if (event.origin !== window.location.origin) {
+      return;
+    }
+    if (event.source !== window.parent) {
+      return;
+    }
     if (typeof (event.data) === 'string' && event.data.indexOf('ajax-completed$$') === 0) {
       this.onRequest(JSON.parse(event.data.split('$$')[1]));
     }