Skip to content
Merged
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
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
30 changes: 23 additions & 7 deletions src/services/auditLogStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,21 +88,37 @@ 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.
*/
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
Expand Down
39 changes: 39 additions & 0 deletions tests/auditLogStore.streaming.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ──────────────────────────────────────────────────────────────
Expand Down
Loading