code/cl8y-dex-terraclassic

Fork 0

Security: CSV export has no formula injection prevention or documented risk acceptance [SEC-F12] #432

New issue

Closed

opened 2026-06-29 15:33:11 +00:00 by totdking · 12 comments

totdking commented

2026-06-29 15:33:11 +00:00

(Migrated from gitlab.com)

No description provided.

totdking commented

2026-06-29 15:34:33 +00:00

(Migrated from gitlab.com)

Summary

indexer/src/api/text_csv.rs implements RFC 4180-compliant CSV escaping via csv_escape_cell. The function quotes cells containing a comma, double-quote, newline, or leading/trailing space. It does not check for or escape leading =, +, -, or @ characters, which spreadsheet clients (Excel, LibreOffice, Google Sheets) interpret as formula prefixes. The only CSV test (get_trader_trades_csv_returns_text_csv in indexer/tests/api_traders.rs lines 165-186) checks content-type and header row field names only. No test asserts formula injection safety. No inline comment provides risk acceptance for the controlled-field argument.

What Was Checked

indexer/src/api/text_csv.rs lines 5-17: csv_escape_cell checks comma, double-quote, newline, CR, leading/trailing space. No check for =, +, -, @ prefix. Confirmed by full file read.
Exported fields reviewed across trader_swaps_csv, trader_limit_fills_csv, trader_limit_cancellations_csv: numeric IDs (i64), terra1... addresses, hex tx hashes, ISO-8601 timestamps, denom strings (uluna, CW20 contract addresses), side enum (bid/ask). None are free-text user inputs; none start with formula characters under normal chain operation.
indexer/tests/api_traders.rs lines 165-186: CSV test checks content-type: text/csv and that body contains header field names (id,pair_address,block_height, tx_hash). No formula injection test present. Confirmed by direct read.
No comment in text_csv.rs or api_traders.rs documents that fields are controlled and formula injection risk is accepted.

Expected (per checklist)

Either: (a) a test asserts that a cell starting with = is escaped or prefixed to neutralize formula interpretation, or (b) a documented comment states that all exported fields are controlled numeric/address/hash values that cannot originate from user-supplied free text, and no formula injection prefix is therefore required.

Actual

No formula injection prefix prevention is implemented. No test asserts safety. No documented risk acceptance exists. The fields are controlled in practice (low risk), but the checklist requires one of the two verification forms and neither is present.

Evidence

indexer/src/api/text_csv.rs line 6: must_quote condition covers comma, quote, newline, CR, space but not starts_with('='), starts_with('+'), starts_with('-'), or starts_with('@')
indexer/tests/api_traders.rs lines 165-186: CSV test asserts content-type and header row only, no formula injection assertion
No risk-acceptance comment found in text_csv.rs

Suggested Fix

Add a comment to csv_escape_cell in text_csv.rs stating that all exported fields originate from validated chain data (numeric IDs, terra1 addresses, hex hashes, ISO timestamps, controlled enums) and cannot begin with a formula character. Alternatively, add a unit test that constructs a TradeResponse with offer_asset set to "=HYPERLINK(...)" and asserts the CSV cell does not start with = (or that the cell is quoted in a way that neutralizes it).

Verification Checklist

text_csv.rs contains a comment documenting that exported fields are controlled and cannot originate from user-supplied free text
Or: a test asserts formula-injection safety for leading =, +, -, @ values in a CSV cell
The chosen approach (comment or test) is visible in the source file

Labels

security, pre-launch

Cc: @PlasticDigits

