Skip to content

fix: a dropped file's name reaches a toast as text - #939

Merged
blaipr merged 1 commit into
mainfrom
fix/a-dropped-file-name-reaches-a-toast-as-text
Sep 24, 2026
Merged

blaipr merged 1 commit into
mainfrom
fix/a-dropped-file-name-reaches-a-toast-as-text

Conversation

@blaipr

@blaipr blaipr commented Sep 24, 2026

Copy link
Copy Markdown
Member

toasts.min.js renders every message with innerHTML — deliberately, since messages carry <br>
separators. So anything concatenated into a toast has to be escaped first, and the file upload's two
rejection messages concatenated the dropped File's name and type raw:

sysPassApp.msg.error(LANG[18] + "<br>" + c.name + " (Max: …)")   // too large
sysPassApp.msg.error(LANG[19] + "<br>" + c.type)                  // type not allowed

A file named <img src=x onerror=…> that trips either check ran its handler in the page.

The grade, stated plainly

It is the user's own local file, so on its own this is self-XSS — reachable only by persuading
somebody to upload a crafted name. It is fixed anyway because the sink is real, the fix is one
expression, and the next message to put a value into a toast would inherit the same mistake.

The change

$("<div/>").text(value).html() — set the value as text, read it back escaped — at both call sites.
jQuery is already on every page. The toasts keep rendering their <br>s; only the interpolated
values change.

Test

FileNamesReachToastsAsTextTest scans app-util.min.js for every toast message that interpolates
the chosen file's name or type, and asserts each is wrapped in the escape. It checks the text
immediately around each occurrence rather than matching the whole call, because the escape itself
contains parentheses: the first version stopped at the ) inside .text(c.name) and failed with
the fix in place — worth recording, since a guard that fails both ways proves nothing.

Mutation-verified: reverting the JS fails it, naming the byte offset.

Deliberately not in this change

The toast sink is the wider issue. app-main.min.js's response handler routes a server's
description and messages into the same innerHTML, and app-requests.min.js puts a failed
request's entire responseText there. Those are strings from ActionResponse, which the PHP
side does not pass through $_e() the way it does templates — so they are safe only while no
server message ever carries user text. Switching toasts to text would break every <br>; making
the server's messages safe by construction is a separate, larger decision.

toasts.min.js renders every message with innerHTML, deliberately, since messages carry <br>
separators — so whatever is concatenated into one has to be escaped first. The file upload's
two rejection messages concatenated the dropped File's name and type raw, and a file named
<img src=x onerror=...> that tripped either check ran its handler.

It is the user's own local file, so on its own this is self-XSS, reachable only by
persuading somebody to upload a crafted name. It is fixed because the sink is real and the
fix is one expression: $("<div/>").text(value).html() at both call sites.

The wider issue is left for a separate decision: app-main.min.js routes a server's
description and messages into the same innerHTML, and app-requests.min.js puts a failed
request's entire responseText there, while ActionResponse strings are not escaped the way
templates are.
@blaipr
blaipr merged commit 9d5a5b8 into main Sep 24, 2026
8 checks passed
@blaipr
blaipr deleted the fix/a-dropped-file-name-reaches-a-toast-as-text branch September 24, 2026 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant