Three small hardening notes from a security review: CSV formula-prefix neutralization, full C0 escaping in the JSON reporter, and type diagnostics in gbench/compare.py

BODY

Three small hardening notes from a security review of the library (static read at commit 04b5f41e, 2026-08-28). To state the conclusion first: the review found no vulnerabilities. Every string that reaches the report writers — benchmark names, counter names, labels, skip/error messages — is set by the benchmark's author, who is also the author of the native code the user already builds and runs, so on the library's own sinks there is no producer distinct from the code author, and the review closed each of the three notes below with zero delta (no security impact). They are offered purely as cheap defense-in-depth for the consumer side — results artifacts and the report-reader tools — where files can travel to readers beyond their author.

Note 1 — CSV output: neutralize leading =, +, -, @ in cell values

Where: CsvEscape (src/csv_reporter.cc:35-49) wraps a field in quotes and doubles embedded quotes, but a field beginning with =, +, -, or @ is otherwise emitted verbatim. Author-set strings reach CSV cells at the counter-name header (src/csv_reporter.cc:81-84), skip/error messages (:112-118), and report labels (:155-157).

Why harden: CSV quoting does not stop spreadsheet applications from re-interpreting a quoted cell that begins with one of those four characters as a formula (the classic spreadsheet formula/DDE class — the OWASP CSV Injection page catalogs the standard examples). Within the library's own trust model this is zero delta: anyone running a third-party benchmark binary has already executed that author's native code, which is strictly more capability than any formula payload, and current spreadsheet builds warn before evaluating such content. But --benchmark_out_format=csv artifacts are often shared onward — a results file attached to a PR, a CI export opened by a maintainer who did not run the benchmark — and those readers get no say in the benchmark's counter names. One branch in CsvEscape removes the class for all of them.

Suggested change: in CsvEscape, if the field begins with any of = + - @, prefix the cell content with a single quote ' (or another documented neutral marker). A small unit test covering the four prefixes plus an embedded quote/CR payload would pin the behavior.

Note 2 — JSON output: escape the full C0 control range in StrEscape

Where: StrEscape (src/json_reporter.cc:37-69) escapes \b \f \n \r \t \" \\ but passes every other C0 control character through raw inside JSON strings. The sinks are the same author-set strings: benchmark name (src/json_reporter.cc:253), error/skip messages (:288-291), counter keys and label (:319-342).

Why harden: RFC 8259 forbids raw U+0000–U+001F inside JSON strings, so a benchmark name or skip message containing e.g. U+0001 or U+001B makes --benchmark_out=json emit an artifact strict parsers reject. The review again closed this at zero delta — the strings are author-trusted, and the in-repo consumers (tools/gbench, tools/compare.py) go through Python's stdlib json, whose worst failure is a JSONDecodeError on a developer's own machine. But any third-party tooling that ingests benchmark_out JSON is entitled to the valid JSON the reporter's format promises, and the fix is local: it cannot perturb any currently well-formed output.

Suggested change: in StrEscape, emit \\\u00XX for every remaining byte below 0x20 (a range check alongside the existing switch). Optionally also decide and document a policy for invalid UTF-8 input bytes (currently passed through raw), even if the policy is just replacement.

Note 3 — gbench / compare.py: fail with a diagnostic instead of a traceback on malformed artifacts

Where: the report-reader path is the one place data can arrive from a third party — a benchmark_out JSON supplied as a file. Shape and type mismatches currently surface as bare exceptions deep in the formatting internals: len(bc["name"]) raises TypeError when the name is not a string (tools/gbench/report.py:82-83); string-valued times hit the {:+16.4f}-style format specifiers and raise ValueError (:425-453); an unknown time_unit yields a None multiplier that later TypeErrors (:192); missing keys raise KeyError in partitioning; deeply nested input reaches RecursionError via json.load (tools/gbench/util.py:140-154).

Why harden: this is robustness, not security, and the review closed it at zero delta — the tool runs in the reader's own process at the reader's own privilege, Python raises before anything memory-unsafe happens, and "CLI crashes on a malformed input file" is ordinary issue territory. It still bites in practice: someone comparing results attached to a PR, or produced by a different or older benchmark version, gets a stack trace instead of a reason.

Suggested change: one shape check at artifact load time — names are strings, times are numeric, the expected per-run keys are present, time_unit is known — rejecting with a message that names the offending field, e.g. a.json: run[3].name is not a string. A single check at load keeps the policy out of the formatting internals and covers compare.py and the gbench classes at once.

What this is and is not

All three notes come from one review pass and none claims a vulnerability: under the library's input model the strings involved are author-set, the review's impact analysis closed each one at zero delta, and the suggested changes are defense-in-depth for artifact consumers only. Each is small and self-contained; happy to send PRs for any or all of them.