Refactor AddCertificateReports to prevent crash When processing certificate stats with duplicate fingerprints, ReplaceOrAddNew would delete the existing stats report that was previously added and pointed to by first_report or prev_report. By adding tracking for duplicate fingerprints we can find the existing report and avoid replacing it. Bug: chromium:486495143 Change-Id: Iabc41ae064476c1e5853cdff1dbbcab449f8df27 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/459320 Reviewed-by: Evan Shrubsole <eshr@webrtc.org> Reviewed-by: Henrik Boström <hbos@webrtc.org> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47242}

diff --git a/pc/BUILD.gn b/pc/BUILD.gn
index c07cbd3..f2045cb 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn

@@ -1461,6 +1461,7 @@
     "../rtc_base/system:plan_b_only",
     "../system_wrappers",
     "//third_party/abseil-cpp/absl/base:nullability",
+    "//third_party/abseil-cpp/absl/container:flat_hash_set",
     "//third_party/abseil-cpp/absl/functional:any_invocable",
     "//third_party/abseil-cpp/absl/strings",
     "//third_party/abseil-cpp/absl/strings:string_view",

diff --git a/pc/legacy_stats_collector.cc b/pc/legacy_stats_collector.cc
index f41de1c..6978f1b 100644
--- a/pc/legacy_stats_collector.cc
+++ b/pc/legacy_stats_collector.cc

@@ -21,6 +21,7 @@
 #include <vector>
 
 #include "absl/base/nullability.h"
+#include "absl/container/flat_hash_set.h"
 #include "absl/functional/any_invocable.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/string_view.h"
@@ -792,8 +793,13 @@
 
   StatsReport* first_report = nullptr;
   StatsReport* prev_report = nullptr;
+  absl::flat_hash_set<std::string> visited_fingerprints;
   for (SSLCertificateStats* stats = cert_stats.get(); stats;
        stats = stats->issuer.get()) {
+    if (!visited_fingerprints.insert(stats->fingerprint).second) {
+      break;
+    }
+
     StatsReport::Id id(StatsReport::NewTypedId(
         StatsReport::kStatsReportTypeCertificate, stats->fingerprint));
 

diff --git a/pc/legacy_stats_collector.h b/pc/legacy_stats_collector.h
index 25e61ed..f36e509 100644
--- a/pc/legacy_stats_collector.h
+++ b/pc/legacy_stats_collector.h

@@ -114,6 +114,11 @@
 
   bool UseStandardBytesStats() const { return use_standard_bytes_stats_; }
 
+  StatsReport* AddCertificateReportsForTest(
+      std::unique_ptr<SSLCertificateStats> cert_stats) {
+    return AddCertificateReports(std::move(cert_stats));
+  }
+
  private:
   // Struct that's populated on the network thread and carries the values to
   // the signaling thread where the stats are added to the stats reports.

diff --git a/pc/legacy_stats_collector_unittest.cc b/pc/legacy_stats_collector_unittest.cc
index 63c33ec..f039115 100644
--- a/pc/legacy_stats_collector_unittest.cc
+++ b/pc/legacy_stats_collector_unittest.cc

@@ -57,6 +57,7 @@
 #include "rtc_base/null_socket_server.h"
 #include "rtc_base/rtc_certificate.h"
 #include "rtc_base/socket_address.h"
+#include "rtc_base/ssl_certificate.h"
 #include "rtc_base/ssl_identity.h"
 #include "rtc_base/ssl_stream_adapter.h"
 #include "rtc_base/system/plan_b_only.h"
@@ -1446,6 +1447,21 @@
                          remote_ders);
 }
 
+TEST_F(LegacyStatsCollectorTest,
+       AddCertificateReports_DuplicateFingerprintDoesNotCrash) {
+  auto pc = CreatePeerConnection();
+  auto stats = CreateStatsCollector(pc.get());
+
+  // Create duplicate fingerprints chain
+  auto issuer_stats = std::make_unique<SSLCertificateStats>(
+      "same_fingerprint", "sha-1", "cert_issuer", nullptr);
+  auto cert_stats = std::make_unique<SSLCertificateStats>(
+      "same_fingerprint", "sha-1", "cert_leaf", std::move(issuer_stats));
+
+  // This should not crash (Use-After-Free)
+  stats->AddCertificateReportsForTest(std::move(cert_stats));
+}
+
 // This test verifies that all certificates without chains are correctly
 // reported.
 TEST_F(LegacyStatsCollectorTest, ChainlessCertificateReportsCreated) {