Ghost icon indicating copy to clipboard operation
Ghost copied to clipboard

🐛 Fix XSS Vulnerability in SVG Upload Handling

Open TeneBrae93 opened this issue 1 year ago • 1 comments

This PR addresses a vulnerability in the handling of SVG file uploads (especially in staff profile pictures) within Ghost CMS. By integrating DOMPurify, we ensure that SVG files are sanitized to remove potential XSS attacks, enhancing the security of the platform without affecting the core functionalities.

Changes Made:

  • Integrated DOMPurify to sanitize SVG content during the upload process in update.js.
  • Updated package.json to include dompurify as a dependency, ensuring the sanitization process is robust and effective.

Impact:

  • Enhances the security of Ghost CMS by preventing XSS attacks through SVG uploads.
  • Maintains the user experience and functionalities related to SVG uploads, ensuring only malicious content is targeted and removed.

Testing:

  • Manual testing was conducted with various SVG files to ensure legitimate uploads remain unaffected while potential XSS payloads are sanitized.

This fix contributes to the ongoing effort to maintain and improve the security posture of Ghost CMS, ensuring it remains a safe and reliable platform for users and developers alike.

Additional Notes:

  • This PR is submitted in response to the ability to execute JavaScript in SVG files when uploaded as profile pictures. Further discussions and contributions to this fix are welcome. This was initially submitted to [email protected] but was declined since staff are trusted users. This PR will fix the core issue and prevent XSS in SVG uploads.

Thank you for considering this contribution to enhance Ghost CMS's security.

TeneBrae93 avatar Feb 02 '24 22:02 TeneBrae93

If the sanitized SVG content is then served via an SVG file (meaning Content-Type: image/svg+xml) the sanitization should have only the SVG namespace (taken example from here):

const clean = DOMPurify.sanitize(dirty, {USE_PROFILES: {svg: true}});

Because if not, we introduce namespace confusion and could be prone to mXSS. for example, in a normal parsing (as done in DOMPurify): image vs how the browser will parse it when the Content-Type: image/svg+xml image

Yaniv-git avatar Feb 28 '24 08:02 Yaniv-git

@TeneBrae93 Thanks for the comprehensive report and proposed fix! I've merged a PR here. dompurify had a few issues, namely that it always modifies the base file/content and doesn't provide a flag to tell you if it actually did any cleaning, meaning we couldn't write a replicable test using it.

9larsons avatar May 28 '24 14:05 9larsons