High watch CVSS 8.8 chrome OOB 📄 Reporter bug report 🔧 Commit mapped

Overview

High

Severity

8.8

CVSS

Exploited ITW

Fixed

Fix Status

ImpactStack buffer overflow in Tracing

DescriptionStack buffer overflow in Tracing

ComponentTracing

Sub-componentTracing

Bug ClassOOB

Action tierwatch — corruption primitive but gated (local) — not default-reachable
Computed by PatchGap's deterministic exploitability ruler from the fixing commit. patch-now = RCE-class & attacker-reachable in a default config, or exploited in the wild; watch = real security signal below that bar; noise = no security relevance.

Tracker384186539

Fix commitb6ec777da385 (chromium/src) +51/-10

CWECWE-121 (Stack buffer overflow)

CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H

CISA KEVNot listed

CreditedHan Zheng (HexHive)

Reported2024-12-15 (per vendor advisory)

Fix landed2024-12-23 (public git)

Disclosed2025-01-14

Patch gap22 days — fix public in source before the advisory

Background

Tracing service: Chrome’s services/tracing component that connects to Perfetto’s system tracing daemon over a UNIX domain socket to collect performance traces.
`sockaddr_un`: the UNIX-domain socket address struct whose sun_path member is a fixed-size character array (typically 108 bytes) that holds the socket’s filesystem path.
Producer socket name: the path a tracing producer connects to, taken from the kProducerSockEnvName environment variable and copied into sockaddr_un::sun_path.
OOB write: writing past the bounds of a buffer, here a stack-allocated sockaddr_un whose sun_path array is overflowed by an oversized producer socket name.

Root Cause Analysis

When SystemTracingService opened a producer connection, it copied the caller-influenced producer_sock_name string into the fixed-size sun_path field of a stack-allocated struct sockaddr_un saddr without first checking that the name fit. Because sun_path is a fixed array (bounded by sizeof(sockaddr_un::sun_path)), a producer socket name longer than that bound caused the copy to write past the end of the struct on the stack, corrupting adjacent stack memory. The invariant that a UNIX socket path must be shorter than sizeof(sockaddr_un::sun_path) - 1 (leaving room for the NUL terminator) was never enforced.

The fix adds an explicit length check that returns base::unexpected(EINVAL) and refuses to make the connection when producer_sock_name.size() > sizeof(sockaddr_un::sun_path) - 1, so no oversized string ever reaches the copy into sun_path.

Key insight

The core mistake was copying an externally supplied socket name into the fixed-size sockaddr_un::sun_path buffer without a bounds check; the fix validates the length against sizeof(sockaddr_un::sun_path) - 1 up front and returns an error for an invalid name instead of proceeding to the overflowing copy.

Attack Path

Control the socket name An attacker sets the kProducerSockEnvName environment variable (or otherwise influences the producer socket name) to a string longer than sizeof(sockaddr_un::sun_path) - 1.
Trigger connection setup The tracing code path in SystemTracingService runs and constructs a stack struct sockaddr_un saddr to open the producer connection.
Overflow the stack buffer The oversized name is copied into the fixed-size sun_path array, writing past the struct and corrupting adjacent stack memory.
Escalate from corruption By shaping the overflow, the attacker overwrites saved registers or local state on the stack to redirect control flow or corrupt security-relevant data.

Impact Assessment

An attacker who can influence the producer socket name gains a stack buffer overflow in the process hosting the tracing service, enabling memory corruption that can lead to control-flow hijacking or crashes. The precondition is control over the producer socket name environment variable and the ability to trigger a system tracing producer connection, so exploitation depends on a local attacker or a compromised component able to set that value.

Changed Functions

Function	Change	Notes
`if` services/tracing/perfetto/system_test_utils.cc	modified
`if` services/tracing/public/cpp/system_tracing_service_unittest.cc	modified
`TEST_F` services/tracing/public/cpp/system_tracing_service_unittest.cc	modified

Files Changed

services/tracing/perfetto/system_test_utils.cc
services/tracing/perfetto/system_test_utils.h
services/tracing/public/cpp/system_tracing_service.cc
services/tracing/public/cpp/system_tracing_service_unittest.cc

