Skip to content

Fix stored XSS via event handlers in user profile description#974

Open
az10b wants to merge 1 commit intoLinkStackOrg:mainfrom
az10b:fix/stored-xss-description
Open

Fix stored XSS via event handlers in user profile description#974
az10b wants to merge 1 commit intoLinkStackOrg:mainfrom
az10b:fix/stored-xss-description

Conversation

@az10b
Copy link
Copy Markdown

@az10b az10b commented Apr 10, 2026

Summary

  • strip_tags() in UserController::editPage() allows <a> tags but does not strip HTML attributes, allowing JavaScript event handlers (e.g. onmouseover, onfocus, onclick) to pass through
  • The description is rendered unescaped via {!! !!} in linkinfo.blade.php, enabling stored XSS
  • Added regex filters to strip all on* event handler attributes (both quoted and unquoted values) before saving to the database

Steps to reproduce

  1. Login as any registered user
  2. POST directly to /studio/page (bypassing CKEditor) with pageDescription set to:
    <a href="https://x" onmouseover="alert(document.cookie)">Hover me</a>
  3. Visit /info/{link_id} — hovering the link triggers the JavaScript

Test plan

  • Verified the onmouseover payload is stripped after the fix
  • Verified legitimate HTML tags (<a>, <p>, <strong>, etc.) still work
  • Verified href attributes on <a> tags are preserved

strip_tags() allows <a> tags but does not remove HTML attributes,
allowing event handlers like onmouseover to pass through. The
description is rendered unescaped via {!! !!} in linkinfo.blade.php,
enabling stored XSS.

Add regex filters using word boundary matching to strip all on* event
handler attributes (both quoted and unquoted values) from the sanitized
description before saving to the database.
@az10b az10b force-pushed the fix/stored-xss-description branch from a6e636f to 9718b86 Compare April 10, 2026 03:50
@az10b
Copy link
Copy Markdown
Author

az10b commented Apr 10, 2026

Note: This PR uses regex-based stripping of on* event handler attributes as a minimal fix. For a more robust long-term solution, consider using HTMLPurifier which parses the DOM rather than relying on pattern matching:

$config = \HTMLPurifier_Config::createDefault();
$config->set('HTML.Allowed', 'a[href],p,strong,i,ul,ol,li,blockquote,h2,h3,h4');
$purifier = new \HTMLPurifier($config);
$pageDescription = $purifier->purify($request->pageDescription);

This would replace the entire strip_tags() + regex + strip_tags_except_allowed_protocols() chain and is generally considered more resilient against edge-case bypasses.

@az10b
Copy link
Copy Markdown
Author

az10b commented Apr 10, 2026

I realize this explanation might not be that clear here is the full write up of the issue. https://github.com/az10b/security-advisories/blob/main/stored_xss_linkstack.md

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