Skip to content

Commit fa6fb2b

Browse files
teancomclaude
andauthored
Fix XSS via mDNS server name injection in launcher (#10)
Replace inline onclick handlers with addEventListener to prevent JavaScript injection through crafted mDNS service names. The escapeHtml() function only escapes HTML element content characters (<, >, &) but not single quotes, which allowed breaking out of the JS string literal in the onclick attribute context. Adds regression test that verifies user-controlled values are never interpolated into HTML attributes and that click handlers use addEventListener instead of inline onclick. --------- Co-authored-by: David Bishop <git@gnuconsulting.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 912133f commit fa6fb2b

2 files changed

Lines changed: 87 additions & 2 deletions

File tree

src-tauri/resources/index.html

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,8 +432,8 @@ <h1>Music Assistant</h1>
432432
} else {
433433
serverList.innerHTML = servers
434434
.map(
435-
(server) => `
436-
<div class="server-item" onclick="connectToServer('${escapeHtml(server.url)}', '${escapeHtml(server.name)}')">
435+
(server, index) => `
436+
<div class="server-item" data-server-index="${index}">
437437
<img src="logo.png" alt="" class="server-icon">
438438
<div class="server-info">
439439
<div class="server-name">${escapeHtml(server.name)}</div>
@@ -443,6 +443,11 @@ <h1>Music Assistant</h1>
443443
`
444444
)
445445
.join("");
446+
// Bind click handlers via JS to avoid inline handler injection
447+
serverList.querySelectorAll("[data-server-index]").forEach((el) => {
448+
const server = servers[parseInt(el.dataset.serverIndex)];
449+
el.addEventListener("click", () => connectToServer(server.url, server.name));
450+
});
446451
}
447452
} catch (e) {
448453
serverList.innerHTML = `

src-tauri/resources/index.test.mjs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/**
2+
* Regression test for XSS via mDNS server name injection.
3+
*
4+
* Verifies that the launcher's server list rendering does not
5+
* interpolate user-controlled strings into inline event handlers.
6+
*
7+
* Run: node src-tauri/resources/index.test.mjs
8+
*/
9+
10+
import { readFileSync } from "fs";
11+
import { strict as assert } from "assert";
12+
13+
const html = readFileSync(new URL("./index.html", import.meta.url), "utf8");
14+
15+
// ---- Test 1: No inline onclick handlers with interpolated values ----
16+
// The server list template should NOT contain onclick with escapeHtml
17+
// interpolation, which is vulnerable to quote-breakout injection.
18+
const onclickPattern = /onclick="connectToServer\('\$\{escapeHtml/;
19+
assert.ok(
20+
!onclickPattern.test(html),
21+
"FAIL: Found inline onclick with escapeHtml interpolation — vulnerable to XSS via quote breakout"
22+
);
23+
24+
// ---- Test 2: Server items use data attributes + addEventListener ----
25+
assert.ok(
26+
html.includes("data-server-index"),
27+
"FAIL: Server items should use data-server-index attributes"
28+
);
29+
assert.ok(
30+
html.includes("addEventListener"),
31+
"FAIL: Click handlers should be bound via addEventListener, not inline onclick"
32+
);
33+
34+
// ---- Test 3: Simulate the escapeHtml function and verify XSS payloads are inert ----
35+
// Replicate the browser's textContent/innerHTML escaping behavior
36+
function escapeHtml(text) {
37+
return text.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;");
38+
// Note: textContent/innerHTML does NOT escape ' " or `
39+
}
40+
41+
const xssPayloads = [
42+
"z');alert(1);('",
43+
"z',alert(1),'",
44+
'z");alert(1);("',
45+
"z`);alert(1);(`",
46+
"z');new Image().src=`http://evil.com`;('",
47+
"<script>alert(1)</script>",
48+
"test' onclick='alert(1)",
49+
];
50+
51+
for (const payload of xssPayloads) {
52+
const escaped = escapeHtml(payload);
53+
54+
// escapeHtml must neutralize HTML tag injection in element content
55+
assert.ok(
56+
!escaped.includes("<script>"),
57+
`FAIL: escapeHtml did not neutralize script tag in: ${payload}`
58+
);
59+
}
60+
61+
// ---- Test 4: The template in index.html uses safe integer index, not user content ----
62+
// Extract the server-item template from the source and verify it only
63+
// interpolates a numeric index into attributes, never escapeHtml(server.name/url).
64+
const templateSection = html.slice(html.indexOf(".map("), html.indexOf(".join("));
65+
66+
// The data attribute must use a safe integer index
67+
assert.ok(
68+
templateSection.includes("data-server-index"),
69+
"FAIL: Template should use data-server-index for click binding"
70+
);
71+
72+
// User-controlled values must NOT appear in any HTML attribute context
73+
// (they should only appear inside element content via escapeHtml)
74+
const attrPattern = /=["'][^"']*\$\{escapeHtml\(server\.(name|url)\)/;
75+
assert.ok(
76+
!attrPattern.test(templateSection),
77+
"FAIL: User-controlled escapeHtml values must not appear in HTML attributes"
78+
);
79+
80+
console.log("All tests passed.");

0 commit comments

Comments
 (0)