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

include/benchmark/benchmark_api.h

@@ -54,7 +54,8 @@ BENCHMARK_EXPORT void SetBenchmarkFilter(std::string value);
 
 BENCHMARK_EXPORT int32_t GetBenchmarkVerbosity();
 
-BENCHMARK_EXPORT BenchmarkReporter* CreateDefaultDisplayReporter();
+BENCHMARK_EXPORT std::unique_ptr<BenchmarkReporter>
+CreateDefaultDisplayReporter();
 
 BENCHMARK_EXPORT size_t RunSpecifiedBenchmarks();
 BENCHMARK_EXPORT size_t RunSpecifiedBenchmarks(std::string spec);

src/benchmark.cc

@@ -585,12 +585,13 @@ 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;
+std::unique_ptr<BenchmarkReporter> CreateDefaultDisplayReporter() {
+  // Ownership of the returned reporter is transferred to the caller, so a fresh
+  // reporter is created on every call. Do NOT cache it in a `static`: the
+  // caller owns and destroys it, which would leave the cached pointer dangling
+  // and cause a use-after-free on the next call.
+  return internal::CreateReporter(FLAGS_benchmark_format,
+                                  internal::GetOutputOptions());
 }
 
 size_t RunSpecifiedBenchmarks() {
@@ -629,7 +630,7 @@ size_t RunSpecifiedBenchmarks(BenchmarkReporter* display_reporter,
   std::unique_ptr<BenchmarkReporter> default_display_reporter;
   std::unique_ptr<BenchmarkReporter> default_file_reporter;
   if (display_reporter == nullptr) {
-    default_display_reporter.reset(CreateDefaultDisplayReporter());
+    default_display_reporter = CreateDefaultDisplayReporter();
     display_reporter = default_display_reporter.get();
   }
   auto& Out = display_reporter->GetOutputStream();

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,26 @@
+#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, so it must 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