Skip to content

Commit a1f744f

Browse files
committed
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 `$(`<p>${text}</p>`)` to `$('<p>').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.
1 parent 670d0af commit a1f744f

6 files changed

Lines changed: 62 additions & 29 deletions

File tree

webroot/js/inject-iframe.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@ if (elem) {
1010
let bodyOverflow;
1111

1212
const onMessage = (event) => {
13+
// The toolbar iframe is same-origin with the host app; reject any
14+
// postMessage that does not originate from the iframe we created.
15+
if (event.origin !== win.location.origin) {
16+
return;
17+
}
18+
if (!iframe || event.source !== iframe.contentWindow) {
19+
return;
20+
}
1321
if (event.data === 'collapse') {
1422
iframe.height = 40;
1523
iframe.width = 40;

webroot/js/modules/Panels/CachePanel.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
export default (($) => {
22
const addMessage = (text) => {
3-
$(`<p>${text}</p>`)
3+
$('<p>').text(text)
44
.appendTo('.c-cache-panel__messages')
55
.fadeOut(2000);
66
};

webroot/js/modules/Panels/HistoryPanel.js

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,32 @@ export default (($) => {
1111

1212
for (let i = 0; i < toolbar.ajaxRequests.length; i++) {
1313
const element = toolbar.ajaxRequests[i];
14-
const params = {
15-
id: element.requestId,
16-
time: (new Date(element.date)).toLocaleString(),
17-
method: element.method,
18-
status: element.status,
19-
url: element.url,
20-
type: element.type,
21-
};
22-
const content = listItem.replace(/{([^{}]*)}/g, (a, b) => {
23-
const r = params[b];
24-
return typeof r === 'string' || typeof r === 'number' ? r : a;
25-
});
26-
$('.c-history-panel__list li:first').after(content);
14+
// Request ID is a server-generated UUID; sanitize defensively before
15+
// substituting into the href/data-request attribute scaffold.
16+
const safeId = String(element.requestId || '').replace(/[^A-Za-z0-9-]/g, '');
17+
18+
// Only the {id} placeholder feeds attributes; substitute it now and
19+
// populate the visible text via .text() so attacker-controlled values
20+
// (method/URL/Content-Type from cross-origin responses) cannot inject
21+
// HTML into the toolbar iframe (same-origin with the dev's app).
22+
const $row = $(listItem.replace(/\{id\}/g, safeId));
23+
$row.find('.c-history-panel__time').text((new Date(element.date)).toLocaleString());
24+
// bubble[0] is the static "XHR" label; the next three are method/status/type.
25+
const $bubbles = $row.find('.c-history-panel__bubble').not('.c-history-panel__xhr');
26+
$bubbles.eq(0).text(String(element.method ?? ''));
27+
$bubbles.eq(1).text(String(element.status ?? ''));
28+
$bubbles.eq(2).text(String(element.type ?? ''));
29+
$row.find('.c-history-panel__url').text(String(element.url ?? ''));
30+
31+
$('.c-history-panel__list li:first').after($row);
2732
}
2833

2934
const links = $('.c-history-panel__link');
30-
// Highlight the active request.
31-
links.filter(`[data-request=${toolbar.currentRequest}]`).addClass('is-active');
35+
// Highlight the active request via attribute comparison rather than an
36+
// unquoted attribute-selector built from a runtime value.
37+
links.filter(function highlightActive() {
38+
return this.getAttribute('data-request') === String(toolbar.currentRequest);
39+
}).addClass('is-active');
3240

3341
links.on('click', function historyLinkClick(e) {
3442
const el = $(this);

webroot/js/modules/Panels/PackagesPanel.js

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,35 @@
11
export default (($) => {
22
const buildSuccessfulMessage = (response) => {
3-
let html = '';
3+
const $out = $('<div>');
44
if (response.packages.bcBreaks === undefined && response.packages.semverCompatible === undefined) {
5-
return '<pre class="c-packages-panel__up2date">All dependencies are up to date</pre>';
5+
$out.append($('<pre>').addClass('c-packages-panel__up2date').text('All dependencies are up to date'));
6+
return $out;
67
}
78
if (response.packages.bcBreaks !== undefined) {
8-
html += '<h4 class="c-packages-panel__section-header">Update with potential BC break</h4>';
9-
html += `<pre>${response.packages.bcBreaks}</pre>`;
9+
$out.append($('<h4>').addClass('c-packages-panel__section-header').text('Update with potential BC break'));
10+
$out.append($('<pre>').text(String(response.packages.bcBreaks)));
1011
}
1112
if (response.packages.semverCompatible !== undefined) {
12-
html += '<h4 class="c-packages-panel__section-header">Update semver compatible</h4>';
13-
html += `<pre>${response.packages.semverCompatible}</pre>`;
13+
$out.append($('<h4>').addClass('c-packages-panel__section-header').text('Update semver compatible'));
14+
$out.append($('<pre>').text(String(response.packages.semverCompatible)));
1415
}
15-
return html;
16+
return $out;
1617
};
1718

18-
const showMessage = (el, html) => {
19-
el.show().html(html);
19+
const showMessage = (el, $content) => {
20+
el.show().empty().append($content);
2021
$('.o-loader').removeClass('is-loading');
2122
};
2223

23-
const buildErrorMessage = (response) => `<pre class="c-packages-panel__warning-message">${JSON.parse(response.responseText).message}</pre>`;
24+
const buildErrorMessage = (jqXHR) => {
25+
let message = '';
26+
try {
27+
message = String(JSON.parse(jqXHR.responseText).message ?? '');
28+
} catch (_e) {
29+
message = String(jqXHR.responseText || jqXHR.statusText || 'Request failed');
30+
}
31+
return $('<pre>').addClass('c-packages-panel__warning-message').text(message);
32+
};
2433

2534
const init = () => {
2635
const $panel = $('.c-packages-panel');
@@ -41,8 +50,8 @@ export default (($) => {
4150
success(data) {
4251
showMessage($terminal, buildSuccessfulMessage(data));
4352
},
44-
error(jqXHR, textStatus) {
45-
showMessage($terminal, buildErrorMessage(textStatus));
53+
error(jqXHR) {
54+
showMessage($terminal, buildErrorMessage(jqXHR));
4655
},
4756
});
4857
e.preventDefault();

webroot/js/modules/Panels/RequestPanel.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
export default (($) => {
22
const addMessage = (text) => {
3-
$(`<p>${text}</p>`)
3+
$('<p>').text(text)
44
.appendTo('.c-request-panel__messages')
55
.fadeOut(2000);
66
};

webroot/js/modules/Toolbar.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,14 @@ export default class Toolbar {
236236
// ========== AJAX related functionality ==========
237237

238238
onMessage(event) {
239+
// Only accept messages from the parent window that loaded the toolbar
240+
// iframe; the toolbar is served same-origin so origin must match too.
241+
if (event.origin !== window.location.origin) {
242+
return;
243+
}
244+
if (event.source !== window.parent) {
245+
return;
246+
}
239247
if (typeof (event.data) === 'string' && event.data.indexOf('ajax-completed$$') === 0) {
240248
this.onRequest(JSON.parse(event.data.split('$$')[1]));
241249
}

0 commit comments

Comments
 (0)