From b9f3a8aa0c8d18b69474381086ef6c6c4919b9e8 Mon Sep 17 00:00:00 2001 From: Mark McIntosh Date: Tue, 24 Feb 2026 01:04:12 -0500 Subject: [PATCH] fix: sanitize public form submission data to prevent stored XSS Form submissions from unauthenticated users were stored with raw string values, allowing script injection payloads to execute when admins view submissions. Add recursive sanitizeDeep() that HTML-encodes all string values in the arbitrary submission JSON before storage. --- packages/core/src/routes/public-forms.test.ts | 220 ++++++++++++++++++ packages/core/src/routes/public-forms.ts | 29 ++- 2 files changed, 248 insertions(+), 1 deletion(-) create mode 100644 packages/core/src/routes/public-forms.test.ts diff --git a/packages/core/src/routes/public-forms.test.ts b/packages/core/src/routes/public-forms.test.ts new file mode 100644 index 000000000..a1e9587db --- /dev/null +++ b/packages/core/src/routes/public-forms.test.ts @@ -0,0 +1,220 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { Hono } from 'hono' + +// Track what gets stored +let capturedSubmissionData: string | null = null + +// Mock TurnstileService — disabled by default so submissions go through +vi.mock('../plugins/core-plugins/turnstile-plugin/services/turnstile', () => ({ + TurnstileService: class MockTurnstileService { + getSettings = vi.fn().mockResolvedValue(null) + isEnabled = vi.fn().mockResolvedValue(false) + verifyToken = vi.fn().mockResolvedValue({ success: true }) + } +})) + +import { publicFormsRoutes } from './public-forms' + +function createMockDb(formExists = true) { + return { + prepare: vi.fn().mockImplementation((sql: string) => { + // Form lookup + if (sql.includes('FROM forms WHERE')) { + return { + bind: vi.fn().mockReturnThis(), + first: vi.fn().mockResolvedValue( + formExists + ? { + id: 'form-1', + name: 'test_form', + display_name: 'Test Form', + is_active: 1, + turnstile_enabled: 0, + turnstile_settings: null + } + : null + ), + all: vi.fn().mockResolvedValue({ results: [] }), + run: vi.fn().mockResolvedValue({ success: true }) + } + } + // INSERT into form_submissions — capture the data + if (sql.includes('INSERT INTO form_submissions')) { + return { + bind: vi.fn().mockImplementation((...args: any[]) => { + // submission_data is the 3rd bound param (index 2) + capturedSubmissionData = args[2] + return { + run: vi.fn().mockResolvedValue({ success: true }), + first: vi.fn().mockResolvedValue(null), + all: vi.fn().mockResolvedValue({ results: [] }) + } + }), + run: vi.fn().mockResolvedValue({ success: true }), + first: vi.fn().mockResolvedValue(null), + all: vi.fn().mockResolvedValue({ results: [] }) + } + } + // UPDATE forms (submission count) + if (sql.includes('UPDATE forms')) { + return { + bind: vi.fn().mockReturnThis(), + run: vi.fn().mockResolvedValue({ success: true }), + first: vi.fn().mockResolvedValue(null), + all: vi.fn().mockResolvedValue({ results: [] }) + } + } + // Default + return { + bind: vi.fn().mockReturnThis(), + first: vi.fn().mockResolvedValue(null), + all: vi.fn().mockResolvedValue({ results: [] }), + run: vi.fn().mockResolvedValue({ success: true }) + } + }) + } +} + +function createTestApp(db: any) { + const app = new Hono() + + app.use('/api/forms/*', async (c, next) => { + c.env = { DB: db } as any + await next() + }) + + app.route('/api/forms', publicFormsRoutes) + return app +} + +describe('POST /api/forms/:identifier/submit — XSS sanitization', () => { + beforeEach(() => { + vi.clearAllMocks() + capturedSubmissionData = null + }) + + it('should HTML-encode script tags in string fields', async () => { + const db = createMockDb() + const app = createTestApp(db) + + const res = await app.request('/api/forms/form-1/submit', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + data: { + name: 'John', + comment: '' + } + }) + }) + + const json = await res.json() as any + expect(res.status).toBe(200) + expect(json.success).toBe(true) + + // Verify stored data has encoded HTML + const stored = JSON.parse(capturedSubmissionData!) + expect(stored.name).toBe('John') + expect(stored.comment).toBe('<script>alert("xss")</script>') + }) + + it('should sanitize nested objects recursively', async () => { + const db = createMockDb() + const app = createTestApp(db) + + const res = await app.request('/api/forms/form-1/submit', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + data: { + user: { + name: '', + address: { + street: '123 Main St', + city: 'Baltimore' + } + } + } + }) + }) + + expect(res.status).toBe(200) + const stored = JSON.parse(capturedSubmissionData!) + expect(stored.user.name).toBe('<img src=x onerror=alert(1)>') + expect(stored.user.address.street).toBe('123 Main St') + expect(stored.user.address.city).toBe('<b>Baltimore</b>') + }) + + it('should sanitize arrays of strings', async () => { + const db = createMockDb() + const app = createTestApp(db) + + const res = await app.request('/api/forms/form-1/submit', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + data: { + tags: ['safe', '', 'also safe'] + } + }) + }) + + expect(res.status).toBe(200) + const stored = JSON.parse(capturedSubmissionData!) + expect(stored.tags).toEqual([ + 'safe', + '<script>bad</script>', + 'also safe' + ]) + }) + + it('should pass through numbers, booleans, and null unchanged', async () => { + const db = createMockDb() + const app = createTestApp(db) + + const res = await app.request('/api/forms/form-1/submit', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + data: { + age: 25, + active: true, + optional: null, + name: 'Test' + } + }) + }) + + expect(res.status).toBe(200) + const stored = JSON.parse(capturedSubmissionData!) + expect(stored.age).toBe(25) + expect(stored.active).toBe(true) + expect(stored.optional).toBeNull() + expect(stored.name).toBe('Test') + }) + + it('should handle event handler injection attempts', async () => { + const db = createMockDb() + const app = createTestApp(db) + + const res = await app.request('/api/forms/form-1/submit', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + data: { + field1: '" onmouseover="alert(1)', + field2: "' onclick='alert(1)'", + field3: '' + } + }) + }) + + expect(res.status).toBe(200) + const stored = JSON.parse(capturedSubmissionData!) + // All angle brackets and quotes should be encoded + expect(stored.field1).not.toContain('"') + expect(stored.field2).not.toContain("'") + expect(stored.field3).not.toContain('<') + expect(stored.field3).not.toContain('>') + }) +}) diff --git a/packages/core/src/routes/public-forms.ts b/packages/core/src/routes/public-forms.ts index a525ce883..229889c0d 100644 --- a/packages/core/src/routes/public-forms.ts +++ b/packages/core/src/routes/public-forms.ts @@ -1,5 +1,28 @@ import { Hono } from 'hono' import { TurnstileService } from '../plugins/core-plugins/turnstile-plugin/services/turnstile' +import { sanitizeInput } from '../utils/sanitize' + +/** + * Recursively sanitize all string values in arbitrary JSON data. + * HTML-encodes entities (e.g., < becomes <) to prevent stored XSS + * when form submission data is rendered in admin templates. + */ +function sanitizeDeep(value: unknown): unknown { + if (typeof value === 'string') { + return sanitizeInput(value) + } + if (Array.isArray(value)) { + return value.map(sanitizeDeep) + } + if (value !== null && typeof value === 'object') { + const result: Record = {} + for (const [k, v] of Object.entries(value)) { + result[k] = sanitizeDeep(v) + } + return result + } + return value // numbers, booleans, null pass through +} type Bindings = { DB: D1Database @@ -538,6 +561,10 @@ publicFormsRoutes.post('/:identifier/submit', async (c) => { } } + // Sanitize all string values in submission data to prevent stored XSS. + // HTML-encodes entities (e.g., < becomes <) before storage. + const sanitizedData = sanitizeDeep(body.data) as Record + // Create submission const submissionId = crypto.randomUUID() const now = Date.now() @@ -550,7 +577,7 @@ publicFormsRoutes.post('/:identifier/submit', async (c) => { `).bind( submissionId, form.id, - JSON.stringify(body.data), + JSON.stringify(sanitizedData), null, // user_id (for authenticated users) c.req.header('cf-connecting-ip') || null, c.req.header('user-agent') || null,