Skip to content

Harden reporters against formula injection and invalid JSON / input shape (fixes #2299) #2300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jdymitarai wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Harden reporters against formula injection and invalid JSON / input shape (fixes #2299) #2300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 3 additions & 14 deletions src/csv_reporter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "benchmark_api_internal.h"
#include "check.h"
#include "complexity.h"
#include "string_util.h"

// File format reference: http://edoceo.com/utilitas/csv-file-format.

Expand All @@ -33,19 +34,7 @@ const std::vector<const char*> elements = {
"error_occurred", "error_message"};

std::string CsvEscape(const std::string& s) {
std::string tmp;
tmp.reserve(s.size() + 2);
for (char c : s) {
switch (c) {
case '"':
tmp += "\"\"";
break;
default:
tmp += c;
break;
}
}
return '"' + tmp + '"';
return internal::CsvEscape(s);
}
} // namespace

Expand Down Expand Up @@ -80,7 +69,7 @@ void CSVReporter::ReportRuns(const std::vector<Run>& reports) {
}
for (auto B = user_counter_names_.begin();
B != user_counter_names_.end();) {
Out << ",\"" << *B++ << "\"";
Out << "," << CsvEscape(*B++);
}
Out << "\n";

Expand Down
32 changes: 1 addition & 31 deletions src/json_reporter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,37 +35,7 @@ namespace benchmark {
namespace {

std::string StrEscape(const std::string& s) {
std::string tmp;
tmp.reserve(s.size());
for (char c : s) {
switch (c) {
case '\b':
tmp += "\\b";
break;
case '\f':
tmp += "\\f";
break;
case '\n':
tmp += "\\n";
break;
case '\r':
tmp += "\\r";
break;
case '\t':
tmp += "\\t";
break;
case '\\':
tmp += "\\\\";
break;
case '"':
tmp += "\\\"";
break;
default:
tmp += c;
break;
}
}
return tmp;
return internal::JsonStrEscape(s);
}

std::string FormatKV(std::string const& key, std::string const& value) {
Expand Down
65 changes: 65 additions & 0 deletions src/string_util.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,4 +267,69 @@ double stod(const std::string& str, size_t* pos) {
}
#endif

namespace internal {

std::string CsvEscape(const std::string& s) {
std::string tmp;
tmp.reserve(s.size() + 2);
if (!s.empty() &&
(s[0] == '=' || s[0] == '+' || s[0] == '-' || s[0] == '@')) {
tmp += '\'';
}
for (char c : s) {
switch (c) {
case '"':
tmp += "\"\"";
break;
default:
tmp += c;
break;
}
}
return '"' + tmp + '"';
}

std::string JsonStrEscape(const std::string& s) {
std::string tmp;
tmp.reserve(s.size());
for (char c : s) {
switch (c) {
case '\b':
tmp += "\\b";
break;
case '\f':
tmp += "\\f";
break;
case '\n':
tmp += "\\n";
break;
case '\r':
tmp += "\\r";
break;
case '\t':
tmp += "\\t";
break;
case '\\':
tmp += "\\\\";
break;
case '"':
tmp += "\\\"";
break;
default:
if (static_cast<unsigned char>(c) < 0x20) {
char buf[7];
snprintf(buf, sizeof(buf), "\\u%04x",
static_cast<unsigned int>(static_cast<unsigned char>(c)));
tmp += buf;
} else {
tmp += c;
}
break;
}
}
return tmp;
}

} // end namespace internal

} // end namespace benchmark
10 changes: 10 additions & 0 deletions src/string_util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ using std::stoul; // NOLINT(misc-unused-using-decls)
#endif
// NOLINTEND

namespace internal {

BENCHMARK_EXPORT
std::string CsvEscape(const std::string& s);

BENCHMARK_EXPORT
std::string JsonStrEscape(const std::string& s);

} // end namespace internal

} // end namespace benchmark

#endif // BENCHMARK_STRING_UTIL_H_
41 changes: 41 additions & 0 deletions test/reporter_list_gtest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,47 @@ TEST(ReporterListTest, CSVListsNameColumn) {
BENCHMARK_RESTORE_DEPRECATED_WARNING
}

const std::vector<BenchmarkInstance>& ListEscapedBenchmarks() {
static const std::vector<BenchmarkInstance>* const benchmarks = [] {
RegisterBenchmark("=FormulaInjection", BM_ReporterListDummy);
RegisterBenchmark("+PlusPrefix", BM_ReporterListDummy);
RegisterBenchmark("-MinusPrefix", BM_ReporterListDummy);
RegisterBenchmark("@AtPrefix", BM_ReporterListDummy);
RegisterBenchmark("BM_ANSI_\x1b[31mRed\x1b[0m", BM_ReporterListDummy);
auto* result = new std::vector<BenchmarkInstance>();
std::ostringstream err_stream;
FindBenchmarksInternal(
"(=FormulaInjection|\\+PlusPrefix|-MinusPrefix|@AtPrefix|BM_ANSI_).*",
result, &err_stream);
return result;
}();
return *benchmarks;
}

TEST(ReporterListTest, CSVEscapesFormulaPrefixes) {
BENCHMARK_DISABLE_DEPRECATED_WARNING
CSVReporter reporter;
std::ostringstream out;
reporter.SetOutputStream(&out);
reporter.List(ListEscapedBenchmarks());
std::string s = out.str();
EXPECT_NE(s.find("\"'=FormulaInjection\"\n"), std::string::npos);
EXPECT_NE(s.find("\"'+PlusPrefix\"\n"), std::string::npos);
EXPECT_NE(s.find("\"'-MinusPrefix\"\n"), std::string::npos);
EXPECT_NE(s.find("\"'@AtPrefix\"\n"), std::string::npos);
BENCHMARK_RESTORE_DEPRECATED_WARNING
}

TEST(ReporterListTest, JSONEscapesC0ControlChars) {
JSONReporter reporter;
std::ostringstream out;
reporter.SetOutputStream(&out);
reporter.List(ListEscapedBenchmarks());
std::string s = out.str();
EXPECT_NE(s.find("\"name\": \"BM_ANSI_\\u001b[31mRed\\u001b[0m\""),
std::string::npos);
}

} // namespace
} // namespace internal
} // namespace benchmark
58 changes: 58 additions & 0 deletions test/string_util_gtest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,4 +200,62 @@ TEST_P(HumanReadableFixture, HumanReadableNumber) {
ASSERT_THAT(str, ::testing::MatchesRegex(std::get<2>(GetParam())));
}

