Fix use-after-free in CreateDefaultDisplayReporter (#2240)

src/benchmark.cc

@@ -586,11 +586,14 @@ 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;
+  // Ownership of the returned reporter is transferred to the caller (note the
+  // `.release()` below and the `unique_ptr` in `RunSpecifiedBenchmarks`).
+  // Do NOT cache this in a `static`: the caller deletes the reporter, which
+  // would leave a cached pointer dangling and cause a use-after-free on the
+  // next call. Create a fresh reporter each time instead.
+  return internal::CreateReporter(FLAGS_benchmark_format,
+                                  internal::GetOutputOptions())
+      .release();
 }
 
 size_t RunSpecifiedBenchmarks() {

test/CMakeLists.txt

@@ -257,6 +257,7 @@ if (BENCHMARK_ENABLE_GTEST_TESTS)
   add_gtest(string_util_gtest)
   add_gtest(perf_counters_gtest)
   add_gtest(reporter_list_gtest)
+  add_gtest(create_default_display_reporter_gtest)
   add_gtest(time_unit_gtest)
   add_gtest(min_time_parse_gtest)
   add_gtest(profiler_manager_gtest)

test/create_default_display_reporter_gtest.cc

@@ -0,0 +1,27 @@
+#include <memory>
+
+#include "benchmark/benchmark.h"
+#include "gtest/gtest.h"
+
+namespace benchmark {
+namespace {
+
+// Regression test for a heap-use-after-free (issue #2240):
+// CreateDefaultDisplayReporter() transfers ownership of the returned reporter
+// to the caller (RunSpecifiedBenchmarks() wraps it in a unique_ptr and deletes
+// it). It must therefore return a fresh object on every call. A previous
+// implementation cached the pointer in a function-local `static`, so the second
+// call returned an already-deleted reporter -> use-after-free.
+TEST(CreateDefaultDisplayReporterTest, ReturnsFreshOwnedReporterEachCall) {
+  std::unique_ptr<BenchmarkReporter> first(CreateDefaultDisplayReporter());
+  std::unique_ptr<BenchmarkReporter> second(CreateDefaultDisplayReporter());
+
+  ASSERT_NE(first, nullptr);
+  ASSERT_NE(second, nullptr);
+  // Distinct objects: neither call hands back a shared/cached pointer that the
+  // other unique_ptr would also try to delete (double-free / use-after-free).
+  EXPECT_NE(first.get(), second.get());
+}
+
+}  // namespace
+}  // namespace benchmark