From a7aab05e41fa6eb9429a9b6808a6157e8172218d Mon Sep 17 00:00:00 2001 From: kouts Date: Mon, 1 Jun 2026 08:36:14 +0300 Subject: [PATCH 1/2] test: added xss rich text post issue example --- .../fixtures/xssRichTextFalsePositive/app.vue | 5 ++++ .../xssRichTextFalsePositive/nuxt.config.ts | 8 +++++++ .../server/api/rich-text.post.ts | 10 ++++++++ ...xssValidator.richTextFalsePositive.test.ts | 24 +++++++++++++++++++ 4 files changed, 47 insertions(+) create mode 100644 test/fixtures/xssRichTextFalsePositive/app.vue create mode 100644 test/fixtures/xssRichTextFalsePositive/nuxt.config.ts create mode 100644 test/fixtures/xssRichTextFalsePositive/server/api/rich-text.post.ts create mode 100644 test/xssValidator.richTextFalsePositive.test.ts diff --git a/test/fixtures/xssRichTextFalsePositive/app.vue b/test/fixtures/xssRichTextFalsePositive/app.vue new file mode 100644 index 00000000..2b1be090 --- /dev/null +++ b/test/fixtures/xssRichTextFalsePositive/app.vue @@ -0,0 +1,5 @@ + diff --git a/test/fixtures/xssRichTextFalsePositive/nuxt.config.ts b/test/fixtures/xssRichTextFalsePositive/nuxt.config.ts new file mode 100644 index 00000000..aef6ca4e --- /dev/null +++ b/test/fixtures/xssRichTextFalsePositive/nuxt.config.ts @@ -0,0 +1,8 @@ +export default defineNuxtConfig({ + modules: ['../../../src/module'], + security: { + xssValidator: { + methods: ['POST'], + }, + }, +}) diff --git a/test/fixtures/xssRichTextFalsePositive/server/api/rich-text.post.ts b/test/fixtures/xssRichTextFalsePositive/server/api/rich-text.post.ts new file mode 100644 index 00000000..a740eea8 --- /dev/null +++ b/test/fixtures/xssRichTextFalsePositive/server/api/rich-text.post.ts @@ -0,0 +1,10 @@ +import { defineEventHandler, readRawBody } from 'h3' + +export default defineEventHandler(async (event) => { + const body = await readRawBody(event, 'utf8') + + return { + ok: true, + body, + } +}) diff --git a/test/xssValidator.richTextFalsePositive.test.ts b/test/xssValidator.richTextFalsePositive.test.ts new file mode 100644 index 00000000..e81bd5a0 --- /dev/null +++ b/test/xssValidator.richTextFalsePositive.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from 'vitest' +import { fileURLToPath } from 'node:url' +import { fetch, setup } from '@nuxt/test-utils' + +describe('[nuxt-security] XSS validator rich-text false positive', async () => { + await setup({ + rootDir: fileURLToPath(new URL('./fixtures/xssRichTextFalsePositive', import.meta.url)), + }) + + it('allows a rich-text-like JSON payload sent to POST endpoints', async () => { + const res = await fetch('/api/rich-text', { + method: 'POST', + headers: { + 'content-type': 'application/json', + }, + body: JSON.stringify({ + description: '

hello

', + }), + }) + + expect(res.status).toBe(200) + expect(res.statusText).toBe('OK') + }) +}) From a75a06dadb77b2b9a4f643c2d308d3423c1357ca Mon Sep 17 00:00:00 2001 From: kouts Date: Sat, 4 Jul 2026 16:44:23 +0300 Subject: [PATCH 2/2] fix:(#720): prevent false positives on rich-text payloads and remove bypass --- src/runtime/server/middleware/xssValidator.ts | 31 +- src/runtime/server/utils/xssPayload.ts | 71 +++++ test/xssPayload.test.ts | 282 ++++++++++++++++++ 3 files changed, 359 insertions(+), 25 deletions(-) create mode 100644 src/runtime/server/utils/xssPayload.ts create mode 100644 test/xssPayload.test.ts diff --git a/src/runtime/server/middleware/xssValidator.ts b/src/runtime/server/middleware/xssValidator.ts index d215308f..04423bbb 100644 --- a/src/runtime/server/middleware/xssValidator.ts +++ b/src/runtime/server/middleware/xssValidator.ts @@ -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) @@ -11,38 +11,19 @@ export default defineEventHandler(async(event) => { ...rules.xssValidator, escapeHtml: undefined } - if (rules.xssValidator.escapeHtml === false) { - // No html escaping (by default "<" is replaced by "<" and ">" by ">") - 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 - } - 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' diff --git a/src/runtime/server/utils/xssPayload.ts b/src/runtime/server/utils/xssPayload.ts new file mode 100644 index 00000000..dd73fa65 --- /dev/null +++ b/src/runtime/server/utils/xssPayload.ts @@ -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 + +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 { + 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) +} diff --git a/test/xssPayload.test.ts b/test/xssPayload.test.ts new file mode 100644 index 00000000..565ad4b3 --- /dev/null +++ b/test/xssPayload.test.ts @@ -0,0 +1,282 @@ +import { describe, expect, it } from 'vitest' +import { hasMaliciousPayload } from '../src/runtime/server/utils/xssPayload' +import type { IFilterXSSOptions } from 'xss' + +const defaultOptions: IFilterXSSOptions = {} + +describe('hasMaliciousPayload', () => { + describe('safe inputs', () => { + it('returns false for plain text', () => { + expect(hasMaliciousPayload('hello world', defaultOptions)).toBe(false) + }) + + it('returns false for allowed HTML tags', () => { + expect(hasMaliciousPayload('

paragraph

', defaultOptions)).toBe(false) + }) + + it('returns false for allowed attributes', () => { + expect(hasMaliciousPayload('link', defaultOptions)).toBe(false) + }) + + it('returns false for rich text with inline styles', () => { + expect(hasMaliciousPayload('

hello

', defaultOptions)).toBe(false) + }) + + it('returns false for numbers and booleans', () => { + expect(hasMaliciousPayload(42, defaultOptions)).toBe(false) + expect(hasMaliciousPayload(true, defaultOptions)).toBe(false) + expect(hasMaliciousPayload(null, defaultOptions)).toBe(false) + }) + + it('returns false for objects with safe string values', () => { + expect(hasMaliciousPayload({ name: 'John', bio: '

Hello

' }, defaultOptions)).toBe(false) + }) + + it('returns false for arrays with safe values', () => { + expect(hasMaliciousPayload(['hello', 'bold'], defaultOptions)).toBe(false) + }) + }) + + describe('malicious tags', () => { + it('detects ', defaultOptions)).toBe(true) + }) + + it('detects ', defaultOptions)).toBe(true) + }) + + it('detects tags', () => { + expect(hasMaliciousPayload('', defaultOptions)).toBe(true) + }) + + it('detects tags', () => { + expect(hasMaliciousPayload('', defaultOptions)).toBe(true) + }) + + it('detects with nested payload', () => { + expect(hasMaliciousPayload('', defaultOptions)).toBe(true) + }) + }) + + describe('malicious attributes', () => { + it('detects onclick handlers', () => { + expect(hasMaliciousPayload('
click
', defaultOptions)).toBe(true) + }) + + it('detects onerror handlers', () => { + expect(hasMaliciousPayload('', defaultOptions)).toBe(true) + }) + + it('detects onload handlers', () => { + expect(hasMaliciousPayload('', defaultOptions)).toBe(true) + }) + + it('detects onmouseover handlers', () => { + expect(hasMaliciousPayload('hover', defaultOptions)).toBe(true) + }) + }) + + describe('dangerous URL attributes', () => { + it('detects javascript: in href', () => { + expect(hasMaliciousPayload('click', defaultOptions)).toBe(true) + }) + + it('detects javascript: in src', () => { + expect(hasMaliciousPayload('', defaultOptions)).toBe(true) + }) + + it('detects data: URI in src', () => { + expect(hasMaliciousPayload('', defaultOptions)).toBe(true) + }) + }) + + describe('nested payloads', () => { + it('detects malicious value in nested object', () => { + const payload = { + user: { + profile: { + bio: '' + } + } + } + expect(hasMaliciousPayload(payload, defaultOptions)).toBe(true) + }) + + it('detects malicious value in array', () => { + const payload = ['safe text', ''] + expect(hasMaliciousPayload(payload, defaultOptions)).toBe(true) + }) + + it('detects malicious value in array within object', () => { + const payload = { + tags: ['bold', ''] + } + expect(hasMaliciousPayload(payload, defaultOptions)).toBe(true) + }) + + it('returns false when all nested values are safe', () => { + const payload = { + items: [ + { title: 'Hello', content: '

World

' }, + { title: 'Foo', content: 'Bar' } + ] + } + expect(hasMaliciousPayload(payload, defaultOptions)).toBe(false) + }) + }) + + describe('edge cases', () => { + it('returns false for empty string', () => { + expect(hasMaliciousPayload('', defaultOptions)).toBe(false) + }) + + it('returns false for empty object', () => { + expect(hasMaliciousPayload({}, defaultOptions)).toBe(false) + }) + + it('returns false for empty array', () => { + expect(hasMaliciousPayload([], defaultOptions)).toBe(false) + }) + + it('handles closing tag without opening (not malicious)', () => { + expect(hasMaliciousPayload('', defaultOptions)).toBe(false) + }) + + it('detects uppercase event handlers', () => { + expect(hasMaliciousPayload('
x
', defaultOptions)).toBe(true) + }) + + it('detects mixed-case event handlers', () => { + expect(hasMaliciousPayload('
x
', defaultOptions)).toBe(true) + }) + + it('handles strings that look like HTML but use unknown tags', () => { + // Non-whitelisted tags trigger detection even if harmless-looking + expect(hasMaliciousPayload('hello', defaultOptions)).toBe(true) + }) + + it('flags angle brackets that parse as tags (expected false positive)', () => { + // The xss parser sees `< 2 and 3 >` as a non-whitelisted tag — this is an acceptable + // trade-off for a security validator (overly cautious with angle brackets) + expect(hasMaliciousPayload('1 < 2 and 3 > 1', defaultOptions)).toBe(true) + }) + + it('flags any less-than that the parser sees as a tag opener (expected false positive)', () => { + // Even `a < b` is parsed as containing a tag — this is inherent to the xss library's parser + expect(hasMaliciousPayload('a < b', defaultOptions)).toBe(true) + }) + + it('allows greater-than signs (not tag openers)', () => { + expect(hasMaliciousPayload('a > b', defaultOptions)).toBe(false) + }) + + it('detects javascript: with leading whitespace in href', () => { + expect(hasMaliciousPayload('x', defaultOptions)).toBe(true) + }) + + it('detects vbscript: protocol in href', () => { + expect(hasMaliciousPayload('x', defaultOptions)).toBe(true) + }) + + it('allows legitimate data: image URIs in src', () => { + // data:image/png is considered safe by the xss library's default safeAttrValue + expect(hasMaliciousPayload('', defaultOptions)).toBe(false) + }) + + it('detects data:image/svg+xml with embedded payload', () => { + // SVG can contain scripts and event handlers + expect(hasMaliciousPayload('', defaultOptions)).toBe(true) + }) + + it('allows legitimate raster data URIs (png, jpg, etc.)', () => { + expect(hasMaliciousPayload('', defaultOptions)).toBe(false) + expect(hasMaliciousPayload('', defaultOptions)).toBe(false) + expect(hasMaliciousPayload('', defaultOptions)).toBe(false) + expect(hasMaliciousPayload('', defaultOptions)).toBe(false) + }) + + it('detects data: image/ protocol with leading whitespace', () => { + expect(hasMaliciousPayload('', defaultOptions)).toBe(true) + expect(hasMaliciousPayload('x', defaultOptions)).toBe(true) + }) + + it('detects SVG with event handler', () => { + expect(hasMaliciousPayload('', defaultOptions)).toBe(true) + }) + + it('detects payload hidden in deeply nested structure', () => { + const payload = { a: { b: { c: { d: { e: '' } } } } } + expect(hasMaliciousPayload(payload, defaultOptions)).toBe(true) + }) + + it('handles object with prototype pollution keys safely', () => { + const payload = { __proto__: '', constructor: '' } + expect(hasMaliciousPayload(payload, defaultOptions)).toBe(true) + }) + + it('handles multipart-like array of objects (as returned by readMultipartFormData)', () => { + const payload = [ + { name: 'file', filename: 'safe.txt', data: Buffer.from('hello') }, + { name: 'field', data: '' } + ] + expect(hasMaliciousPayload(payload, defaultOptions)).toBe(true) + }) + + it('returns false for multipart-like array with safe values', () => { + const payload = [ + { name: 'title', data: 'My Document' }, + { name: 'body', data: '

Safe content

' } + ] + expect(hasMaliciousPayload(payload, defaultOptions)).toBe(false) + }) + + it('returns true when nesting exceeds depth limit (safety cutoff)', () => { + // Build a payload nested beyond MAX_DEPTH (20) + let payload: any = { value: '' } + for (let i = 0; i < 25; i++) { + payload = { safe: payload } + } + expect(hasMaliciousPayload(payload, defaultOptions)).toBe(true) + }) + + it('still detects malicious content at edge of depth limit', () => { + // Exactly at MAX_DEPTH, detection should still work + let payload: any = { value: '' } + for (let i = 0; i < 19; i++) { + payload = { safe: payload } + } + expect(hasMaliciousPayload(payload, defaultOptions)).toBe(true) + }) + + it('returns false for safe content deep but within depth limit', () => { + let payload: any = { value: '

safe

' } + for (let i = 0; i < 15; i++) { + payload = { safe: payload } + } + expect(hasMaliciousPayload(payload, defaultOptions)).toBe(false) + }) + + it('does not skip detection when payload contains statusMessage', () => { + // statusMessage is user-controlled request data — must not affect detection + const payload = { statusMessage: 'Bad Request', description: '' } + expect(hasMaliciousPayload(payload, defaultOptions)).toBe(true) + }) + }) + + describe('custom options', () => { + it('respects custom whiteList allowing script tags', () => { + const options: IFilterXSSOptions = { + whiteList: { script: [], p: [], a: ['href'] } + } + expect(hasMaliciousPayload('', options)).toBe(false) + }) + + it('detects non-whitelisted tags with custom whiteList', () => { + const options: IFilterXSSOptions = { + whiteList: { p: [], a: ['href'] } + } + expect(hasMaliciousPayload('
hello
', options)).toBe(true) + }) + }) +})