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.
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 novulnerabilities. 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 valuesWhere:
CsvEscape(src/csv_reporter.cc:35-49) wraps a field in quotes and doublesembedded quotes, but a field beginning with
=,+,-, or@is otherwiseemitted 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=csvartifacts are often shared onward — a results fileattached 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
CsvEscaperemoves the class for all of them.Suggested change: in
CsvEscape, if the field begins with any of= + - @, prefixthe 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
StrEscapeWhere:
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=jsonemit 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 aJSONDecodeErroron a developer's own machine. But any third-party tooling thatingests
benchmark_outJSON is entitled to the valid JSON the reporter's formatpromises, and the fix is local: it cannot perturb any currently well-formed output.
Suggested change: in
StrEscape, emit\u00XXfor every remaining byte below0x20 (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_outJSON supplied as a file. Shape and type mismatches currentlysurface 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 unknowntime_unityields a None multiplier that later TypeErrors(
:192); missing keys raise KeyError in partitioning; deeply nested input reachesRecursionError 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_unitis known — rejectingwith 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 internalsand 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.