Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 6 additions & 25 deletions src/runtime/server/middleware/xssValidator.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { defineEventHandler, createError, getQuery, readBody, readMultipartFormData } from 'h3'
import { FilterXSS, type IFilterXSSOptions } from 'xss'
import { type IFilterXSSOptions } from 'xss'
import { resolveSecurityRules } from '../../nitro/context'
import type { HTTPMethod } from '../../../types/middlewares'
import { hasMaliciousPayload } from '../utils/xssPayload'

export default defineEventHandler(async(event) => {
const rules = resolveSecurityRules(event)
Expand All @@ -11,38 +11,19 @@ export default defineEventHandler(async(event) => {
...rules.xssValidator,
escapeHtml: undefined

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
if (rules.xssValidator.escapeHtml === false) {
// No html escaping (by default "<" is replaced by "&lt;" and ">" by "&gt;")
filterOpt.escapeHtml = (value: string) => value
}
const xssValidator = new FilterXSS(filterOpt)

if (event.node.req.socket.readyState !== 'readOnly') {
if (
rules.xssValidator.methods &&
rules.xssValidator.methods.includes(
event.node.req.method! as HTTPMethod
)
) {
const method = event.node.req.method
if (method && (rules.xssValidator.methods as readonly string[]).includes(method)) {
const valueToFilter =
event.node.req.method === 'GET'
method === 'GET'
? getQuery(event)
: event.node.req.headers['content-type']?.includes(
'multipart/form-data'
)
? await readMultipartFormData(event)
: await readBody(event)
// Fix for problems when one middleware is returning an error and it is catched in the next
if (valueToFilter && Object.keys(valueToFilter).length) {
// Only skip XSS filtering if statusMessage === 'Bad Request' (for error propagation)
if (valueToFilter.statusMessage === 'Bad Request') {
return
}
Comment on lines -37 to -40

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 stringifiedValue = JSON.stringify(valueToFilter)
const processedValue = xssValidator.process(
JSON.stringify(valueToFilter)
)
if (processedValue !== stringifiedValue) {
if (hasMaliciousPayload(valueToFilter, filterOpt)) {
const badRequestError = {
statusCode: 400,
statusMessage: 'Bad Request'
Expand Down
71 changes: 71 additions & 0 deletions src/runtime/server/utils/xssPayload.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { FilterXSS, type IFilterXSSOptions } from 'xss'

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...


function createMaliciousHtmlDetector (baseOptions: IFilterXSSOptions) {
let isMalicious = false

const detector = new FilterXSS({
...baseOptions,
onIgnoreTag (tag, html, options) {
if (!options.isClosing) {
isMalicious = true
}
return baseOptions.onIgnoreTag?.(tag, html, options)
},
onIgnoreTagAttr (tag, name, attrValue, isWhiteAttr) {
if (name.startsWith('on')) {
isMalicious = true
}
return baseOptions.onIgnoreTagAttr?.(tag, name, attrValue, isWhiteAttr)
},
safeAttrValue (tag, name, attrValue, cssFilter) {
if (DANGEROUS_URL_ATTRS.has(name) && DANGEROUS_PROTOCOLS.test(attrValue)) {
isMalicious = true
}
if (baseOptions.safeAttrValue) {
return baseOptions.safeAttrValue(tag, name, attrValue, cssFilter)
}
return attrValue
}
})

return (value: string): boolean => {
isMalicious = false
detector.process(value)
return isMalicious
}
}

function isRecord (value: unknown): value is Record<string, unknown> {
return !!value && typeof value === 'object' && !Array.isArray(value)
}

export function hasMaliciousPayload (value: unknown, options: IFilterXSSOptions): boolean {
const isMaliciousHtml = createMaliciousHtmlDetector(options)

const inspect = (current: unknown, depth = 0): boolean => {
if (depth > MAX_DEPTH) {
return true
}

if (typeof current === 'string') {
return isMaliciousHtml(current)
}

if (Array.isArray(current)) {
return current.some(item => inspect(item, depth + 1))
}

if (isRecord(current)) {
return Object.values(current)
.some(item => inspect(item, depth + 1))
}

return false
}

return inspect(value)
}
5 changes: 5 additions & 0 deletions test/fixtures/xssRichTextFalsePositive/app.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<template>
<div>
<NuxtPage />
</div>
</template>
8 changes: 8 additions & 0 deletions test/fixtures/xssRichTextFalsePositive/nuxt.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export default defineNuxtConfig({
modules: ['../../../src/module'],
security: {
xssValidator: {
methods: ['POST'],
},
},
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { defineEventHandler, readRawBody } from 'h3'

export default defineEventHandler(async (event) => {
const body = await readRawBody(event, 'utf8')

return {
ok: true,
body,
}
})
Loading
Loading