From 283ee51383837541b37429260db0216d9c616cf5 Mon Sep 17 00:00:00 2001
From: devtejasx <tejasnagmote520@gmail.com>
Date: Thu, 23 Jul 2026 20:57:45 +0530
Subject: [PATCH] Fix use-after-free in CreateDefaultDisplayReporter (#2240)

CreateDefaultDisplayReporter() transfers ownership of the returned
reporter to the caller, but cached that owning pointer in a
function-local static. After the first RunSpecifiedBenchmarks() call
destroyed the reporter, a subsequent call returned the same, now
dangling pointer, causing a heap-use-after-free in
BenchmarkReporter::GetOutputStream().

Return std::unique_ptr<BenchmarkReporter> so the ownership transfer is
expressed in the type, and drop the static so every call returns a
fresh reporter.
---
 include/benchmark/benchmark_api.h             |  3 ++-
 src/benchmark.cc                              | 15 ++++++-----
 test/CMakeLists.txt                           |  1 +
 test/create_default_display_reporter_gtest.cc | 26 +++++++++++++++++++
 4 files changed, 37 insertions(+), 8 deletions(-)
 create mode 100644 test/create_default_display_reporter_gtest.cc

diff --git a/include/benchmark/benchmark_api.h b/include/benchmark/benchmark_api.h
index 6d98aba581..1bff47278c 100644
--- a/include/benchmark/benchmark_api.h
+++ b/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);
diff --git a/src/benchmark.cc b/src/benchmark.cc
index 91280295e8..083c630e6e 100644
--- a/src/benchmark.cc
+++ b/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();
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index fe88841dd9..d10e780a52 100644
--- a/test/CMakeLists.txt
+++ b/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)
diff --git a/test/create_default_display_reporter_gtest.cc b/test/create_default_display_reporter_gtest.cc
new file mode 100644
index 0000000000..755ffab981
--- /dev/null
+++ b/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