fix: escape URL in shariff widget to prevent reflected XSS#53
Open
GustavoCasiano wants to merge 1 commit into
Open
fix: escape URL in shariff widget to prevent reflected XSS#53GustavoCasiano wants to merge 1 commit into
GustavoCasiano wants to merge 1 commit into
Conversation
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
Fixes a reflected XSS vulnerability in the Shariff plugin where the current page URL was assigned to
data-mail-bodyanddata-urlattributes without sanitization. An attacker could craft a URL with a raw"character to break out of the HTML attribute and inject arbitrary HTML/JavaScript into the page.Affected attribute:
Proof of concept (curl, bypasses browser URL-encoding):
Changes
Security fix — reflected XSS:
ShariffPlugin.php: applyhtmlspecialchars(..., ENT_QUOTES, 'UTF-8')to$requestedUrlbefore assigning to template (defense in depth — templates already use|escape, but the raw value was passed in).ShariffBlockPlugin.php: same fix.Improvement — suppress console error when backend is not installed:
ShariffPlugin.php/ShariffBlockPlugin.php: only set$backendUrlwhen theshariff-backend/directory actually exists on disk. Previously the attribute was always emitted, causingJSON.parseerrors in the browser console when the optional PHP backend was not installed.templates/shariffButtons.tpl: renderdata-backend-urlonly when$backendUrlis non-null.Testing
Classification