### Summary `indexer/src/api/text_csv.rs` implements RFC 4180-compliant CSV escaping via `csv_escape_cell`. The function quotes cells containing a comma, double-quote, newline, or leading/trailing space. It does not check for or escape leading `=`, `+`, `-`, or `@` characters, which spreadsheet clients (Excel, LibreOffice, Google Sheets) interpret as formula prefixes. The only CSV test (`get_trader_trades_csv_returns_text_csv` in `indexer/tests/api_traders.rs` lines 165-186) checks content-type and header row field names only. No test asserts formula injection safety. No inline comment provides risk acceptance for the controlled-field argument. --- ### What Was Checked - `indexer/src/api/text_csv.rs` lines 5-17: `csv_escape_cell` checks comma, double-quote, newline, CR, leading/trailing space. No check for `=`, `+`, `-`, `@` prefix. Confirmed by full file read. - Exported fields reviewed across `trader_swaps_csv`, `trader_limit_fills_csv`, `trader_limit_cancellations_csv`: numeric IDs (`i64`), `terra1...` addresses, hex tx hashes, ISO-8601 timestamps, denom strings (`uluna`, CW20 contract addresses), `side` enum (`bid`/`ask`). None are free-text user inputs; none start with formula characters under normal chain operation. - `indexer/tests/api_traders.rs` lines 165-186: CSV test checks `content-type: text/csv` and that body contains header field names (`id,pair_address,block_height`, `tx_hash`). No formula injection test present. Confirmed by direct read. - No comment in `text_csv.rs` or `api_traders.rs` documents that fields are controlled and formula injection risk is accepted. --- ### Expected (per checklist) Either: (a) a test asserts that a cell starting with `=` is escaped or prefixed to neutralize formula interpretation, or (b) a documented comment states that all exported fields are controlled numeric/address/hash values that cannot originate from user-supplied free text, and no formula injection prefix is therefore required. --- ### Actual No formula injection prefix prevention is implemented. No test asserts safety. No documented risk acceptance exists. The fields are controlled in practice (low risk), but the checklist requires one of the two verification forms and neither is present. --- ### Evidence - `indexer/src/api/text_csv.rs` line 6: `must_quote` condition covers comma, quote, newline, CR, space but not `starts_with('=')`, `starts_with('+')`, `starts_with('-')`, or `starts_with('@')` - `indexer/tests/api_traders.rs` lines 165-186: CSV test asserts content-type and header row only, no formula injection assertion - No risk-acceptance comment found in `text_csv.rs` --- ### Suggested Fix Add a comment to `csv_escape_cell` in `text_csv.rs` stating that all exported fields originate from validated chain data (numeric IDs, terra1 addresses, hex hashes, ISO timestamps, controlled enums) and cannot begin with a formula character. Alternatively, add a unit test that constructs a `TradeResponse` with `offer_asset` set to `"=HYPERLINK(...)"` and asserts the CSV cell does not start with `=` (or that the cell is quoted in a way that neutralizes it). --- ### Verification Checklist - [ ] `text_csv.rs` contains a comment documenting that exported fields are controlled and cannot originate from user-supplied free text - [ ] Or: a test asserts formula-injection safety for leading `=`, `+`, `-`, `@` values in a CSV cell - [ ] The chosen approach (comment or test) is visible in the source file --- ### Labels `security`, `pre-launch` Cc: @PlasticDigits

PlasticDigits commented

2026-06-29 15:36:36 +00:00

(Migrated from gitlab.com)

Option (a) accepted

totdking commented

2026-06-29 15:39:07 +00:00

(Migrated from gitlab.com)

mentioned in issue #381

PlasticDigits commented

2026-06-29 15:41:48 +00:00

(Migrated from gitlab.com)

mentioned in commit c14c989fa1

mentioned in commit c14c989fa192542e89ed25cbd7f24c07c04ed70a

PlasticDigits commented

2026-06-29 15:41:58 +00:00

(Migrated from gitlab.com)

mentioned in merge request !960

PlasticDigits commented

2026-06-29 15:49:35 +00:00

(Migrated from gitlab.com)

mentioned in commit 6afbcb194f

mentioned in commit 6afbcb194f92e57a5a96c37464ae40adf43f003f

PlasticDigits commented

2026-06-29 15:52:30 +00:00

