Harden dashboard welcome message against XSS#106
Open
sdebacker wants to merge 1 commit into
Open
Conversation
The admin dashboard renders the remotely fetched welcome message unescaped
({!! $welcomeMessage !!}). It was only filtered with strip_tags(), which keeps
the attributes of allowed tags, so a compromised or man-in-the-middled upstream
could smuggle in event handlers (onclick, onerror, …) or javascript: URIs and
execute script in the admin context.
Parse the fetched HTML and strip every attribute, restoring only a safe href
(http/https/mailto, relative or anchor) on links.
https://claude.ai/code/session_01EUUnydptJJw8Az5AAVQ1r3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Second-pass security fix following the search SQL-injection fix (#104).
The admin dashboard renders the welcome message unescaped:
The message is fetched from a remote URL (
WELCOME_MESSAGE_URL) and was only filtered withstrip_tags($body, '<a>...').strip_tags()restricts tags but keeps their attributes, so the allowed tags could still carry:<a onmouseover=…>,<p onclick=…><a href="javascript:…">If the configured upstream is compromised — or served over plain
httpand man-in-the-middled — this becomes stored/reflected XSS executing in the admin panel (full CMS takeover via an authenticated admin's session). The existing SSRF guard (isAllowedUrl) does not address the content of the response.Fix
sanitizeHtml()now:strip_tags(unchanged allowlist).DOMDocumentand removes every attribute from every element.hrefon<a>(http/https/mailto, or relative/anchor) —javascript:/data:/etc. are dropped.DOM-based rebuilding (rather than regex) avoids attribute-parsing bypasses. Legitimate formatting and safe links are preserved; UTF-8 content is handled correctly.
Verification
Sanitizer logic exercised against representative inputs:
<a href="https://x.com" onclick="alert(1)">link</a><a href="https://x.com">link</a><a href="javascript:alert(1)">x</a><a>x</a><a href="JaVaScRiPt:alert(1)">x</a><a>x</a><a href="mailto:a@b.com">mail</a><a href="mailto:a@b.com">mail</a><p onmouseover="alert(1)">hover</p><p>hover</p>Café résumé <em>ok</em>Café résumé <em>ok</em>php -lpasses (PHP 8.4). The package has novendor/in this environment (the suite runs against a host app), so the full test suite wasn't run here.https://claude.ai/code/session_01EUUnydptJJw8Az5AAVQ1r3
Generated by Claude Code