Skip to content

[bug] XSS middleware silently discards all text after an unclosed <script>/<style>/<textarea> tag (data loss) #201

Description

@leo-aa88

What happened

The XSS sanitizer (stripHTML in framework/xss.go, wired into the default
runtime middleware stack for every app) discards all text that follows an
unclosed "skip element" start tag — <script>, <style>, <textarea>,
<iframe>, <object>, <embed>, <noscript>. Because the tokenizer enters
skip mode on the start tag and never leaves it without a matching end tag,
every subsequent text token is dropped and the truncated value is what reaches
(and is persisted by) the handler.

This is distinct from the already-fixed #118 (incomplete a<b tags): here the
input contains a complete, well-formed start tag, but no closing tag, so it
matches completeHTMLTag and triggers the skip path.

Realistic trigger — a plain text/description field whose legitimate content
merely mentions one of these element names:

input:  "To embed a video use the <iframe> element with a src attribute"
stored: "To embed a video use the "
input:  "Wrap long input in a <textarea> so it scrolls"
stored: "Wrap long input in a "

A properly closed <script>...</script>foo keeps foo, so the data loss is
specific to unbalanced markup, which is easy to produce in prose, docs, code
snippets, or CMS-style content.

Steps to reproduce

1. gombit new demo   (any auth mode)
2. Add a resource with a text field, e.g. gombit make resource note --fields "body:text"
3. POST /api/v1/notes  {"body":"Use the <iframe> tag to embed"}
4. GET the row back: body is stored as "Use the " — everything after the
   unclosed skip tag was silently discarded.

Impact

Silent data loss / corruption of user-submitted content in any string field,
for all apps (the XSS middleware is on by default). The value is truncated
without any error, so neither the client nor the server logs indicate the loss.

Suggested fix

For an unclosed skip element, strip the tag itself but keep the following text
as plain text (the sanitizer already removes tags, so trailing text carries no
executable markup), rather than dropping the remainder of the string. At
minimum, treat an unterminated skip element as "strip the tag, keep the text"
instead of fail-closed silent truncation.

Reproduces on main.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggood first issueGood for newcomershelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions