fix(exporters): neutralize CSV formula injection (#1300) - #1302
Closed
saidai-bhuvanesh wants to merge 1 commit into
Closed
fix(exporters): neutralize CSV formula injection (#1300)#1302saidai-bhuvanesh wants to merge 1 commit into
saidai-bhuvanesh wants to merge 1 commit into
Conversation
generateBatchCSVString quoted/escaped cells but did not protect values starting with =, +, -, @, tab or CR, so a malicious field value could be executed as a spreadsheet formula on CSV open (CWE-1236). Add a sanitizeCsvCell guard that prefixes trigger-leading values with a single quote, applied via a shared csvCell helper to every dynamic cell. Add regression tests. Closes Nitya-003#1300
|
@openhands-agent is attempting to deploy a commit to the Nitya Gosain's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@saidai-bhuvanesh, please resolve the commit so that it will be merged soon ...... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The supply-chain CSV export escaped commas and quotes but did not protect cells whose values start with a spreadsheet formula trigger character (
=,+,-,@, tab, or CR). A malicious or accidental field value such as a crop type, farmer name, or stage note of=HYPERLINK("http://bad.test","Click")would be executed as a formula when the exported CSV was opened in Excel / Google Sheets / LibreOffice (CWE-1236 — CSV formula injection). This PR adds formula-injection protection to the CSV export path.Problem
generateBatchCSVStringinfrontend/src/utils/exporters.tsbuilt each cell by hand as"${value}"with only quote-escaping. It never inspected the first character of the value. Spreadsheet applications treat a leading=,+,-,@, tab, or carriage return as the start of a formula, so any user-controlled field that began with one of those characters would be evaluated on open rather than displayed as text. Every user-controlled cell in the export was affected: batch fields (cropType,farmerName,farmerAddress,origin,quantity,harvestDate,status,currentStage,certifications,description) and every supply-chain timeline field (stage,location,actor,temperature,humidity,txHash,notes).Fix
Introduce a
sanitizeCsvCell(value)guard that detects values beginning with a formula trigger character and prefixes them with a single quote ('). Spreadsheets treat a leading single quote as the literal-text marker and silently strip it on display, so the value renders identically to the user but can never be executed as a formula.sanitizeCsvCellis wrapped by a sharedcsvCell(value)helper that also performs the existing quote/escaping, and every dynamic cell ingenerateBatchCSVStringnow goes throughcsvCellinstead of being quoted by hand. Static header strings are not user-controlled and are unchanged.Behaviour preserved:
"(Jane "Farmer"→"Jane ""Farmer""").null/undefinedcells render as empty, numbers are coerced normally (0is kept, not dropped).=/@mid-string (e.g.a=b,hello @user) are not affected — only values that start with a trigger character are neutralized.Verification
frontend/src/utils/exporters.test.tswith 8 tests: trigger-character prefixing for= + - @ \t \r, benign-value pass-through, nullish/number coercion, mid-string non-triggering, embedded-quote escaping round-trip, and a payload test that asserts no cell in the generated CSV begins (after its opening quote) with a trigger character across every user-controlled field including the timeline.frontend/src/test/exporters.test.ts(3 tests) still passes.npx vitest run src/utils/exporters.test.ts→ 8/8 passing.Note on the issue's referenced paths
The issue lists
apps/web/src/lib/comparisonExport.tsandapps/web/tests/compare-pricing.test.tsx, which do not exist in this repository (the frontend lives underfrontend/, notapps/web/, and the compare page itself has no CSV export). The actual CSV export utility isfrontend/src/utils/exporters.ts(used by the batch journey export), and that is the path fixed here. The same sanitization pattern is applicable to any future CSV export added to the compare page.Changes
frontend/src/utils/exporters.ts— addsanitizeCsvCell+csvCell, route every dynamic cell throughcsvCell, exportsanitizeCsvCell.frontend/src/utils/exporters.test.ts— new regression tests (8 tests).Closes #1300