Audit Directions

Fixed-size socket paths
Search for copies into sockaddr_un::sun_path (and similar fixed address arrays) that lack a check against sizeof(...sun_path) - 1 on externally influenced strings.
Env-derived buffers
Look for getenv results stored as raw const char* and reused later, since a subsequent setenv can invalidate that pointer and cause a use-after-free.
Unvalidated external inputs into fixed arrays
Audit paths where environment variables, IPC parameters, or file paths flow into stack-allocated fixed-size buffers without a length guard before the copy.

Patch

view commit

From b6ec777da385a63fd8c57f2a86acdb3e43420eb0 Mon Sep 17 00:00:00 2001
From: Chinglin Yu <chinglinyu@chromium.org>
Date: Sun, 22 Dec 2024 19:48:09 -0800
Subject: [PATCH] Reland "tracing: handle invalid producer socket name"

This reverts commit 3d784689fc8ac31c97f03712f2efba5eb5a1512a.

Reason for revert: fixed unittest crash in Mac ASAN build.

The test case saves/restores environment variables using
setenv()/getenv(). The original environment variable value is saved as
const char* to the internal buffer allocated by setenv(), which is prone to UAF if the same environment variable is set again. Fix by copying the value from getenv() to avoid the crash. Also fixed system_test_utils.cc that has a similar usage pattern and has suppressed ASAN failure in Mac
ASAN build.

Original change's description:
> Revert "tracing: handle invalid producer socket name"
>
> This reverts commit 8fe29b08f181fda986f9d58c1f3edc283457dc7a.
>
> Reason for revert: b/384862278
>
> Original change's description:
> > tracing: handle invalid producer socket name
> >
> > Don't make the socket connection on seeing an invalid socket name and
> > return an invalid FD to the opener.
> >
> > Bug: 384186539
> > Test: Unit test.
> > Change-Id: I3fce049c4bbe5a078dfe5456208697afa71262ea
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6100572
> > Reviewed-by: Charles Dick <cwd@google.com>
> > Reviewed-by: Mikhail Khokhlov <khokhlov@google.com>
> > Reviewed-by: Stephen Nusko <nuskos@chromium.org>
> > Commit-Queue: Chinglin Yu <chinglinyu@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1397727}
>
> Bug: 384186539
> Change-Id: Ia54883fc1e9d60a17f8224cf0270a2e32ad04c05
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6097780
> Auto-Submit: Adem Derinel <derinel@google.com>
> Owners-Override: Adem Derinel <derinel@google.com>
> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/main@{#1397801}

Bug: 384186539, 384862278
Change-Id: Ib0c63080d13afa958a127166dd1fa15f9b122859
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6108519
Reviewed-by: Mikhail Khokhlov <khokhlov@google.com>
Reviewed-by: Charles Dick <cwd@google.com>
Commit-Queue: Chinglin Yu <chinglinyu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1399697}
---

diff --git a/services/tracing/perfetto/system_test_utils.cc b/services/tracing/perfetto/system_test_utils.cc
index 24370e5c..92c9c257 100644
--- a/services/tracing/perfetto/system_test_utils.cc
+++ b/services/tracing/perfetto/system_test_utils.cc
@@ -44,7 +44,10 @@
   // multiple tests run we need to restore the value so each test is
   // hermetic.
 
-  old_tmpdir_ = getenv("TMPDIR");
+  const auto* old_tmpdir = getenv("TMPDIR");
+  if (old_tmpdir) {
+    old_tmpdir_ = old_tmpdir;
+  }
   setenv("TMPDIR", tmp_dir.GetPath().value().c_str(), true);
   // Set up the system socket locations in a valid tmp directory.
   producer_ = tmp_dir.GetPath().Append(FILE_PATH_LITERAL("producer")).value();
