fix(format): escape and formula-guard the CSV filename column - #6
Conversation
write_csv writes every field with a bare write!, applying no escaping of
any kind. The size, hash and entropy columns are machine-generated and
safe by construction, but the filename is chosen by whoever wrote the
file.
Two defects, both in that one column:
- A file named "=cmd|..." executes as a formula when the examiner opens
the CSV. Run over a directory the path renders relative, so the
lead-in lands at the start of the cell.
- A filename may legally contain a comma or a quote on every platform
this tool runs on. Emitted raw, the comma acts as a delimiter and
silently shifts every later column.
These two tests fail on the current implementation:
unguarded formula filename "=cmd|'-c calc'!A1.txt" in row:
11,d74981efa70a0c880b8d8c1985d075dbcbf679b99a5f9914e5aaf96b831a9e24,=cmd|'-c calc'!A1.txt
comma-bearing filename must be quoted as "/evidence/report,final.txt":
11,d74981efa70a0c880b8d8c1985d075dbcbf679b99a5f9914e5aaf96b831a9e24,/evidence/report,final.txt
The formula test uses a relative path deliberately. An absolute path
begins with a separator, which masks the lead-in and would make the test
pass against the unfixed code.
RED commit: tests only, no implementation change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Route the filename through jsonguard::csv_field, the fleet's shared sanitizer. It applies RFC 4180 quoting and neutralizes a leading =, +, - or @ with an apostrophe. The filename is the only attacker-chosen column here; size, hash and entropy are machine-generated and safe by construction, so they keep their bare write!. This closes both defects at once: a comma or quote in a filename can no longer shift the columns, and a file named "=cmd|..." no longer executes when the examiner opens the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
The GREEN commit added a dependency and no supply-chain record for it, so CI
caught what the local gate did not:
Vetting Failed!
1 unvetted dependencies:
jsonguard:0.2.4 missing ["safe-to-deploy"]
cargo-vet named the right mechanism itself:
NOTE: this project trusts Albert Hui (h4x0r) - consider cargo vet trust jsonguard
`jsonguard` is ours and comes from crates.io, which is ADR-0018 case 2. Verified
against the registry first — one owner, and every published version from 0.1.0
through 0.2.4 is theirs:
owners: h4x0r | Albert Hui | user
0.2.4 -> h4x0r 2026-07-25
0.2.3 -> h4x0r 2026-06-14
0.2.2 .. 0.1.0 -> h4x0r 2026-05-21
$ cargo vet trust jsonguard h4x0r --criteria safe-to-deploy
Both records are load-bearing. Dropping the `imports.lock` publisher entry and
keeping only the audit reproduces the original failure exactly, so it is not
regenerable cache here:
Vetting Failed!
1 unvetted dependencies:
jsonguard:0.2.4 missing ["safe-to-deploy"]
With both:
Vetting Succeeded (170 fully audited, 5 partially audited, 629 exempted)
Cargo.lock is deliberately untouched. This branch still carries the stale lock
it inherited from main; re-syncing it belongs to the lock PR, not here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Correction to the jsonguard section above — quoting is not formula-guardingThe section above says the CR/LF hole is closed. That is true only for record-splitting, and stating it that way understates a real defect. Reconciled against the source, the precise picture: // text.rs:95-98
let guarded = match cleaned.chars().next() {
Some('=' | '+' | '-' | '@') => alloc::format!("'{cleaned}"),
_ => cleaned,
};The formula guard fires only on character 0. Anything that occupies position 0 without being stripped defeats it. There are two protections and they are independent — a field can get one and not the other:
So my original probe measured the quoting correctly and drew too narrow a conclusion from it. CR and LF do not split the record — and they also do not get formula-guarded. Whether a given spreadsheet strips a leading newline or space and then evaluates the cell is untested here; the mechanism is what is being reported, not a demonstrated exploit. The leading-space row is the worst case: it is the only input in the table that receives neither protection. None of this is introduced by this PR, and And the more serious one, restated plainlyOf the two residual gaps, the control-character stripping is the one that matters most for a forensic tool, and it is worth separating from the injection story entirely:
Tabs and other control characters are legal in filenames on Linux and NTFS. A forensic tool that reports a filename differing from the one on disk, presented as an observation, is a more serious failure than the injection vector this function exists to prevent: the injection is a hazard to the examiner's spreadsheet, this is a defect in the evidence itself. It belongs upstream in |
The lock pinned jsonguard 0.2.4, whose formula guard fires only on character 0. A value leading with whitespace or an invisible format character therefore reached the cell unguarded -- " =cmd" got neither quoting nor an apostrophe, and the zero-width family (U+200B, U+200C, U+200D, U+2060, U+FEFF, U+180E, U+00AD) behaved the same. 0.2.5 keys the guard on the first VISIBLE character, skipping Unicode whitespace and the whole General_Category Cf range. The requirement was already jsonguard = "0.2", a caret that admits 0.2.5, so this is a lock refresh with no manifest change. Full suite passes unchanged -- no expected-CSV fixture shifted, which was the risk worth checking: values leading with whitespace now receive an apostrophe they previously did not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Correction: the
|
`[policy.ewf] audit-as-crates-io = false` asserts ewf is a first-party crate that needs no audit. It is not: ewf resolves as `source = "registry+https://github.com/rust-lang/crates.io-index"`, both before and after the jsonguard bump. "ewf is ours" is true of the project; it was not true of this artifact's provenance, so the entry waived auditing for a real crates.io download. ADR-0018 has a mechanism for ours-but-consumed-from-crates.io, and it is the second one: `cargo vet trust`. Switching to it also retires the now-redundant `[[exemptions.ewf]]`, so two overlapping suppressions become one record bound to a publisher identity that cargo-vet checks against crates.io -- setting the id to a wrong value makes vet report `ewf:0.2.3 missing`, so the record is load-bearing rather than decorative. Also refreshes imports.lock, which cached jsonguard's publisher record only up to 0.2.4. CI runs `cargo vet --locked`, which may not update that cache, so vet could not establish who published 0.2.5 and the existing `[[trusted.jsonguard]]` rule had nothing to match. That is why this failed while the trust entry looked correct.
write_csv(src/format/csv.rs:6) wrote every field with a barewrite!— no quoting, no formula guard. Size, hash and entropy are machine-generated and safe by construction. Thefilenamecolumn is not: it is chosen by whoever wrote the file.Two defects in that one column, both closed here.
RED —
fb670e2(tests only)Verified independently against unfixed
write_csv, on a detached worktree atorigin/mainwith only the test commit applied:The formula test uses a relative path deliberately. An absolute path begins with a separator, which pushes the lead-in off position 0 and would make the test pass against the unfixed code. Run over a directory the path renders relative, so the lead-in really does land at the start of the cell.
GREEN —
c0cc534Routes the filename through
jsonguard::csv_field, the fleet's shared sanitizer, rather than hand-rolling an escaper. Only the filename column is routed; the machine-generated columns keep their barewrite!.What
jsonguard0.2.4 actually does — measured, not assumedjsonguard = "0.2"resolves to 0.2.4, the latest published (2026-07-25). Probed directly rather than taken on trust:report,final.txt"report,final.txt"report"final.txt"report""final.txt"=cmd|'-c calc'!A1.txt'=cmd|'-c calc'!A1.txt+cmd/-cmd/@cmd'+cmd/'-cmd/'@cmda\nb.txt"a\nb.txt"a\rb.txt"a\rb.txt"證據.txt證據.txtCorrection worth recording: the published 0.2.4 does not have an open CR/LF record-splitting hole.
text.rs:85-91explicitly preserves\nand\rso they trigger quoting, andneeds_csv_quoting(text.rs:79) matches on,"\n\r. CR and LF are quoted, so they cannot break the record structure. I have not inspected jsonguard PR #5, so I cannot say what it does change — only that this particular bypass is closed in what we actually resolve to.Two residual gaps in 0.2.4, neither introduced here
1. The formula guard only fires on character 0. A leading space slips past:
csv_fieldguards only whencleaned.chars().next()is one of= + - @. Any character that is neither stripped nor a lead-in shifts the payload off position 0 and the guard does not fire. Whether a given spreadsheet then trims the leading space and evaluates the cell is untested here — flagging the mechanism, not claiming a working exploit.2. Control characters are silently deleted, and
lossydoes not say so.is_display_unsafe(text.rs:19-29) filters outU+0000..=U+001F,U+007F,U+0080..=U+009Fand the bidi controls. TheGuarded.lossyflag is set only byas_utf8_lossy()— invalid UTF-8 — so this stripping is invisible in the return value.For a forensic tool that is worth naming: the CSV can report a filename that differs from the filename on disk, with nothing signalling the difference. It matters most on Linux and NTFS, where control characters in names are legal. This is a
jsonguardchange, not a blazehash one, and it is strictly better than the current state of emitting the raw byte sequence unescaped — so it is recorded here rather than fixed here.write_csvalso currently discardsGuarded.lossyentirely. Surfacing it (a warning, or a column) is a follow-up.Note for whoever merges second
This branch's
Cargo.lockaddsjsonguardon top of the stale lock. #5 re-syncs that lock. Whichever lands second will need itsCargo.lockregenerated — a re-resolution, no version changes.Provenance
The two commits were authored on an existing local branch earlier in this session. I did not re-write them: their RED is sharper than the one I was briefed to write (it covers the comma-shifts-columns case as well as the formula lead-in, and it is deliberate about the relative path). I verified both independently — re-running the RED against unfixed code to confirm it genuinely fails, then the GREEN — and probed
jsonguard0.2.4's real behaviour rather than restating the caveat I was handed.Gates on this branch:
cargo build,cargo test,cargo clippy --all-targets -- -D warnings,cargo fmt --checkall pass.cargo deny checkfails on RUSTSEC-2026-0222 (wasmtime25.0.3 viayara-x0.9.0) — pre-existing onmain, unrelated, detailed in #5.🤖 Generated with Claude Code