TEST(StringUtilTest, CsvEscape) {
// Empty string
EXPECT_EQ(benchmark::internal::CsvEscape(""), "\"\"");

// Standard safe strings
EXPECT_EQ(benchmark::internal::CsvEscape("BM_basic"), "\"BM_basic\"");
EXPECT_EQ(benchmark::internal::CsvEscape("hello world"), "\"hello world\"");

// Embedded double-quotes
EXPECT_EQ(benchmark::internal::CsvEscape("foo\"bar"), "\"foo\"\"bar\"");

// Formula prefixes neutralized with leading single-quote
EXPECT_EQ(benchmark::internal::CsvEscape("=1+1"), "\"'=1+1\"");
EXPECT_EQ(benchmark::internal::CsvEscape("=SUM(A1:A2)"), "\"'=SUM(A1:A2)\"");
EXPECT_EQ(benchmark::internal::CsvEscape("+cmd"), "\"'+cmd\"");
EXPECT_EQ(benchmark::internal::CsvEscape("-10"), "\"'-10\"");
EXPECT_EQ(benchmark::internal::CsvEscape("@admin"), "\"'@admin\"");

// Formula prefix with embedded quotes and carriage return
EXPECT_EQ(benchmark::internal::CsvEscape("=cmd|' /C calc'!A0"),
"\"'=cmd|' /C calc'!A0\"");
EXPECT_EQ(benchmark::internal::CsvEscape("=foo\r\n\"bar\""),
"\"'=foo\r\n\"\"bar\"\"\"");
}

TEST(StringUtilTest, JsonStrEscape) {
// Empty string
EXPECT_EQ(benchmark::internal::JsonStrEscape(""), "");

// Safe strings
EXPECT_EQ(benchmark::internal::JsonStrEscape("hello world"), "hello world");

// Standard short escapes
EXPECT_EQ(benchmark::internal::JsonStrEscape("\"quoted\\backslash\""),
"\\\"quoted\\\\backslash\\\"");
EXPECT_EQ(benchmark::internal::JsonStrEscape(
"tab\tnewline\nreturn\rbackspace\bformfeed\f"),
"tab\\tnewline\\nreturn\\rbackspace\\bformfeed\\f");

// C0 control characters (RFC 8259 Section 7: 0x00 to 0x1F)
EXPECT_EQ(benchmark::internal::JsonStrEscape(std::string("\x00", 1)),
"\\u0000");
EXPECT_EQ(benchmark::internal::JsonStrEscape("\x01"), "\\u0001");
EXPECT_EQ(benchmark::internal::JsonStrEscape("\x07"), "\\u0007");
EXPECT_EQ(benchmark::internal::JsonStrEscape("\x0b"), "\\u000b");
EXPECT_EQ(benchmark::internal::JsonStrEscape("\x1b"), "\\u001b");
EXPECT_EQ(benchmark::internal::JsonStrEscape("\x1f"), "\\u001f");

// ANSI escape sequences in skip/error messages
EXPECT_EQ(benchmark::internal::JsonStrEscape("\x1b[31mred text\x1b[0m"),
"\\u001b[31mred text\\u001b[0m");

// Printable ASCII and UTF-8 multi-byte characters remain unescaped
EXPECT_EQ(benchmark::internal::JsonStrEscape(" 0123456789!@#$%^&*()~`"),
" 0123456789!@#$%^&*()~`");
EXPECT_EQ(benchmark::internal::JsonStrEscape("中文测试"), "中文测试");
}

} // end namespace
5 changes: 4 additions & 1 deletion tools/gbench/report.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1489,7 +1489,10 @@ def load_result():
cls.json = load_result()

def test_json_diff_report_pretty_printing(self):
import util
try:
from gbench import util
except ImportError:
import util

expected_names = [
"99 family 0 instance 0 repetition 0",
Expand Down
Loading