Fix #545: Add support for HTML5 pointer and transition event handlers - #909
Fix #545: Add support for HTML5 pointer and transition event handlers#909jeffrey-theog06 wants to merge 4 commits into
Conversation
|
Hey @jeffrey-theog06 , great find and clean fix! Before we can merge, one thing needs addressing: CI is failing [lint-java] — the project enforces Google Java Format via Spotless. Please run the following before pushing: mvn spotless:apply After that, verify locally with: mvn spotless:check It should exit with BUILD SUCCESS — then push and the [lint-java] check should go green. The dependency-review failure is a pre-existing issue in the repo's dependency tree unrelated to your change. Thanks! |
Alright.. Thanks for coming up and suggesting it.. will work on it and update my PR soon.. |
There was a problem hiding this comment.
🟡 Changes recommended
Additional trigger groups were added without corresponding unit tests for the newly supported animation/transition/auxclick handlers, and one new test fixture uses unnecessarily slangy content.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR updates the utils.FindXSS backend detection logic so that successful XSS submissions using newer HTML5 event handler attributes (notably pointer/animation/transition handlers) are recognized, allowing challenges to correctly award completion (Fixes #545).
Changes:
- Expanded
FindXSS.javascriptTriggersto include pointer, animation, transition event handlers, plusonauxclick. - Added unit tests in
FindXSSTestto validate detection foronpointeroverandonpointerenter.
File summaries
| File | Description |
|---|---|
| src/main/java/utils/FindXSS.java | Extends the hardcoded list of JavaScript-triggering attributes to cover additional modern HTML5 event handlers. |
| src/test/java/utils/FindXSSTest.java | Adds test coverage for pointer-event-based XSS detection. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| "onanimationcancel", | ||
| "onanimationend", | ||
| "onanimationiteration", | ||
| "onanimationstart", | ||
| "ontransitioncancel", |
…d remove slangy alert content
There was a problem hiding this comment.
🟢 Ready to approve
The added triggers align with the existing detection approach and are backed by targeted unit tests that exercise the new behavior.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Description
This PR fixes an issue where successful XSS attacks using modern HTML5 event handlers (such as
onpointerover,onpointerenter, etc.) were not recognized by the Security Shepherd backend, preventing players from being awarded challenge completion.Root Cause
In XSS challenges where standard event handlers (like
onclickoronerror) are filtered, players bypass these filters using alternative event handlers. However, the backend validation helperFindXSS.javavalidates if XSS was successful by searching the sanitized input for specific event handlers defined in a hardcoded listjavascriptTriggers. Because pointer, animation, and transition events were missing from this list, the server did not detect the successful exploitation.Changes
FindXSS.java: Added the following event handlers to thejavascriptTriggersarray:onpointerdown,onpointerup,onpointercancel,onpointermove,onpointerover,onpointerout,onpointerenter,onpointerleave,ongotpointercapture,onlostpointercapture,onpointerrawupdateonanimationcancel,onanimationend,onanimationiteration,onanimationstartontransitioncancel,ontransitionend,ontransitionrun,ontransitionstartonauxclickFindXSSTest.java: Added unit tests (search_detectsOnpointeroverAlert,search_detectsOnpointerenterAlert) to verify that the pointer events successfully trigger XSS detection.Closes #545