@@ -59,7 +62,7 @@
   if (used_tmpdir_) {
     if (old_tmpdir_) {
       // Restore the old value back to its initial value.
-      setenv("TMPDIR", old_tmpdir_, true);
+      setenv("TMPDIR", old_tmpdir_->c_str(), true);
     } else {
       // TMPDIR wasn't set originally so unset it.
       unsetenv("TMPDIR");
diff --git a/services/tracing/perfetto/system_test_utils.h b/services/tracing/perfetto/system_test_utils.h
index fd3c56f..06bef90 100644
--- a/services/tracing/perfetto/system_test_utils.h
+++ b/services/tracing/perfetto/system_test_utils.h
@@ -40,7 +40,7 @@
   void StartService();
 
   const bool used_tmpdir_;
-  const char* old_tmpdir_ = nullptr;
+  std::optional<std::string> old_tmpdir_;
   std::string consumer_;
   std::string producer_;
   std::unique_ptr<perfetto::ServiceIPCHost> service_;
diff --git a/services/tracing/public/cpp/system_tracing_service.cc b/services/tracing/public/cpp/system_tracing_service.cc
index 57ca7d9..86df2a5e 100644
--- a/services/tracing/public/cpp/system_tracing_service.cc
+++ b/services/tracing/public/cpp/system_tracing_service.cc
@@ -68,6 +68,10 @@
     if (socket_fd_.get() == -1) {
       return base::unexpected(errno);
     }
+    if (producer_sock_name.size() > sizeof(sockaddr_un::sun_path) - 1) {
+      DVLOG(3) << "Oversized producer socket name " << producer_sock_name;
+      return base::unexpected(EINVAL);
+    }
 
     struct sockaddr_un saddr;
     memset(&saddr, 0, sizeof(saddr));
diff --git a/services/tracing/public/cpp/system_tracing_service_unittest.cc b/services/tracing/public/cpp/system_tracing_service_unittest.cc
index bf6358c7..b98b9ab 100644
--- a/services/tracing/public/cpp/system_tracing_service_unittest.cc
+++ b/services/tracing/public/cpp/system_tracing_service_unittest.cc
@@ -4,8 +4,10 @@
 
 #include "services/tracing/public/cpp/system_tracing_service.h"
 
+#include <sys/un.h>
 #include <unistd.h>
 
+#include <cstdint>
 #include <optional>
 
 #include "base/files/scoped_temp_dir.h"
@@ -33,21 +35,26 @@
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     system_service_ = std::make_unique<MockSystemService>(temp_dir_);
 
+    // Save the current producer socket name from env if currently set.
+    const auto* producer_sock_name_env = getenv(kProducerSockEnvName);
+    if (producer_sock_name_env) {
+      saved_producer_sock_env_ = producer_sock_name_env;
+    }
     // Override the default system producer socket.
-    saved_producer_sock_env_ = getenv(kProducerSockEnvName);
     ASSERT_EQ(0, setenv(kProducerSockEnvName,
                         system_service_->producer().c_str(), 1));
   }
 
   void TearDown() override {
-    system_service_ = nullptr;
     // Restore the value of Perfetto producer socket name env variable.
     if (saved_producer_sock_env_) {
-      ASSERT_EQ(0,
-                setenv(kProducerSockEnvName, saved_producer_sock_env_, true));
+      ASSERT_EQ(0, setenv(kProducerSockEnvName,
+                          saved_producer_sock_env_->c_str(), 1));
     } else {
       ASSERT_EQ(0, unsetenv(kProducerSockEnvName));
     }
+
+    system_service_ = nullptr;
     task_environment_.RunUntilIdle();
   }
 
@@ -57,7 +64,7 @@
       base::SingleThreadTaskRunner::GetCurrentDefault()};
   base::ScopedTempDir temp_dir_;
   std::unique_ptr<MockSystemService> system_service_;
-  const char* saved_producer_sock_env_ = nullptr;
+  std::optional<std::string> saved_producer_sock_env_;
 };
 
 // Test the OpenProducerSocket implementation. Expect a valid socket file
@@ -93,7 +100,6 @@
   bool callback_called = false;
 
   // Set the producer socket name to a nonexistent path.
-  saved_producer_sock_env_ = getenv(kProducerSockEnvName);
   ASSERT_EQ(0, setenv(kProducerSockEnvName, "nonexistent_socket", 1));
 
   base::RunLoop run_loop;
