From f1f148581780074f8a590a051e9de9044d85ffba Mon Sep 17 00:00:00 2001 From: devtejasx Date: Tue, 14 Jul 2026 03:21:14 +0530 Subject: [PATCH] Fix heap-use-after-free in CreateDefaultDisplayReporter (#2240) CreateDefaultDisplayReporter() cached its reporter in a function-local static and returned that pointer. But the returned pointer is owned by the caller: RunSpecifiedBenchmarks() wraps it in a std::unique_ptr that frees it when the call returns. The first call therefore freed the static reporter, and the next call returned the same, now-dangling pointer, which was dereferenced when setting up the output streams -- a heap-use-after-free reachable by simply calling RunSpecifiedBenchmarks more than once (reported via fuzzing on the no-match filter path). Return a fresh, caller-owned reporter on every call. Add default_display_reporter_test, which holds two reporters from two calls alive at once and asserts they are distinct objects; it fails on the old static implementation and passes with this fix. Verified locally with g++ 15.2; benchmark_test and memory_manager_test still pass. Signed-off-by: devtejasx --- include/benchmark/benchmark_api.h | 2 ++ src/benchmark.cc | 10 +++++----- test/CMakeLists.txt | 3 +++ test/default_display_reporter_test.cc | 28 +++++++++++++++++++++++++++ 4 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 test/default_display_reporter_test.cc diff --git a/include/benchmark/benchmark_api.h b/include/benchmark/benchmark_api.h index 6d98aba581..5594451a89 100644 --- a/include/benchmark/benchmark_api.h +++ b/include/benchmark/benchmark_api.h @@ -54,6 +54,8 @@ BENCHMARK_EXPORT void SetBenchmarkFilter(std::string value); BENCHMARK_EXPORT int32_t GetBenchmarkVerbosity(); +// Creates the display reporter selected by --benchmark_format. Returns a +// fresh instance on every call; the caller owns and frees it. BENCHMARK_EXPORT BenchmarkReporter* CreateDefaultDisplayReporter(); BENCHMARK_EXPORT size_t RunSpecifiedBenchmarks(); diff --git a/src/benchmark.cc b/src/benchmark.cc index 7217fc4ed2..f0f1fc4955 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc @@ -586,11 +586,11 @@ ConsoleReporter::OutputOptions GetOutputOptions(bool force_no_color) { } // end namespace internal BenchmarkReporter* CreateDefaultDisplayReporter() { - static auto* default_display_reporter = - internal::CreateReporter(FLAGS_benchmark_format, - internal::GetOutputOptions()) - .release(); - return default_display_reporter; + // Each caller owns the returned reporter, so never cache it in a static: + // the first caller would free it and later calls would dangle (#2240). + return internal::CreateReporter(FLAGS_benchmark_format, + internal::GetOutputOptions()) + .release(); } size_t RunSpecifiedBenchmarks() { diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 280113ccea..2e2395aa8b 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -213,6 +213,9 @@ benchmark_add_test(NAME user_counters_thousands_test COMMAND user_counters_thous compile_output_test(memory_manager_test) benchmark_add_test(NAME memory_manager_test COMMAND memory_manager_test --benchmark_min_time=0.01s) +compile_benchmark_test(default_display_reporter_test) +benchmark_add_test(NAME default_display_reporter_test COMMAND default_display_reporter_test) + compile_output_test(profiler_manager_test) benchmark_add_test(NAME profiler_manager_test COMMAND profiler_manager_test --benchmark_min_time=0.01s) diff --git a/test/default_display_reporter_test.cc b/test/default_display_reporter_test.cc new file mode 100644 index 0000000000..f4891f9386 --- /dev/null +++ b/test/default_display_reporter_test.cc @@ -0,0 +1,28 @@ +// Regression test for https://github.com/google/benchmark/issues/2240: +// CreateDefaultDisplayReporter() cached its result in a function-local static +// even though each caller owns (and frees) the returned pointer, so the +// second call returned a dangling pointer. It must hand out a fresh reporter +// on every call. + +#include +#include + +#include "benchmark/benchmark.h" + +int main(int argc, char** argv) { + benchmark::MaybeReenterWithoutASLR(argc, argv); + benchmark::Initialize(&argc, argv); + + std::unique_ptr first( + benchmark::CreateDefaultDisplayReporter()); + std::unique_ptr second( + benchmark::CreateDefaultDisplayReporter()); + + if (first == nullptr || second == nullptr || first.get() == second.get()) { + std::cerr << "CreateDefaultDisplayReporter must return a fresh, " + "caller-owned reporter on every call\n"; + return 1; + } + + return 0; +}