Fixed in 3.14.0 · Found while building the process name preview for #202 · Present in every release from 1.0.0 to 3.13.0 · Modules: all

FreeITSM's pages build much of their HTML in the browser, and put text people have typed - a ticket subject, a process label, a person's name - through a small "escape" helper first, so it shows as text instead of being read as markup. Those helpers made text safe between tags but not inside an attribute: they left the double quote alone. Text containing a quote could end the attribute it was placed in and add one of its own - an event handler that runs code in the browser of whoever is looking, with their access.

Who could do it: the clearest case needs no account at all. A ticket raised by email keeps the sender's subject, and a ticket that is linked to another (parent, child, duplicate, related) shows that subject as the tooltip of its badge on the other ticket. A subject such as Printer down" onmouseover="… ran its handler when an analyst moved the pointer over the badge. Other cases needed someone who can already write the text - a connector label in Process Mapper, which is where it was found.

The usual way to escape text in the browser is three lines:

function escapeHtml(text) {
    const div = document.createElement('div');
    div.textContent = text;
    return div.innerHTML;
}

Reading innerHTML back escapes <, > and & - enough between tags. It does not escape " or ', because inside an element's text they mean nothing. So this is safe:

`<span>${escapeHtml(subject)}</span>`

and this is not:

`<a title="${escapeHtml(subject)}">`

The helper looks finished, it works in every test that puts text between tags, and nothing on screen changes until somebody types a quote. It had been copied into the codebase 107 times - escapeHtml, esc, escHtml and a dozen local variants, in 100 files - and many of their callers put the result inside attributes: tooltips, value="", alt="", data- attributes.

The Process Mapper case (a connector label inside value="" in the step's detail panel) was fixed first, on its own. Looking for others turned up the same three lines everywhere, and a sweep of the attributes they fed found the linked-ticket badge above, where the text comes from outside the organisation. Rather than decide 107 times which callers were exposed, every helper now escapes quotes - which is what an escape helper should do anyway.

Filenames of email attachments were already safe: mail import removes " from them before they are stored.

Every helper now ends:

return div.innerHTML.replace(/"/g, '"').replace(/'/g, ''');

The output looks exactly the same on screen - " and ' display as " and ' - and is now safe in an attribute (either quote style) as well as between tags. Every script that changed has its cache-buster bumped, so browsers fetch the new copy.

In headless Chrome, a linked-ticket badge built with a subject of Printer down" onmouseover="..." x=": with the old helper the badge gained a working onmouseover attribute; with the new one it did not, and the tooltip showed the whole subject as typed. The browser-driven task tests, the case report test and the security findings suite pass on the new code (the one security finding that fails, a fail-open guard in project reports, fails identically before the change).

tests/escape-helpers-quotes.php scans every page and script for a helper that sets textContent and returns innerHTML without escaping quotes. It fails on the old code (naming each file) and passes on the new; its control proves it still recognises the old three-line helper, so a new page that copies it cannot slip through.

No - upgrading is the fix, and a hard refresh (Ctrl-F5) makes sure your browser is using the new scripts. If you want to look for anything planted, search for ticket subjects containing onmouse, onfocus, onerror or onload; ordinary subjects never contain them.

Do not write the three-line helper. If a page needs one, copy the fixed version - or better, build the element with the DOM (el.title = text, el.textContent = text), which needs no escaping at all. The guard test will catch the old pattern.

Related: Process Mapper · Bugs resolved