Escaping is not enough: the second decode inside an onclick

Our bug bounty platform has a back office. It displays reports, researcher accounts and submissions as server-rendered tables. Every one of those rows carries values that came from a public form: a name, an email address, an organisation, a profile link.

This is the textbook injection case: untrusted data rendered into a privileged interface. The correct instinct is to escape. We were escaping. It was not enough.

Finding 1: escaping survives the HTML, not the second decode

A table row carries action buttons, and each button needs to know which entry it acts on. The natural construction interpolates the value into the handler:

<button onclick="viewReport('&#39;user&#39;')">View</button>

The value is properly escaped in the HTML sense. The document is valid. And the construction is exploitable anyway.

The reason is that an event-handler attribute is not text, it is code. The browser proceeds in two steps: it first decodes the HTML entities in the attribute value, then hands the result to the JavaScript parser. HTML escaping is therefore undone before the JavaScript is read. An apostrophe escaped as ' becomes an apostrophe again, closes the string literal, and what follows is interpreted as code.

One value is enough, and it is nothing exotic: an organisation name containing an apostrophe. In our case the expected names look like “Amadou Daouda M’Bodj”. The apostrophe is legitimate, common, and it was the payload.

The rule that follows: escaping is relative to the destination context, not to the type of the data. Escaping for HTML and then dropping the result into a JavaScript context is escaping for the wrong target. “We escape everything” is not an answer until “escaped into what?” has been asked.

The fix: never interpolate into a handler again

The correction is not to escape better, it is to stop putting data into an executable context at all. Values go into data-* attributes, which are text and stay text, and a single delegated listener on the container reads them at click time:

<button data-act="status" data-ts="...">View</button>

document.getElementById('pb').addEventListener('click', function (ev) {
  var b = ev.target.closest('button[data-act]');
  if (!b) return;
  var act = b.dataset.act, ts = b.dataset.ts || '';
  if (act === 'status') pickStatus(ts, b.dataset.s);
});

The data never crosses the JavaScript parser. It is read out of the DOM as a string, at the point of use. There is no longer any context in which an apostrophe can mean anything other than an apostrophe.

Finding 2: the CSP was protecting us and silently breaking the UI

The second finding is the most instructive, and we were not looking for it.

The admin serves a strict content security policy, with a per-request nonce and no 'unsafe-inline':

script-src 'nonce-<random-value>';

A nonce authorises <script> elements carrying the matching attribute. It does not authorise event-handler attributes. An onclick is still inline script, it carries no nonce and cannot carry one, so it is blocked outright.

The consequence: every top-level control in the back office still wired with onclick was dead. Not degraded, not slow: inert. The language toggle, the reload, the CSV export, the panel close.

Nobody had reported it, and that is the part worth keeping. A control that does nothing at all produces no visible error, no alert, and no server-side log entry. The browser console shows the CSP refusal, but nobody keeps the console open in an internal tool they use every day.

Two lessons sit on top of each other here. First, a strict CSP is also a detector: it turns a bad practice into an outage, and an outage is preferable to a silent vulnerability. Second, silence is not evidence that something works. The absence of user complaints measures only how often a thing is used.

Finding 3: javascript: was happy to be stored

A third defect, independent of the first two. Researchers can supply a profile link, rendered as an anchor in the account pane. That field had no scheme check at all. A value such as javascript:alert(1) stored cleanly, and waited for an administrator to click it.

The control is a scheme allowlist, not a filter for dangerous patterns:

_PROFILE_URL_RE = re.compile(r'^https?://[^\s<>"\']+$', re.IGNORECASE)

A field destined to become a link has no business accepting anything other than http or https. Trying to forbid what is dangerous is a race you lose; enumerating what is acceptable terminates.

A second line, in storage

We added a check at the storage layer that rejects angle brackets and control characters in display names:

_NAME_FORBIDDEN_RE = re.compile(r'[<>\x00-\x1f\x7f]')

This is not the primary defence, and it is worth being clear about that: the display templates escape, and must keep doing so. It is a second line with a precise purpose. Display names currently reach three surfaces: the back office, the hall of fame and the public profile. There will be more. Keeping markup out of the database means a renderer written six months from now cannot be handed a payload that is already stored and ready to use.

Accents, apostrophes and hyphens obviously stay legal. A security control that rejects “M’Bodj” is not a security control, it is a defect.

What we take from it

1. Escaping is relative to a destination. HTML, attribute, URL, JavaScript, CSS: five contexts, five rules. Data crossing two contexts is decoded twice.
2. Do not put data into an executable context. data-* plus event delegation removes the entire class of problem rather than fixing one instance of it.
3. A strict CSP reveals as much as it protects. If it breaks something, that something probably needed fixing.
4. Allow explicitly rather than forbid. A list of acceptable schemes terminates; a list of dangerous payloads does not.

All three defects were fixed before the programme opened to the public. If you find something else on our surfaces, the channel exists for exactly that: [email protected].