Security Hardening#53
Merged
Merged
Conversation
Alert HTML content was stored without sanitization, enabling stored XSS if an attacker could create alerts (e.g. via privilege escalation). The setHtmlAttribute mutator ensures all HTML is cleaned through HTMLPurifier before persistence, matching the pattern already established on Group, Party, User, and Network models.
AlertBanner rendered alert HTML via v-html without client-side sanitization. This adds DOMPurify as a defense-in-depth layer alongside the server-side Purify mutator on the Alert model, matching the pattern used in ReadMore.vue.
The escapeHtml function was duplicated verbatim in GroupActions.vue and GroupPage.vue. This extracts it into a shared helper module so there is one canonical implementation for HTML-escaping user data in Vue template interpolations.
The text prop path rendered via v-html without DOMPurify sanitization. While no current callers use this path, it was an unsanitized sink that could introduce XSS if a future caller wired up the text prop. Also fixed a ReferenceError where text.length and maxChars were missing their this. prefix in the needsTruncating computed property.
The invite flows in GroupController and PartyController interpolated
user-supplied email addresses into translation strings rendered via
{!! !!} without escaping. This wraps them with e() to prevent XSS
through crafted email-like input.
Mobile click-to-call links (tel: URIs) are a reasonable thing to allow in sanitized descriptions and carry no XSS risk unlike javascript: or data: URIs.
Bumps axios from ^1.6.4 to ^1.15.0 to exit the vulnerable range (1.0.0-1.14.0) which includes SSRF, credential leakage, and CSRF vulnerabilities. This is the minimum version bump needed to resolve the CVEs without a major version change.
Collaborator
Author
|
QA 👍 Things seem to be fixed. |
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
2 participants
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.
Description
This PR addresses the remaining High and Medium severity issues from the security audit report and resolves all unresolved review comments from PR #51.
Deferred: M3 (quill-paste-smart) requires Quill 2.x which is a breaking change — risk is mitigated by server-side Purify mutators.
QA Notes:
We will need to test this on the test cluster.