Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Update) Remove XSS cleaner and remove XSS vulnerabilities #3222

Draft
wants to merge 1 commit into
base: 8.x.x
Choose a base branch
from

Conversation

Roardom
Copy link
Collaborator

@Roardom Roardom commented Nov 6, 2023

We've been mostly relying on the 3rd party xss cleaner to make sure user submitted content is clean. This PR fixes up any leftover holes in the bbcode parser that allow xss vulnerabilities, and as a result, the 3rd party library isn't needed anymore. It cleans responsibly by first, running htmlspecialchars() over the content, followed by validating any untrusted input used inside html attributes. It validates urls by returning them as redirects relying on the browser not supporting executable protocols (like javascript:).

@Roardom
Copy link
Collaborator Author

Roardom commented Nov 20, 2023

It validates urls by returning them as redirects relying on the browser not supporting executable protocols (like javascript:).

This is the definition of an open redirect vulnerability which is unacceptable and will require rethinking.

Copy link

what-the-diff bot commented Jan 5, 2024

PR Summary

  • Enhanced BBCode Regular Expression
    The old code checker for the namedquote configuration has been improved to allow any character in the quote tag.

  • Removal of namedlink BBCode configuration
    The namedlink BBCode configuration has been removed from the system.

  • Significant improvements to parse method
    This method now sanitizes and effectively encodes special characters in our source string. It also converts BBCode tags like url, img, and video to equivalent HTML tags. The HTML tags produced from this conversion are more secure with attributes like href and src properly sanitized. Additionally, performance has been improved by removing some redundant block elements.

  • Introducing sanitizeURL method
    A new method to make sure URLs are safe and valid before use.

  • Optimized Livewire components
    Changes have been made to how the Livewire components handle data entry. These components now directly update or create comments and replies with sanitized text, improving security and integrity of data.

  • Removed redundant set methods from models
    Certain methods from various models like Article, Message, Playlist, etc. were unnecessary and hence have been eliminated.

  • Dependency cleanup
    A dependency named voku/anti-xss that is no longer needed has been removed from the project.

  • Database Update
    A new migration file is added to correctly decode certain fields in the database records. This fixes and normalises these fields to ensure consistency across the application.

@Roardom
Copy link
Collaborator Author

Roardom commented Jan 5, 2024

It validates urls by returning them as redirects relying on the browser not supporting executable protocols (like javascript:).

This is the definition of an open redirect vulnerability which is unacceptable and will require rethinking.

Open redirect vulnerabilities are only valid for links ([url]). [img] bbcode could technically still be implemented like this correctly, but [url] bbcode should be sent to a page that counts down from e.g. 15 seconds before redirecting to the link to avoid this vulnerability. I've opted to whitelist protocols instead to avoid this issue, but in the future this can be perhaps redesigned.

@Roardom Roardom force-pushed the bbcode-improvements branch 2 times, most recently from a852f82 to 8ab3967 Compare March 1, 2024 12:53
@HDVinnie HDVinnie closed this Mar 27, 2024
@Roardom Roardom reopened this May 26, 2024
@Roardom Roardom changed the base branch from 7.x.x to 8.x.x May 26, 2024 16:34
We've been mostly relying on the 3rd party xss cleaner to make sure user submitted content is clean. This PR fixes up any leftover holes in the bbcode parser that allow xss vulnerabilities, and as a result, the 3rd party library isn't needed anymore. It cleans responsibly by first, running `htmlspecialchars()` over the content, followed by validating any untrusted input used inside html attributes. It validates urls by returning them as redirects relying on the browser not supporting executable protocols (like `javascript:`).
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.

None yet

2 participants