diff --git a/README.md b/README.md index fb45381e..eaf670f4 100644 --- a/README.md +++ b/README.md @@ -1285,8 +1285,9 @@ Both ends of the pipeline attach `error` listeners. If the database stream or th Every CSV field is processed by `escapeCsvField()` in `src/services/auditLogStore.js`: -1. **Leading-character neutralisation** — cells beginning with `=`, `+`, `-`, `@`, TAB, or CR are prefixed with a single quote (`'`). This prevents spreadsheet software (Excel, LibreOffice Calc, Google Sheets) from interpreting the cell as a formula or a DDE command. -2. **RFC 4180 quoting** — fields containing commas, double-quotes, or newlines are wrapped in double-quotes; embedded double-quotes are doubled (`"` → `""`). +1. **Leading-whitespace normalisation** — the field is checked after stripping leading whitespace (`trimStart()`), so values like ` =HYPERLINK(...)` or `\t=cmd` are caught even when the dangerous character is not in position 0. +2. **Leading-character neutralisation** — cells whose first non-whitespace character is `=`, `+`, `-`, `@`, `|`, TAB, or CR are prefixed with a single quote (`'`). This covers the full OWASP CSV Injection list and prevents spreadsheet software (Excel, LibreOffice Calc, Google Sheets) from interpreting the cell as a formula or DDE command. +3. **RFC 4180 quoting** — fields containing commas, double-quotes, or newlines are wrapped in double-quotes; embedded double-quotes are doubled (`"` → `""`). #### Tenant isolation diff --git a/src/services/auditLogStore.js b/src/services/auditLogStore.js index b8aaaa61..d738ae1e 100644 --- a/src/services/auditLogStore.js +++ b/src/services/auditLogStore.js @@ -88,10 +88,25 @@ async function appendAuditEvent(event, options = {}) { } /** - * Escapes a single CSV field value to prevent formula-injection attacks - * (cells beginning with =, +, -, @, TAB, or CR are prefixed with a single - * quote so that spreadsheet software treats them as plain text) and to - * conform to RFC 4180 quoting rules. + * Escapes a single CSV field value to prevent formula-injection attacks and + * to conform to RFC 4180 quoting rules. + * + * Formula-injection neutralisation (OWASP CSV Injection guidance): + * - Leading whitespace is stripped before the dangerous-character check so + * that values like " =HYPERLINK(...)" or "\t=cmd" are not overlooked. + * - Any field whose leading non-whitespace character is one of + * = + - @ | \t \r is prefixed with a single quote (') on the *original* + * string so that spreadsheet software treats the cell as plain text. + * + * Covered cases: + * "=SUM(...)" → "'=SUM(...)" + * " =HYPERLINK(...)" → "' =HYPERLINK(...)" (leading space + =) + * "\t=cmd" → "'\t=cmd" (leading tab + =) + * "+cmd|..." → "'+cmd|..." + * "-2+3" → "'-2+3" + * "@SUM(1)" → "'@SUM(1)" + * "|calc.exe" → "'|calc.exe" + * "\r=evil" → "'\r=evil" (leading CR) * * @param {*} val - Raw field value (any type; will be coerced to string). * @returns {string} Safely-escaped CSV field, quoted when necessary. @@ -99,10 +114,11 @@ async function appendAuditEvent(event, options = {}) { function escapeCsvField(val) { const str = val == null ? '' : String(val); - // Neutralise formula-injection: prefix dangerous leading characters. - // Covers the OWASP-recommended set: = + - @ \t \r + // Neutralise formula-injection: inspect the first non-whitespace character. + // Covers the OWASP-recommended set: = + - @ | \t \r + const trimmedFirst = str.trimStart(); const injectionSafe = - str.length > 0 && /^[=+\-@\t\r]/.test(str) ? `'${str}` : str; + trimmedFirst.length > 0 && /^[=+\-@|\t\r]/.test(trimmedFirst) ? `'${str}` : str; // RFC 4180 quoting: wrap in double-quotes if the value contains a // comma, double-quote, or newline; escape embedded double-quotes by diff --git a/tests/auditLogStore.streaming.test.js b/tests/auditLogStore.streaming.test.js index 8e5379d7..63aba2e3 100644 --- a/tests/auditLogStore.streaming.test.js +++ b/tests/auditLogStore.streaming.test.js @@ -113,6 +113,45 @@ describe('escapeCsvField', () => { // Numbers are coerced to string; "-100" starts with - so it gets prefixed expect(escapeCsvField(-100)).toBe("'-100"); }); + + // ── Leading-whitespace regression (issue fix) ───────────────────────────── + + it('neutralizes a field with a leading space then "=" (whitespace bypass)', () => { + expect(escapeCsvField(' =HYPERLINK("http://evil.com")')).toBe("' =HYPERLINK(\"http://evil.com\")"); + }); + + it('neutralizes a field with multiple leading spaces then "+"', () => { + expect(escapeCsvField(' +cmd')).toBe("' +cmd"); + }); + + it('neutralizes a field with a leading tab then "=" (tab-prefix bypass)', () => { + expect(escapeCsvField('\t=MALICIOUS()')).toBe("'\t=MALICIOUS()"); + }); + + it('neutralizes a field with leading space then "-"', () => { + expect(escapeCsvField(' -2+3')).toBe("' -2+3"); + }); + + it('neutralizes a field with leading space then "@"', () => { + expect(escapeCsvField(' @SUM(1)')).toBe("' @SUM(1)"); + }); + + // ── Pipe (|) prefix (OWASP DDE) ────────────────────────────────────────── + + it('prefixes | with a single quote (DDE/pipe injection)', () => { + expect(escapeCsvField('|calc.exe')).toBe("'|calc.exe"); + }); + + it('neutralizes a field with leading space then "|"', () => { + expect(escapeCsvField(' |calc.exe')).toBe("' |calc.exe"); + }); + + // ── Whitespace-only and all-whitespace edge cases ───────────────────────── + + it('does not prefix whitespace-only values', () => { + expect(escapeCsvField(' ')).toBe(' '); + expect(escapeCsvField('\t')).toBe('\t'); + }); }); // ── rowToCsvLine ──────────────────────────────────────────────────────────────