(Migrated from gitlab.com)

Verification — #432 SEC-F12 (CSV formula injection)

Result: PASS — Option (a) (formula-injection tests) is implemented on main (merge 6afbcb19, commit c14c989f).

Acceptance checklist

Criterion	Result	How verified
`text_csv.rs` documents controlled fields or formula-injection test	PASS (test path)	PlasticDigits accepted option (a). `indexer/src/api/text_csv.rs` lines 5–6 document prefix neutralization; lines 118–182 add four unit tests.
Test asserts safety for leading `=`, `+`, `-`, `@`	PASS	`cargo test --lib text_csv::tests` — 4/4 passed: `csv_escape_cell_neutralizes_bare_formula_prefix`, `csv_escape_cell_neutralizes_formula_prefixes` (all four chars), `csv_escape_cell_neutralizes_formula_prefix_with_commas`, `trader_swaps_csv_neutralizes_formula_in_offer_asset`.
Chosen approach visible in source	PASS	`csv_escape_cell` prefixes `=`, `+`, `-`, `@` with `'` before RFC 4180 quoting (lines 7–12).

Implementation summary

Neutralization: csv_escape_cell prepends ' when a cell starts with =, +, -, or @.
Coverage: Unit tests cover bare prefixes, all four formula chars, comma-containing payloads, and end-to-end trader_swaps_csv with offer_asset = "=HYPERLINK(...)".
Docs: docs/indexer-invariants.md and skills/AGENTS_FRONTEND_ORDER_HISTORY.md cross-link SEC-F12 / #432.

Existing integration test (unchanged, still valid)

get_trader_trades_csv_returns_text_csv in indexer/tests/api_traders.rs continues to assert text/csv content-type and header row; formula safety is covered by the dedicated unit tests above.

No repo changes required during this verification pass.

## Verification — #432 SEC-F12 (CSV formula injection) **Result: PASS** — Option (a) (formula-injection tests) is implemented on `main` (merge `6afbcb19`, commit `c14c989f`). ### Acceptance checklist | Criterion | Result | How verified | |-----------|--------|--------------| | `text_csv.rs` documents controlled fields **or** formula-injection test | **PASS** (test path) | PlasticDigits accepted option (a). `indexer/src/api/text_csv.rs` lines 5–6 document prefix neutralization; lines 118–182 add four unit tests. | | Test asserts safety for leading `=`, `+`, `-`, `@` | **PASS** | `cargo test --lib text_csv::tests` — 4/4 passed: `csv_escape_cell_neutralizes_bare_formula_prefix`, `csv_escape_cell_neutralizes_formula_prefixes` (all four chars), `csv_escape_cell_neutralizes_formula_prefix_with_commas`, `trader_swaps_csv_neutralizes_formula_in_offer_asset`. | | Chosen approach visible in source | **PASS** | `csv_escape_cell` prefixes `=`, `+`, `-`, `@` with `'` before RFC 4180 quoting (lines 7–12). | ### Implementation summary - **Neutralization:** `csv_escape_cell` prepends `'` when a cell starts with `=`, `+`, `-`, or `@`. - **Coverage:** Unit tests cover bare prefixes, all four formula chars, comma-containing payloads, and end-to-end `trader_swaps_csv` with `offer_asset = "=HYPERLINK(...)"`. - **Docs:** `docs/indexer-invariants.md` and `skills/AGENTS_FRONTEND_ORDER_HISTORY.md` cross-link SEC-F12 / #432. ### Existing integration test (unchanged, still valid) `get_trader_trades_csv_returns_text_csv` in `indexer/tests/api_traders.rs` continues to assert `text/csv` content-type and header row; formula safety is covered by the dedicated unit tests above. No repo changes required during this verification pass.

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-06-29 15:52:33 +00:00

Brouie commented

2026-06-30 03:15:16 +00:00

(Migrated from gitlab.com)

mentioned in issue #337