CVE-2025-0247
Overview
Background
- `SandboxBroker`
- the Linux broker process that mediates filesystem syscalls made by sandboxed content/GPU processes against a whitelist
Policy. - `SandboxBroker::Policy`
- the set of path rules (exact paths and prefixes) that decides which files a sandboxed process may open and with what permissions.
- Recursive tree walk
- the old
AddTreeenumerated a directory at policy-build time withopendir/readdirand called itself for every subdirectory, adding oneAddPathrule per regular file found. - Prefix rule
- a single
Policyentry (installed byAddTreeInternal) that grants access to everything under a path without walking the filesystem.
Root Cause Analysis
When building the sandbox policy, SandboxBroker::Policy::AddTree walked directory trees such as /etc, /usr/share, and the LD_LIBRARY_PATH-derived directories by recursively calling itself for each subdirectory returned by readdir, appending each child name into an nsAutoCString and issuing a per-file AddPath rule. Because the recursion depth was bounded only by the on-disk directory nesting and symlinks were not resolved or loop-guarded, an attacker who could influence the directory layout the browser scans (deeply nested or self-referential directory structures) could drive AddTree into unbounded recursion, exhausting the stack and corrupting adjacent memory. The invariant that policy construction must terminate in bounded stack and heap use was violated by treating untrusted, arbitrarily-deep filesystem structure as a recursion driver.
The fix removes the recursive file-enumerating AddTree entirely and repurposes the name for the former AddDir logic, which installs a single prefix rule via AddTreeInternal instead of descending the tree. Because no opendir/readdir recursion or per-entry string building happens anymore, the depth of the scanned directory can no longer influence stack or heap consumption.
AddTreeInternal).Attack Path
- Stage a hostile directory tree
On a shared or attacker-controlled system, create a deeply nested or symlink-looped directory under a path Firefox scans (e.g.
/usr/share,/etc, or aLD_LIBRARY_PATHcomponent). - Trigger policy construction
Launching a sandboxed content or GPU process causes the broker to build its
Policy, invokingAddTreeon those roots viaAddGLDependencies,AddLdconfigPaths, orAddLdLibraryEnvPaths. - Force runaway recursion
AddTreerecurses into each subdirectory without a depth or symlink-loop guard, each frame allocating annsAutoCStringand calling deeper. - Corrupt memory The unbounded recursion overruns the stack into adjacent memory, producing the memory corruption the advisory cites.
- Escalate With controlled directory contents shaping the recursion, the corruption can potentially be steered toward arbitrary code execution in the affected process.
Impact Assessment
/etc, /usr/share, /usr/local/share, /dev/dri, or LD_LIBRARY_PATH targets), typically on multi-user or otherwise attacker-provisioned Linux hosts, and per the advisory sufficient effort could turn this corruption into arbitrary code execution.Changed Functions
| Function | Change | Notes |
|---|---|---|
ifsecurity/sandbox/linux/broker/SandboxBroker.cpp |
modified | |
whilesecurity/sandbox/linux/broker/SandboxBroker.cpp |
modified | |
forsecurity/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp |
modified | |
ifsecurity/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp |
modified |
Files Changed
security/sandbox/linux/broker/SandboxBroker.cppsecurity/sandbox/linux/broker/SandboxBroker.hsecurity/sandbox/linux/broker/SandboxBrokerPolicyFactory.cppsecurity/sandbox/linux/gtest/TestBrokerPolicy.cpp
Audit Directions
- Untrusted-depth recursionSearch for functions that recurse over
readdir/opendirresults or other externally-shaped structures without a depth cap or symlink-loop guard. - Filesystem enumeration at startupReview remaining code that scans directories during sandbox or policy setup and confirm access is granted via bounded prefix rules rather than per-entry walks.
- Symlink handlingLook for tree traversals that do not resolve or reject symlinks, since loops there can turn bounded-looking directories into unbounded recursion.
Patch
diff --git a/security/sandbox/linux/broker/SandboxBroker.cpp b/security/sandbox/linux/broker/SandboxBroker.cpp
index cca7c368598..648ab902709 100644
--- a/security/sandbox/linux/broker/SandboxBroker.cpp
+++ b/security/sandbox/linux/broker/SandboxBroker.cpp
@@ -160,34 +160,6 @@ void SandboxBroker::Policy::AddPath(int aPerms, const char* aPath,
void SandboxBroker::Policy::AddTree(int aPerms, const char* aPath) {
struct stat statBuf;
- if (stat(aPath, &statBuf) != 0) {
- return;
- }
- if (!S_ISDIR(statBuf.st_mode)) {
- AddPath(aPerms, aPath, AddAlways);
- } else {
- DIR* dirp = opendir(aPath);
- if (!dirp) {
- return;
- }
- while (struct dirent* de = readdir(dirp)) {
- if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0) {
- continue;
- }
- // Note: could optimize the string handling.
- nsAutoCString subPath;
- subPath.Assign(aPath);
- subPath.Append('/');
- subPath.Append(de->d_name);
- AddTree(aPerms, subPath.get());
- }
- closedir(dirp);
- }
-}
-
-void SandboxBroker::Policy::AddDir(int aPerms, const char* aPath) {
- struct stat statBuf;
-
if (stat(aPath, &statBuf) != 0) {
return;
}
@@ -196,14 +168,14 @@ void SandboxBroker::Policy::AddDir(int aPerms, const char* aPath) {
return;
}
- Policy::AddDirInternal(aPerms, aPath);
+ Policy::AddTreeInternal(aPerms, aPath);
}
void SandboxBroker::Policy::AddFutureDir(int aPerms, const char* aPath) {
- Policy::AddDirInternal(aPerms, aPath);
+ Policy::AddTreeInternal(aPerms, aPath);
}
-void SandboxBroker::Policy::AddDirInternal(int aPerms, const char* aPath) {
+void SandboxBroker::Policy::AddTreeInternal(int aPerms, const char* aPath) {
// Add a Prefix permission on things inside the dir.
nsDependentCString path(aPath);
MOZ_ASSERT(path.Length() <= kMaxPathLen - 1);
@@ -270,7 +242,7 @@ void SandboxBroker::Policy::AddDynamic(int aPerms, const char* aPath) {
size_t len = strlen(aPath);
if (!len) return;
if (aPath[len - 1] == '/') {
- AddDir(aPerms, aPath);
+ AddTree(aPerms, aPath);
} else {
AddPath(aPerms, aPath);
}
@@ -308,7 +280,7 @@ void SandboxBroker::Policy::FixRecursivePermissions() {
nsAutoCString ancestor(path);
// This is slightly different from the loop in AddAncestors: it
- // leaves the trailing slashes attached so they'll match AddDir
+ // leaves the trailing slashes attached so they'll match AddTree
// entries.
while (true) {
// Last() release-asserts that the string is not empty. We
diff --git a/security/sandbox/linux/broker/SandboxBroker.h b/security/sandbox/linux/broker/SandboxBroker.h
index 7e8cf2c3f19..96193d2c853 100644
--- a/security/sandbox/linux/broker/SandboxBroker.h
+++ b/security/sandbox/linux/broker/SandboxBroker.h
@@ -70,7 +70,7 @@ class SandboxBroker final : private SandboxBrokerCommon,
Policy(const Policy& aOther);
~Policy();
- // Add permissions from AddDir/AddDynamic rules to any rules that
+ // Add permissions from AddTree/AddDynamic rules to any rules that
// exist for their descendents, and remove any descendent rules
// made redundant by this process.
//
@@ -87,12 +87,9 @@ class SandboxBroker final : private SandboxBrokerCommon,
// need to be whitelisted, but this allows adding entries for
// them if they'll exist later. See also the overload below.
void AddPath(int aPerms, const char* aPath, AddCondition aCond);
- // This adds all regular files (not directories) in the tree
- // rooted at the given path.
- void AddTree(int aPerms, const char* aPath);
// A directory, and all files and directories under it, even those
// added after creation (the dir itself must exist).
- void AddDir(int aPerms, const char* aPath);
+ void AddTree(int aPerms, const char* aPath);
// A directory, and all files and directories under it, even those
// added after creation (the dir itself may not exist).
void AddFutureDir(int aPerms, const char* aPath);
@@ -128,7 +125,7 @@ class SandboxBroker final : private SandboxBrokerCommon,
// * No /../ path traversal
bool ValidatePath(const char* path) const;
void AddPrefixInternal(int aPerms, const nsACString& aPath);
- void AddDirInternal(int aPerms, const char* aPath);
+ void AddTreeInternal(int aPerms, const char* aPath);
};
// Constructing a broker involves creating a socketpair and a
diff --git a/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp b/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp
index 88f3c9d97a9..e8b23536de4 100644
--- a/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp
+++ b/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp
@@ -301,7 +301,7 @@ static void AddLdconfigPaths(SandboxBroker::Policy* aPolicy) {
});
}
for (const CacheE& e : ldConfigCache) {
- aPolicy->AddDir(e.second, e.first.get());
+ aPolicy->AddTree(e.second, e.first.get());
}
}
@@ -315,7 +315,7 @@ static void AddLdLibraryEnvPaths(SandboxBroker::Policy* aPolicy) {
for (const nsACString& libPath : LdLibraryEnv.Split(':')) {
char* resolvedPath = realpath(PromiseFlatCString(libPath).get(), nullptr);
if (resolvedPath) {
- aPolicy->AddDir(rdonly, resolvedPath);
+ aPolicy->AddTree(rdonly, resolvedPath);
free(resolvedPath);
}
}
@@ -388,16 +388,16 @@ static void AddX11Dependencies(SandboxBroker::Policy* policy) {
static void AddGLDependencies(SandboxBroker::Policy* policy) {
// Devices
- policy->AddDir(rdwr, "/dev/dri");
+ policy->AddTree(rdwr, "/dev/dri");
policy->AddFilePrefix(rdwr, "/dev", "nvidia");
// Hardware info
AddDriPaths(policy);
// /etc and /usr/share (glvnd, libdrm, drirc, ...?)
- policy->AddDir(rdonly, "/etc");
- policy->AddDir(rdonly, "/usr/share");
- policy->AddDir(rdonly, "/usr/local/share");
+ policy->AddTree(rdonly, "/etc");
+ policy->AddTree(rdonly, "/usr/share");
+ policy->AddTree(rdonly, "/usr/local/share");
// Snap puts the usual /usr/share things in a different place, and
// we'll fail to load the library if we don't have (at least) the
@@ -405,7 +405,7 @@ static void AddGLDependencies(SandboxBroker::Policy* policy) {
if (const char* snapDesktopDir = PR_GetEnv("SNAP_DESKTOP_RUNTIME")) {
nsAutoCString snapDesktopShare(snapDesktopDir);
snapDesktopShare.AppendLiteral("/usr/share");
- policy->AddDir(rdonly, snapDesktopShare.get());
+ policy->AddTree(rdonly, snapDesktopShare.get());
}
// Introduced by Snap's core24 changes there is a gpu-2404 dependency and
@@ -413,7 +413,7 @@ static void AddGLDependencies(SandboxBroker::Policy* policy) {
if (const char* snapRoot = PR_GetEnv("SNAP")) {
nsAutoCString snapRootString(snapRoot);
snapRootString.AppendLiteral("/gpu-2404");
- policy->AddDir(rdonly, snapRootString.get());
+ policy->AddTree(rdonly, snapRootString.get());
}
// Note: This function doesn't do anything about Mesa's shader
@@ -447,24 +447,24 @@ void SandboxBrokerPolicyFactory::InitContentPolicy() {
policy->AddPath(rdonly, "/proc/sys/crypto/fips_enabled");
policy->AddPath(rdonly, "/proc/cpuinfo");
policy->AddPath(rdonly, "/proc/meminfo");
- policy->AddDir(rdonly, "/sys/devices/cpu");
- policy->AddDir(rdonly, "/sys/devices/system/cpu");
- policy->AddDir(rdonly, "/lib");
- policy->AddDir(rdonly, "/lib64");
- policy->AddDir(rdonly, "/usr/lib");
- policy->AddDir(rdonly, "/usr/lib32");
- policy->AddDir(rdonly, "/usr/lib64");
- policy->AddDir(rdonly, "/etc");
- policy->AddDir(rdonly, "/usr/share");
- policy->AddDir(rdonly, "/usr/local/share");
+ policy->AddTree(rdonly, "/sys/devices/cpu");
+ policy->AddTree(rdonly, "/sys/devices/system/cpu");
+ policy->AddTree(rdonly, "/lib");
+ policy->AddTree(rdonly, "/lib64");
+ policy->AddTree(rdonly, "/usr/lib");
+ policy->AddTree(rdonly, "/usr/lib32");
+ policy->AddTree(rdonly, "/usr/lib64");
+ policy->AddTree(rdonly, "/etc");
+ policy->AddTree(rdonly, "/usr/share");
+ policy->AddTree(rdonly, "/usr/local/share");
// Various places where fonts reside
- policy->AddDir(rdonly, "/usr/X11R6/lib/X11/fonts");
- policy->AddDir(rdonly, "/nix/store");
+ policy->AddTree(rdonly, "/usr/X11R6/lib/X11/fonts");
+ policy->AddTree(rdonly, "/nix/store");
// https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/blob/e434e680d22260f277f4a30ec4660ed32b591d16/files/fontconfig-flatpak.conf
- policy->AddDir(rdonly, "/run/host/fonts");
- policy->AddDir(rdonly, "/run/host/user-fonts");
- policy->AddDir(rdonly, "/run/host/local-fonts");
- policy->AddDir(rdonly, "/var/cache/fontconfig");
+ policy->AddTree(rdonly, "/run/host/fonts");
+ policy->AddTree(rdonly, "/run/host/user-fonts");
+ policy->AddTree(rdonly, "/run/host/local-fonts");
+ policy->AddTree(rdonly, "/var/cache/fontconfig");
// Bug 1848615
policy->AddPath(rdonly, "/usr");
@@ -542,7 +542,7 @@ void SandboxBrokerPolicyFactory::InitContentPolicy() {
nsAutoCString tmpPath;
rv = confDir->GetNativePath(tmpPath);
if (NS_SUCCEEDED(rv)) {
- policy->AddDir(rdonly, tmpPath.get());
+ policy->AddTree(rdonly, tmpPath.get());
}
}
}
@@ -592,7 +592,7 @@ void SandboxBrokerPolicyFactory::InitContentPolicy() {
nsAutoCString tmpPath;
rv = confDir->GetNativePath(tmpPath);
if (NS_SUCCEEDED(rv)) {
- policy->AddDir(rdonly, tmpPath.get());
+ policy->AddTree(rdonly, tmpPath.get());
}
}
}
@@ -635,7 +635,7 @@ void SandboxBrokerPolicyFactory::InitContentPolicy() {
nsAutoCString tmpPath;
rv = ffDir->GetNativePath(tmpPath);
if (NS_SUCCEEDED(rv)) {
- policy->AddDir(rdonly, tmpPath.get());
+ policy->AddTree(rdonly, tmpPath.get());
}
}
@@ -645,7 +645,7 @@ void SandboxBrokerPolicyFactory::InitContentPolicy() {
// from the whole repository. MOZ_DEVELOPER_REPO_DIR is set by mach run.
const char* developer_repo_dir = PR_GetEnv("MOZ_DEVELOPER_REPO_DIR");
if (developer_repo_dir) {
- policy->AddDir(rdonly, developer_repo_dir);
+ policy->AddTree(rdonly, developer_repo_dir);
}
}
@@ -676,7 +676,7 @@ void SandboxBrokerPolicyFactory::InitContentPolicy() {
// When running as a snap, the directory pointed to by $SNAP is guaranteed
// to exist before the app is launched, but unit tests need to create it
// dynamically, hence the use of AddFutureDir().
- policy->AddDir(rdonly, snap);
+ policy->AddTree(rdonly, snap);
}
// Read any extra paths that will get write permissions,
@@ -702,7 +702,7 @@ void SandboxBrokerPolicyFactory::InitContentPolicy() {
nsAutoCString tmpPath;
rv = workDir->GetNativePath(tmpPath);
if (NS_SUCCEEDED(rv)) {
- policy->AddDir(rdonly, tmpPath.get());
+ policy->AddTree(rdonly, tmpPath.get());
}
}
}
@@ -720,7 +720,7 @@ void SandboxBrokerPolicyFactory::InitContentPolicy() {
policy->AddPrefix(rdonly, tmpPath.get());
policy->AddPath(rdonly, tmpPath.get());
} else {
- policy->AddDir(rdonly, tmpPath.get());
+ policy->AddTree(rdonly, tmpPath.get());
}
}
}
@@ -742,11 +742,11 @@ void SandboxBrokerPolicyFactory::InitContentPolicy() {
if (allowAlsa) {
// Bug 1309098: ALSA support
- policy->AddDir(rdwr, "/dev/snd");
+ policy->AddTree(rdwr, "/dev/snd");
}
if (allowPulse) {
- policy->AddDir(rdwrcr, "/dev/shm");
+ policy->AddTree(rdwrcr, "/dev/shm");
}
#ifdef MOZ_WIDGET_GTK
@@ -776,7 +776,7 @@ void SandboxBrokerPolicyFactory::InitContentPolicy() {
// Bug 1434711 - AMDGPU-PRO crashes if it can't read it's marketing ids
// and various other things
if (!headless && HasAtiDrivers()) {
- policy->AddDir(rdonly, "/opt/amdgpu/share");
+ policy->AddTree(rdonly, "/opt/amdgpu/share");
policy->AddPath(rdonly, "/sys/module/amdgpu");
}
@@ -806,7 +806,7 @@ UniquePtr<SandboxBroker::Policy> SandboxBrokerPolicyFactory::GetContentPolicy(
// No read blocking at level 2 and below.
// file:// processes also get global read permissions
if (level <= 2 || aFileProcess) {
- policy->AddDir(rdonly, "/");
+ policy->AddTree(rdonly, "/");
// Any other read-only rules will be removed as redundant by
// Policy::FixRecursivePermissions, so there's no need to
// early-return here.
@@ -904,16 +904,16 @@ SandboxBrokerPolicyFactory::GetRDDPolicy(int aPid) {
"/sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq");
policy->AddPath(rdonly, "/sys/devices/system/cpu/cpu0/cache/index2/size");
policy->AddPath(rdonly, "/sys/devices/system/cpu/cpu0/cache/index3/size");
- policy->AddDir(rdonly, "/sys/devices/cpu");
- policy->AddDir(rdonly, "/sys/devices/system/cpu");
- policy->AddDir(rdonly, "/sys/devices/system/node");
- policy->AddDir(rdonly, "/lib");
- policy->AddDir(rdonly, "/lib64");
- policy->AddDir(rdonly, "/usr/lib");
- policy->AddDir(rdonly, "/usr/lib32");
- policy->AddDir(rdonly, "/usr/lib64");
- policy->AddDir(rdonly, "/run/opengl-driver/lib");
- policy->AddDir(rdonly, "/nix/store");
+ policy->AddTree(rdonly, "/sys/devices/cpu");
+ policy->AddTree(rdonly, "/sys/devices/system/cpu");
+ policy->AddTree(rdonly, "/sys/devices/system/node");
+ policy->AddTree(rdonly, "/lib");
+ policy->AddTree(rdonly, "/lib64");
+ policy->AddTree(rdonly, "/usr/lib");
+ policy->AddTree(rdonly, "/usr/lib32");
+ policy->AddTree(rdonly, "/usr/lib64");
+ policy->AddTree(rdonly, "/run/opengl-driver/lib");
+ policy->AddTree(rdonly, "/nix/store");
// Bug 1647957: memory reporting.
AddMemoryReporting(policy.get(), aPid);
@@ -929,7 +929,7 @@ SandboxBrokerPolicyFactory::GetRDDPolicy(int aPid) {
nsAutoCString tmpPath;
rv = ffDir->GetNativePath(tmpPath);
if (NS_SUCCEEDED(rv)) {
- policy->AddDir(rdonly, tmpPath.get());
+ policy->AddTree(rdonly, tmpPath.get());
}
}
@@ -939,7 +939,7 @@ SandboxBrokerPolicyFactory::GetRDDPolicy(int aPid) {
// from the whole repository. MOZ_DEVELOPER_REPO_DIR is set by mach run.
const char* developer_repo_dir = PR_GetEnv("MOZ_DEVELOPER_REPO_DIR");
if (developer_repo_dir) {
- policy->AddDir(rdonly, developer_repo_dir);
+ policy->AddTree(rdonly, developer_repo_dir);
}
}
@@ -959,8 +959,8 @@ SandboxBrokerPolicyFactory::GetRDDPolicy(int aPid) {
// Only built on ARM64 since Tegra is ARM64 SoC with different drivers, so the
// path are not needed on e.g. x86-64
#if defined(__aarch64__)
- policy->AddDir(rdonly, "/sys/devices/system/present");
- policy->AddDir(rdonly, "/sys/module/tegra_fuse");
+ policy->AddTree(rdonly, "/sys/devices/system/present");
+ policy->AddTree(rdonly, "/sys/module/tegra_fuse");
policy->AddPath(rdwr, "/dev/nvmap");
policy->AddPath(rdwr, "/dev/nvhost-ctrl");
policy->AddPath(rdwr, "/dev/nvhost-ctrl-gpu");
@@ -984,21 +984,21 @@ SandboxBrokerPolicyFactory::GetSocketProcessPolicy(int aPid) {
policy->AddPath(rdonly, "/proc/sys/crypto/fips_enabled");
policy->AddPath(rdonly, "/proc/cpuinfo");
policy->AddPath(rdonly, "/proc/meminfo");
- policy->AddDir(rdonly, "/sys/devices/cpu");
- policy->AddDir(rdonly, "/sys/devices/system/cpu");
- policy->AddDir(rdonly, "/lib");
- policy->AddDir(rdonly, "/lib64");
- policy->AddDir(rdonly, "/usr/lib");
- policy->AddDir(rdonly, "/usr/lib32");
- policy->AddDir(rdonly, "/usr/lib64");
- policy->AddDir(rdonly, "/usr/share");
- policy->AddDir(rdonly, "/usr/local/share");
- policy->AddDir(rdonly, "/etc");
+ policy->AddTree(rdonly, "/sys/devices/cpu");
+ policy->AddTree(rdonly, "/sys/devices/system/cpu");
+ policy->AddTree(rdonly, "/lib");
+ policy->AddTree(rdonly, "/lib64");
+ policy->AddTree(rdonly, "/usr/lib");
+ policy->AddTree(rdonly, "/usr/lib32");
+ policy->AddTree(rdonly, "/usr/lib64");
+ policy->AddTree(rdonly, "/usr/share");
+ policy->AddTree(rdonly, "/usr/local/share");
+ policy->AddTree(rdonly, "/etc");
// glibc will try to stat64("/") while populating nsswitch database
// https://sourceware.org/git/?p=glibc.git;a=blob;f=nss/nss_database.c;h=cf0306adc47f12d9bc761ab1b013629f4482b7e6;hb=9826b03b747b841f5fc6de2054bf1ef3f5c4bdf3#l396
// denying will make getaddrinfo() return ENONAME
- policy->AddDir(access, "/");
+ policy->AddPath(access, "/");
AddLdconfigPaths(policy.get());
@@ -1020,7 +1020,7 @@ SandboxBrokerPolicyFactory::GetSocketProcessPolicy(int aPid) {
nsAutoCString tmpPath;
rv = ffDir->GetNativePath(tmpPath);
... (file diff truncated)