Security: Fix Stored XSS vulnerability in PageRequest#117
Conversation
- Add `prepareForValidation` method to `PageRequest.php` to sanitize input data before validation. - Use `strip_tags` and regex to remove `<script>` tags and clean the `title` field. - Use `HTMLPurifier` with forbidden `script` elements and no caching to sanitize the `content` field. - Add unit test `PageRequestTest` to verify the sanitization logic. Co-authored-by: juzaweb <47020363+juzaweb@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
- Added a `clean_html` helper function in `helpers/helpers.php` to encapsulate HTMLPurifier logic with `script` tags explicitly forbidden and cache disabled. - Refactored `PageRequest::prepareForValidation()` to use the new `clean_html()` helper for sanitizing the `content` field. - Addressed code style issues to satisfy `pint` CI checks. Co-authored-by: juzaweb <47020363+juzaweb@users.noreply.github.com>
1 participant
This pull request addresses a Stored Cross-Site Scripting (XSS) vulnerability in the
PageRequest.phpform request class. Previously, malicious HTML and JavaScript payloads could be submitted via thetitleandcontentfields and stored raw in the database, potentially executing when the page is rendered on the frontend.Changes:
prepareForValidationlifecycle hook toPageRequest.phpto sanitize the incoming request data before it is validated and inserted into the database.titlefield is sanitized by first using a regular expression to strip<script>tags (and their contents) and then applyingstrip_tags()to remove any remaining HTML tags.contentfield is thoroughly sanitized usingHTMLPurifier. The configuration explicitly forbids<script>elements and disables the definition cache (Cache.DefinitionImplset tonull) to prevent caching/permission issues, ensuring safe HTML content is stored.PageRequestTest.php) to explicitly verify that the sanitization logic correctly removes dangerous payloads from both thetitleandcontentfields.PR created automatically by Jules for task 6987435458252747160 started by @juzaweb