@@ -147,7 +153,6 @@
   bool callback_called = false;
 
   // Set the producer socket name to a nonexistent path.
-  saved_producer_sock_env_ = getenv(kProducerSockEnvName);
   ASSERT_EQ(0, setenv(kProducerSockEnvName, "nonexistent_socket", 1));
 
   // Bind the pending remote on the current thread.
@@ -167,5 +172,34 @@
   ASSERT_TRUE(callback_called);
 }
 
+TEST_F(SystemTracingServiceTest, BindAndPassPendingRemote_NonexistentLong) {
+  auto sts = std::make_unique<SystemTracingService>();
+  bool callback_called = false;
+
+  // Set the producer socket name to a long nonexistent path.
+  std::string sock_name;
+  while (sock_name.size() < 1024) {
+    sock_name.append("nonexistent/");
+  }
+  ASSERT_GT(sock_name.size(), sizeof(sockaddr_un::sun_path));
+  ASSERT_EQ(0, setenv(kProducerSockEnvName, sock_name.c_str(), 1));
+
+  // Bind the pending remote on the current thread.
+  mojo::Remote<mojom::SystemTracingService> remote;

Loading diff…

Regression Test / PoC

shipped with the fix

diff --git a/services/tracing/public/cpp/system_tracing_service_unittest.cc b/services/tracing/public/cpp/system_tracing_service_unittest.cc
index bf6358c7..b98b9ab 100644
--- a/services/tracing/public/cpp/system_tracing_service_unittest.cc
+++ b/services/tracing/public/cpp/system_tracing_service_unittest.cc
@@ -4,8 +4,10 @@
 
 #include "services/tracing/public/cpp/system_tracing_service.h"
 
+#include <sys/un.h>
 #include <unistd.h>
 
+#include <cstdint>
 #include <optional>
 
 #include "base/files/scoped_temp_dir.h"
@@ -33,21 +35,26 @@
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     system_service_ = std::make_unique<MockSystemService>(temp_dir_);
 
+    // Save the current producer socket name from env if currently set.
+    const auto* producer_sock_name_env = getenv(kProducerSockEnvName);
+    if (producer_sock_name_env) {
+      saved_producer_sock_env_ = producer_sock_name_env;
+    }
     // Override the default system producer socket.
-    saved_producer_sock_env_ = getenv(kProducerSockEnvName);
     ASSERT_EQ(0, setenv(kProducerSockEnvName,
                         system_service_->producer().c_str(), 1));
   }
 
   void TearDown() override {
-    system_service_ = nullptr;
     // Restore the value of Perfetto producer socket name env variable.
     if (saved_producer_sock_env_) {
-      ASSERT_EQ(0,
-                setenv(kProducerSockEnvName, saved_producer_sock_env_, true));
+      ASSERT_EQ(0, setenv(kProducerSockEnvName,
+                          saved_producer_sock_env_->c_str(), 1));
     } else {
       ASSERT_EQ(0, unsetenv(kProducerSockEnvName));
     }
+
+    system_service_ = nullptr;
     task_environment_.RunUntilIdle();
   }
 
@@ -57,7 +64,7 @@
       base::SingleThreadTaskRunner::GetCurrentDefault()};
   base::ScopedTempDir temp_dir_;
   std::unique_ptr<MockSystemService> system_service_;
-  const char* saved_producer_sock_env_ = nullptr;
+  std::optional<std::string> saved_producer_sock_env_;
 };
 
 // Test the OpenProducerSocket implementation. Expect a valid socket file
@@ -93,7 +100,6 @@
   bool callback_called = false;
 
   // Set the producer socket name to a nonexistent path.
-  saved_producer_sock_env_ = getenv(kProducerSockEnvName);
   ASSERT_EQ(0, setenv(kProducerSockEnvName, "nonexistent_socket", 1));
 
   base::RunLoop run_loop;
@@ -147,7 +153,6 @@
   bool callback_called = false;
 
   // Set the producer socket name to a nonexistent path.
