Prevent formula injection in CSV/XLSX exports#928
Conversation
User-controlled identity fields (names, IDs, emails, entitlement and resource metadata) were written directly into CSV and XLSX exports. Spreadsheet applications interpret a cell beginning with =, +, -, or @ as a formula, allowing an attacker who controls an upstream identity field to inject formulas (e.g. HYPERLINK exfiltration) that execute when an operator opens the report. Add a shared sanitizeCell helper that strips control characters and prefixes formula-leading values with a single quote, and apply it to every string cell in both the CSV writer and the XLSX exporter.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // single quote so the spreadsheet renders it as literal text. Embedded control | ||
| // characters — including tabs, carriage returns, and newlines that could be | ||
| // used to smuggle content or lead a formula — are stripped first. | ||
| func sanitizeCell(s string) string { |
There was a problem hiding this comment.
🟡 Suggestion: sanitizeCell is a security-sensitive function — consider adding unit tests covering key edge cases: empty string, strings starting with each trigger character (=, +, -, @), strings with embedded control characters, and strings that are already clean. This ensures regressions are caught if the function is modified later.
General PR Review: Prevent formula injection in CSV/XLSX exportsBlocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0 Review SummaryThis PR adds a Security IssuesNone found. Correctness IssuesNone found. Suggestions
Prompt for AI agents |
gontzess
left a comment
gontzess
left a comment
There was a problem hiding this comment.
1. [issue] XLSX doesn't need the quote-prefix, and it corrupts data. excelRow.Row writes values via f.SetCellValue(sheet, cell, ...). For a Go string, excelize routes that to SetCellStr, which writes a shared-string cell (type s) and removes any formula — it never creates a formula. Excel renders shared strings as literal text, so the XLSX path was not vulnerable to formula injection. Because excelize stores the value verbatim and never sets the quotePrefix style, the leading ' becomes part of the cell's data: @handle → '@handle, -5 → '-5. Net result is altered data with no security gain. Suggest dropping sanitizeCell on the XLSX path (string cells are already safe), or, if you want defensive hardening, set the cell's quotePrefix style rather than mutating the string.
2. [suggestion] No unit test for sanitizeCell. It's a pure function — a small table test would lock the behavior: each prefix (= + - @) gets quoted, control chars stripped, "\t=1+1" → "'=1+1", empty string, and a multibyte/no-op leading char.
3. [suggestion] Full-width Unicode prefixes aren't caught. The first-byte check only matches ASCII. OWASP's CSV-injection guidance also lists full-width variants (＝ ＋ － ＠, U+FF1D etc.) as test cases; a value starting with one slips through. Whether that's exploitable depends on the target app's Unicode normalization — worth a conscious decision rather than leaving it uncovered.
4. [nit] Control-char stripping removes more than needed. unicode.IsControl strips every control rune. Go's csv.Writer already quote-escapes embedded newlines/tabs, so stripping isn't required for CSV correctness — it just drops legitimate whitespace from names/descriptions. Fine as defense-in-depth, but a conscious tradeoff.
CSV approach is otherwise sound: control-char stripping covers tab/CR/LF, the = + - @ set matches OWASP, and csv.Writer prevents separator breakout.
4 participants
Summary
Add sanitization to exported CSV and XLSX files to prevent spreadsheet formula injection attacks. User-controlled data from upstream identity systems is now validated before export to prevent malicious formulas from being executed when files are opened in Excel, Google Sheets, LibreOffice, or other spreadsheet applications.
Changes
sanitizeCell()function inexport.go: Removes control characters (tabs, carriage returns, newlines) and prefixes cells starting with=,+,-, or@with a single quote to force literal text renderingcsv.go): Wrap all 12 cell values in thecsvRow.Row()method withsanitizeCell()xlsx.go): Wrap all 12 cell values in theexcelRow.Row()method withsanitizeCell()Implementation Details
The
sanitizeCell()function:strings.Map()withunicode.IsControl()=,+,-,@)This approach ensures that attacker-controllable values (e.g., user display names, resource names) cannot be weaponized as formulas when exported data is opened in spreadsheet applications.
https://claude.ai/code/session_01Jxcdh6eSR6V8JX9G6iSefs