diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 7902c442..680a5b17 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -30,3 +30,8 @@ **Vulnerability:** Known high-severity vulnerabilities discovered by the audit in `js-yaml` and `nanoid` packages. **Learning:** Deeply nested dependencies (`js-yaml` via `eslint`, `nanoid` via `vitest/vite`) may expose the application to DoS or logic loops. **Prevention:** Use `pnpm.overrides` in the root `package.json` to enforce patched versions across all transitive paths in a pnpm workspace. + +## 2025-02-18 - [Fix CSV Formula Injection in Export] +**Vulnerability:** CSV Formula Injection (Spreadsheet Macro Injection) vulnerability in the CSV export functionality for sessions. +**Learning:** If user-controlled data starting with `=, +, -, @, \t, or \r` is exported directly to CSV and opened in Excel or similar applications, it can be executed as a malicious formula or macro. Simply quoting the CSV field (`"value"`) is insufficient to prevent formula execution in spreadsheet software. +**Prevention:** Prepend a single quote (`'`) to values that start with a formula prefix (`=, +, -, @, \t, \r`) unless the type is explicitly a number. This safely forces spreadsheet applications to treat the value as plain text. Ensure this utility is tested in isolation. diff --git a/package.json b/package.json index d085ba62..bd1f7c99 100644 --- a/package.json +++ b/package.json @@ -23,6 +23,7 @@ "hono": "^4.12.34", "js-yaml": "4.3.1", "nanoid": "3.3.18", + "deepmerge-ts": "8.0.0", "@auth/core": "^0.41.3", "sharp": "^0.35.3", "postcss": "^8.5.18", diff --git a/packages/web/src/app/api/orgs/[orgSlug]/dashboard/sessions/route.ts b/packages/web/src/app/api/orgs/[orgSlug]/dashboard/sessions/route.ts index 7d6a4d4a..1e02bf60 100644 --- a/packages/web/src/app/api/orgs/[orgSlug]/dashboard/sessions/route.ts +++ b/packages/web/src/app/api/orgs/[orgSlug]/dashboard/sessions/route.ts @@ -10,6 +10,7 @@ import { resolveOrgScopedProjectIds, } from '@/lib/server/dashboard-route-helper' import { canAccessIndividualData, forbiddenByRole } from '@/lib/server/rbac' +import { csvField } from '@/lib/server/csv/export' export const runtime = 'nodejs' export const dynamic = 'force-dynamic' @@ -71,12 +72,6 @@ function mapSessionItem(session: SessionWithInclude): SessionItem { } } -function csvField(value: string | number | null | undefined) { - if (value === null || value === undefined) return '' - const text = String(value) - return /[",\r\n]/.test(text) ? `"${text.replaceAll('"', '""')}"` : text -} - function buildSessionsCsv(sessions: SessionWithInclude[]) { const headers = [ 'Session ID', diff --git a/packages/web/src/lib/server/csv/export.test.ts b/packages/web/src/lib/server/csv/export.test.ts new file mode 100644 index 00000000..2f29ac12 --- /dev/null +++ b/packages/web/src/lib/server/csv/export.test.ts @@ -0,0 +1,41 @@ +import { describe, it, expect } from 'vitest' +import { csvField } from './export' + +describe('csvField', () => { + it('returns empty string for null or undefined', () => { + expect(csvField(null)).toBe('') + expect(csvField(undefined)).toBe('') + }) + + it('formats normal strings correctly', () => { + expect(csvField('hello')).toBe('hello') + expect(csvField('hello world')).toBe('hello world') + }) + + it('formats numbers correctly', () => { + expect(csvField(123)).toBe('123') + expect(csvField(0)).toBe('0') + expect(csvField(-123)).toBe('-123') + }) + + it('escapes strings with quotes, commas, or newlines', () => { + expect(csvField('hello,world')).toBe('"hello,world"') + expect(csvField('hello\nworld')).toBe('"hello\nworld"') + expect(csvField('hello\rworld')).toBe('"hello\rworld"') + expect(csvField('hello"world')).toBe('"hello""world"') + }) + + it('prepends single quote for strings vulnerable to CSV Formula Injection', () => { + expect(csvField('=1+1')).toBe('"\'=1+1"') + expect(csvField('=SUM(A1:A2)')).toBe('"\'=SUM(A1:A2)"') + expect(csvField('+1+1')).toBe('"\'+1+1"') + expect(csvField('-1+1')).toBe('"\'-1+1"') + expect(csvField('@SUM(A1)')).toBe('"\'@SUM(A1)"') + expect(csvField('\t1+1')).toBe('"\'\t1+1"') + expect(csvField('\r1+1')).toBe('"\'\r1+1"') + }) + + it('escapes quotes in malicious inputs properly', () => { + expect(csvField('="hello"')).toBe('"\'=""hello"""') + }) +}) diff --git a/packages/web/src/lib/server/csv/export.ts b/packages/web/src/lib/server/csv/export.ts new file mode 100644 index 00000000..c302317f --- /dev/null +++ b/packages/web/src/lib/server/csv/export.ts @@ -0,0 +1,13 @@ +export function csvField(value: string | number | null | undefined) { + if (value === null || value === undefined) return '' + + if (typeof value !== 'number') { + const text = String(value) + if (/^[=+\-@\t\r]/.test(text)) { + return `"'${text.replaceAll('"', '""')}"` + } + } + + const text = String(value) + return /[",\r\n]/.test(text) ? `"${text.replaceAll('"', '""')}"` : text +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6dfd315f..14a3d6cc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -10,6 +10,7 @@ overrides: hono: ^4.12.34 js-yaml: 4.3.1 nanoid: 3.3.18 + deepmerge-ts: 8.0.0 '@auth/core': ^0.41.3 sharp: ^0.35.3 postcss: ^8.5.18 @@ -2278,8 +2279,8 @@ packages: deep-is@0.1.4: resolution: {integrity: sha512-oIPzksmTg4/MriiaYGO+okXDT7ztn/w3Eptv/+gSIdMdKsJo0u4CfYNFJPy+4SKMuCqGw2wxnA+URMg3t8a/bQ==} - deepmerge-ts@7.1.5: - resolution: {integrity: sha512-HOJkrhaYsweh+W+e74Yn7YStZOilkoPb6fycpwNLKzSPtruFs48nYis0zy5yJz1+ktUhHxoRDJ27RQAWLIJVJw==} + deepmerge-ts@8.0.0: + resolution: {integrity: sha512-ICNjaP0ML+eSdEpJYQC46XiAn/UjAdwbEl0dE8p85ZTeNDinN4Kd4+9jS4OSAuH7st6eC7rQhsqTF5zIDaUm2g==} engines: {node: '>=16.0.0'} deepmerge@4.3.1: @@ -5659,7 +5660,7 @@ snapshots: '@prisma/config@6.19.3(magicast@0.3.5)': dependencies: c12: 3.1.0(magicast@0.3.5) - deepmerge-ts: 7.1.5 + deepmerge-ts: 8.0.0 effect: 3.21.0 empathic: 2.0.0 transitivePeerDependencies: @@ -6660,7 +6661,7 @@ snapshots: deep-is@0.1.4: {} - deepmerge-ts@7.1.5: {} + deepmerge-ts@8.0.0: {} deepmerge@4.3.1: {}