-  saved_producer_sock_env_ = getenv(kProducerSockEnvName);
   ASSERT_EQ(0, setenv(kProducerSockEnvName, "nonexistent_socket", 1));
 
   // Bind the pending remote on the current thread.
@@ -167,5 +172,34 @@
   ASSERT_TRUE(callback_called);
 }
 
+TEST_F(SystemTracingServiceTest, BindAndPassPendingRemote_NonexistentLong) {
+  auto sts = std::make_unique<SystemTracingService>();
+  bool callback_called = false;
+
+  // Set the producer socket name to a long nonexistent path.
+  std::string sock_name;
+  while (sock_name.size() < 1024) {
+    sock_name.append("nonexistent/");
+  }
+  ASSERT_GT(sock_name.size(), sizeof(sockaddr_un::sun_path));
+  ASSERT_EQ(0, setenv(kProducerSockEnvName, sock_name.c_str(), 1));
+
+  // Bind the pending remote on the current thread.
+  mojo::Remote<mojom::SystemTracingService> remote;
+  remote.Bind(sts->BindAndPassPendingRemote(), nullptr);
+
+  base::RunLoop run_loop;
+  auto callback = base::BindLambdaForTesting([&](base::File file) {
+    callback_called = true;
+    ASSERT_FALSE(file.IsValid());
+    run_loop.Quit();
+  });
+
+  remote->OpenProducerSocket(std::move(callback));
+  ASSERT_FALSE(callback_called);
+  run_loop.Run();
+  ASSERT_TRUE(callback_called);
+}
+
 }  // namespace
 }  // namespace tracing

Loading diff…

Original Bug Report

reported by kd...@gmail.com

Security: Potential Stack-Buffer-Overflow in ProducerSocketConnector::ConnectSocket

Steps to reproduce the problem

apply the patch (equal to setting the environment variable)
run ./services_unittests –gtest_filter=SystemTracingServiceTest.OpenProducerSocket_Nonexistent

Problem Description

I’m not sure if it’s a valid assumption (attacker controls the environment variable), but to be safe I reported it.

RCA

In ProducerSocketConnector::ConnectSocket, the producer_sock_name [1] can be read from a environment variable PERFETTO_PRODUCER_SOCK_NAME[3], so if adversary can control the environment variable, the size of the name can be arbitary long.

Then at [2], the producer_sock_name is copied to the struct sockaddr_un’s sun_path, which is fixed for 108 bytes. Therefore, the attacker can craft the PERFETTO_PRODUCER_SOCK_NAME to overwrite the stack, as much as they want.

// services/tracing/public/cpp/system_tracing_service.cc
  base::expected<bool, int> ConnectSocket() {
    std::string producer_sock_name = perfetto::GetProducerSocket(); // [1]
    socket_fd_.reset(socket(AF_UNIX, SOCK_STREAM, 0));
    if (socket_fd_.get() == -1) {
      return base::unexpected(errno);
    }

    struct sockaddr_un saddr;
    memset(&saddr, 0, sizeof(saddr));
    memcpy(saddr.sun_path, producer_sock_name.data(), // [2]
           producer_sock_name.size());

  }

// third_party/perfetto/src/tracing/ipc/default_socket.cc
const char* GetProducerSocket() {
  const char* name = getenv("PERFETTO_PRODUCER_SOCK_NAME"); // [3]
  if (name == nullptr) {
    // ...
  }
  base::ignore_result(UseRunPerfettoBaseDir);  // Silence unused func warnings.
  return name;
}

struct sockaddr_un {
  sa_family_t sun_family; /* AF_UNIX */
  char sun_path[108];     /* Pathname */ //[4]
};

Summary

Security: Potential Stack-Buffer-Overflow in ProducerSocketConnector::ConnectSocket

Custom Questions

Type of crash:

browser

Reporter credit:

Han Zheng (HexHive)

Additional Data

Category: Security
Chrome Channel: Not sure
Regression: N/A

View on issue tracker

References

Vendor advisory Fix commit Bug tracker NVD

On This Page

AI-generated — this data may be wrong

CVE-2025-0438