Fix/xss rich text and statusmessage bypass#736
Conversation
|
@kouts is attempting to deploy a commit to the Baroshem's projects Team on Vercel. A member of the Team first needs to authorize it. |
| // Only skip XSS filtering if statusMessage === 'Bad Request' (for error propagation) | ||
| if (valueToFilter.statusMessage === 'Bad Request') { | ||
| return | ||
| } |
There was a problem hiding this comment.
Removed this check because statusMessage comes from the request body, so it's user controlled. An attacker could send { "statusMessage": "Bad Request", "payload": "<script>alert(1)</script>" } and bypass XSS validation entirely.
If middleware-to-middleware signaling is needed here, event.context would be the right mechanism since it can only be set by trusted server code.
|
|
||
| const DANGEROUS_URL_ATTRS = new Set(['href', 'src', 'background', 'style']) | ||
| const DANGEROUS_PROTOCOLS = /^\s*(?:javascript|vbscript|data\s*:\s*(?!image\/(?!svg)))/i | ||
| const MAX_DEPTH = 20 |
There was a problem hiding this comment.
Depth limit here prevents stack overflow on maliciously nested payloads. Returns true (conservative) when exceeded, meaning deeply nested payloads are treated as suspicious rather than silently accepted. 20 is generous enough for any real API payload while capping recursion before it can crash...
| @@ -11,38 +11,19 @@ export default defineEventHandler(async(event) => { | |||
| ...rules.xssValidator, | |||
| escapeHtml: undefined | |||
There was a problem hiding this comment.
escapeHtml doesn't affect our detection, but the XssValidator config type has it as boolean | undefined while IFilterXSSOptions expects a function type. Explicitly overriding to undefined keeps TypeScript happy without an assertion.
Types of changes
Description
Resolves: #720
Replaces the XSS validator's stringify-and-compare detection with hook-based malicious payload detection. The old approach compared
xss.process(JSON.stringify(body))to the input and rejected on any difference, which caused false positives for legitimate rich-text HTML like e.g.<p style="text-align:center">hello</p>.The new approach uses the
xsslibrary's hooks to detect specific malicious patterns:<script>,<iframe>,<svg>, etc.) viaonIgnoreTagon*) viaonIgnoreTagAttrjavascript:,vbscript:, non-imagedata:) viasafeAttrValuewith an inline regexAlso fixes a security bypass where user-controlled
statusMessage: "Bad Request"in the request body would skip XSS validation entirely.Defense-in-depth:
FilterXSSinstance reused per request instead of per-stringdata:image/svg+xml(which can embed scripts) while allowing raster image formats (png, jpeg, gif, webp)Checklist: