commit 07ac271f2eac7f59f759d735a9cdcf3f5ce02949
parent 0caa00916e637d3c4e990db75d8fb94e93c99ceb
Author: Bob Owen <bobowencode@gmail.com>
Date: Fri, 31 Oct 2025 20:15:51 +0000
Bug 1996225 - Add rules for fonts in subkeys for the GPU sandbox. r=handyman
Differential Revision: https://phabricator.services.mozilla.com/D269988
Diffstat:
4 files changed, 156 insertions(+), 52 deletions(-)
diff --git a/security/sandbox/win/gtest/TestConfigHelpers.cpp b/security/sandbox/win/gtest/TestConfigHelpers.cpp
@@ -11,6 +11,7 @@
#include <windows.h>
#include "nsLiteralString.h"
+#include "nsWindowsHelpers.h"
#include "sandbox/win/src/sandbox.h"
#include "sandbox/win/src/app_container.h"
#include "sandbox/win/src/policy_engine_opcodes.h"
@@ -96,6 +97,16 @@ class MockConfig : public TargetConfig {
EXPECT_CALL(mConfig, AllowFileAccess(Eq(FileSemantics::kAllowReadonly), \
StartsWith(aRulePath)))
+static void SetUpPathsInKey(HKEY aKey,
+ const std::vector<std::wstring_view>& aFontPaths) {
+ for (size_t i = 0; i < aFontPaths.size(); ++i) {
+ const auto* pathBytes = reinterpret_cast<const BYTE*>(aFontPaths[i].data());
+ size_t sizeInBytes = (aFontPaths[i].length() + 1) * sizeof(wchar_t);
+ ::RegSetValueExW(aKey, std::to_wstring(i).c_str(), 0, REG_SZ, pathBytes,
+ sizeInBytes);
+ }
+}
+
class UserFontConfigHelperTest : public testing::Test {
protected:
// We always expect the Windows User font dir rule to be added.
@@ -110,17 +121,11 @@ class UserFontConfigHelperTest : public testing::Test {
if (mTestUserFontKey) {
::RegCloseKey(mTestUserFontKey);
}
- ::RegDeleteKeyW(HKEY_CURRENT_USER, sTestRegKey);
+ ::RegDeleteTreeW(HKEY_CURRENT_USER, sTestRegKey);
}
void SetUpPaths(const std::vector<std::wstring_view>& aFontPaths) {
- for (size_t i = 0; i < aFontPaths.size(); ++i) {
- const auto* pathBytes =
- reinterpret_cast<const BYTE*>(aFontPaths[i].data());
- size_t sizeInBytes = (aFontPaths[i].length() + 1) * sizeof(wchar_t);
- ::RegSetValueExW(mTestUserFontKey, std::to_wstring(i).c_str(), 0, REG_SZ,
- pathBytes, sizeInBytes);
- }
+ SetUpPathsInKey(mTestUserFontKey, aFontPaths);
}
void CreateHelperAndCallAddRules() {
@@ -178,6 +183,25 @@ TEST_F(UserFontConfigHelperTest, PathsOutsideUsersDirAdded) {
CreateHelperAndCallAddRules();
}
+TEST_F(UserFontConfigHelperTest, SubKeyPathsInsideUsersDirAdded) {
+ SetUpPaths({LR"(C:\Users\Moz User\Fonts\FontFile1.ttf)"});
+ std::unique_ptr<HKEY, RegCloseKeyDeleter> subKey;
+ auto lStatus = ::RegCreateKeyExW(mTestUserFontKey, L"SubKey", 0, nullptr,
+ REG_OPTION_VOLATILE, KEY_ALL_ACCESS, nullptr,
+ getter_Transfers(subKey), nullptr);
+ ASSERT_EQ(lStatus, ERROR_SUCCESS);
+ SetUpPathsInKey(subKey.get(), {LR"(C:\Users\Moz User\Fonts\FontFile2.ttf)"});
+
+ // We expect the windows user font rule to be added first.
+ auto& fontFile1 =
+ EXPECT_READONLY_EQ(LR"(C:\Users\Moz User\Fonts\FontFile1.ttf)")
+ .After(mWinUserFontCall);
+ EXPECT_READONLY_EQ(LR"(C:\Users\Moz User\Fonts\FontFile2.ttf)")
+ .After(fontFile1);
+
+ CreateHelperAndCallAddRules();
+}
+
TEST_F(UserFontConfigHelperTest, PathsOutsideUsersDirAddedAtEnd) {
// We set up the paths in a particular order, but this doesn't guarantee the
// order returned from registry calls. However the rule adding code should
@@ -201,6 +225,36 @@ TEST_F(UserFontConfigHelperTest, PathsOutsideUsersDirAddedAtEnd) {
CreateHelperAndCallAddRules();
}
+TEST_F(UserFontConfigHelperTest, SubKeyPathsOutsideUsersDirAddedAtEnd) {
+ // We set up the paths in a particular order, but this doesn't guarantee the
+ // order returned from registry calls. However the rule adding code should
+ // guarantee that non-user dir fonts are added at the end.
+ const auto* userFont1 = LR"(C:\Users\Moz User\Fonts\FontFile1.ttf)";
+ const auto* userFont2 = LR"(C:\Users\Moz User\Fonts\FontFile2.ttf)";
+ const auto* userFont3 = LR"(C:\Users\Moz User\Fonts\FontFile3.ttf)";
+ const auto* pdFont1 = LR"(C:\ProgramData\Fonts\FontFile1.ttf)";
+ const auto* pdFont2 = LR"(C:\ProgramData\Fonts\FontFile2.ttf)";
+ SetUpPaths({pdFont1, userFont1, userFont2});
+ std::unique_ptr<HKEY, RegCloseKeyDeleter> subKey;
+ auto lStatus = ::RegCreateKeyExW(mTestUserFontKey, L"SubKey", 0, nullptr,
+ REG_OPTION_VOLATILE, KEY_ALL_ACCESS, nullptr,
+ getter_Transfers(subKey), nullptr);
+ ASSERT_EQ(lStatus, ERROR_SUCCESS);
+ SetUpPathsInKey(subKey.get(), {pdFont2, userFont3});
+
+ // These font rules won't fit in 1 page.
+ mNumberOfStoragePages = 2;
+
+ auto& userDirFont1 = EXPECT_READONLY_EQ(userFont1).After(mWinUserFontCall);
+ auto& userDirFont2 = EXPECT_READONLY_EQ(userFont2).After(mWinUserFontCall);
+ auto& userDirFont3 =
+ EXPECT_READONLY_EQ(userFont3).After(userDirFont1, userDirFont2);
+ EXPECT_READONLY_EQ(pdFont1).After(userDirFont3);
+ EXPECT_READONLY_EQ(pdFont2).After(userDirFont3);
+
+ CreateHelperAndCallAddRules();
+}
+
TEST_F(UserFontConfigHelperTest, NonStringValueIsIgnored) {
DWORD regValue = 42;
::RegSetValueExW(mTestUserFontKey, L"Liff", 0, REG_DWORD,
diff --git a/security/sandbox/win/src/sandboxbroker/ConfigHelpers.cpp b/security/sandbox/win/src/sandboxbroker/ConfigHelpers.cpp
@@ -13,6 +13,7 @@
#include "nsExceptionHandler.h"
#include "nsStringFwd.h"
#include "nsUnicharUtils.h"
+#include "nsWindowsHelpers.h"
#include "sandbox/win/src/policy_engine_opcodes.h"
namespace mozilla {
@@ -84,8 +85,9 @@ UserFontConfigHelper::UserFontConfigHelper(const wchar_t* aUserFontKeyPath,
const nsString& aWinUserProfile,
const nsString& aLocalAppData)
: mWinUserProfile(aWinUserProfile), mLocalAppData(aLocalAppData) {
- LSTATUS lStatus = ::RegOpenKeyExW(HKEY_CURRENT_USER, aUserFontKeyPath, 0,
- KEY_QUERY_VALUE, &mUserFontKey);
+ LSTATUS lStatus =
+ ::RegOpenKeyExW(HKEY_CURRENT_USER, aUserFontKeyPath, 0,
+ KEY_QUERY_VALUE | KEY_ENUMERATE_SUB_KEYS, &mUserFontKey);
if (lStatus != ERROR_SUCCESS) {
// Ensure that mUserFontKey is null on failure.
mUserFontKey = nullptr;
@@ -98,34 +100,10 @@ UserFontConfigHelper::~UserFontConfigHelper() {
}
}
-void UserFontConfigHelper::AddRules(SizeTrackingConfig& aConfig) const {
- // Always add rule to allow access to windows user specific fonts dir first.
- nsAutoString windowsUserFontDir(mLocalAppData);
- windowsUserFontDir += uR"(\Microsoft\Windows\Fonts\*)"_ns;
- auto result = aConfig.AllowFileAccess(sandbox::FileSemantics::kAllowReadonly,
- windowsUserFontDir.getW());
- if (result != sandbox::SBOX_ALL_OK) {
- NS_ERROR("Failed to add Windows user font dir policy rule.");
- LOG_E("Failed (ResultCode %d) to add read access to: %S", result,
- windowsUserFontDir.getW());
- }
-
- // We failed to open the registry key, we can't do any more.
- if (!mUserFontKey) {
- return;
- }
-
- // We don't want the wild-card for comparisons.
- windowsUserFontDir.SetLength(windowsUserFontDir.Length() - 1);
-
- // Windows user's profile dir with trailing slash for comparisons.
- nsAutoString winUserProfile(mWinUserProfile);
- winUserProfile += L'\\';
-
- // We will add rules for fonts outside of the User's directory at the end, in
- // case we run out of space.
- Vector<nsString> nonUserDirFonts;
-
+static auto AddRulesForKey(HKEY aFontKey, const nsAString& aWindowsUserFontDir,
+ const nsAString& aWinUserProfile,
+ SizeTrackingConfig& aConfig,
+ Vector<nsString>& aNonUserDirFonts) {
for (DWORD valueIndex = 0; /* break on ERROR_NO_MORE_ITEMS */; ++valueIndex) {
DWORD keyType;
wchar_t name[1024];
@@ -135,8 +113,8 @@ void UserFontConfigHelper::AddRules(SizeTrackingConfig& aConfig) const {
// Pass 1 less wchar_t worth, in case we have to add a null.
DWORD dataSizeInBytes = sizeof(data) - sizeof(wchar_t);
LSTATUS lStatus =
- ::RegEnumValueW(mUserFontKey, valueIndex, name, &nameLength, NULL,
- &keyType, dataAsBytes, &dataSizeInBytes);
+ ::RegEnumValueW(aFontKey, valueIndex, name, &nameLength, NULL, &keyType,
+ dataAsBytes, &dataSizeInBytes);
if (lStatus == ERROR_NO_MORE_ITEMS) {
break;
}
@@ -174,35 +152,102 @@ void UserFontConfigHelper::AddRules(SizeTrackingConfig& aConfig) const {
}
// If not in the user's dir, store until the end.
- if (dataSizeInWChars < winUserProfile.Length() ||
- !winUserProfile.Equals(
- nsDependentSubstring(data, winUserProfile.Length()),
+ if (dataSizeInWChars < aWinUserProfile.Length() ||
+ !aWinUserProfile.Equals(
+ nsDependentSubstring(data, aWinUserProfile.Length()),
nsCaseInsensitiveStringComparator)) {
- (void)nonUserDirFonts.emplaceBack(data, dataSizeInWChars);
+ (void)aNonUserDirFonts.emplaceBack(data, dataSizeInWChars);
continue;
}
// Skip if in windows user font dir.
- if (dataSizeInWChars > windowsUserFontDir.Length() &&
- windowsUserFontDir.Equals(
- nsDependentSubstring(data, windowsUserFontDir.Length()),
+ if (dataSizeInWChars > aWindowsUserFontDir.Length() &&
+ aWindowsUserFontDir.Equals(
+ nsDependentSubstring(data, aWindowsUserFontDir.Length()),
nsCaseInsensitiveStringComparator)) {
continue;
}
- result =
+ auto result =
aConfig.AllowFileAccess(sandbox::FileSemantics::kAllowReadonly, data);
if (result != sandbox::SBOX_ALL_OK) {
NS_WARNING("Failed to add specific user font policy rule.");
LOG_W("Failed (ResultCode %d) to add read access to: %S", result, data);
if (result == sandbox::SBOX_ERROR_NO_SPACE) {
- CrashReporter::RecordAnnotationCString(
- CrashReporter::Annotation::UserFontRulesExhausted, "inside");
- return;
+ return result;
}
}
}
+ for (DWORD keyIndex = 0; /* break on ERROR_NO_MORE_ITEMS */; ++keyIndex) {
+ wchar_t name[1024];
+ DWORD nameLength = std::size(name);
+ LSTATUS lStatus = ::RegEnumKeyExW(aFontKey, keyIndex, name, &nameLength,
+ nullptr, nullptr, nullptr, nullptr);
+ if (lStatus == ERROR_NO_MORE_ITEMS) {
+ break;
+ }
+
+ // Skip if we failed to retrieve the key name.
+ if (lStatus != ERROR_SUCCESS) {
+ continue;
+ }
+
+ std::unique_ptr<HKEY, RegCloseKeyDeleter> subKey;
+ lStatus = ::RegOpenKeyExW(aFontKey, name, 0,
+ KEY_QUERY_VALUE | KEY_ENUMERATE_SUB_KEYS,
+ getter_Transfers(subKey));
+ // Skip if we failed to retrieve the key.
+ if (lStatus != ERROR_SUCCESS) {
+ continue;
+ }
+
+ auto result = AddRulesForKey(subKey.get(), aWindowsUserFontDir,
+ aWinUserProfile, aConfig, aNonUserDirFonts);
+ if (result == sandbox::SBOX_ERROR_NO_SPACE) {
+ return result;
+ }
+ }
+
+ return sandbox::SBOX_ALL_OK;
+}
+
+void UserFontConfigHelper::AddRules(SizeTrackingConfig& aConfig) const {
+ // Always add rule to allow access to windows user specific fonts dir first.
+ nsAutoString windowsUserFontDir(mLocalAppData);
+ windowsUserFontDir += uR"(\Microsoft\Windows\Fonts\*)"_ns;
+ auto result = aConfig.AllowFileAccess(sandbox::FileSemantics::kAllowReadonly,
+ windowsUserFontDir.getW());
+ if (result != sandbox::SBOX_ALL_OK) {
+ NS_ERROR("Failed to add Windows user font dir policy rule.");
+ LOG_E("Failed (ResultCode %d) to add read access to: %S", result,
+ windowsUserFontDir.getW());
+ }
+
+ // We failed to open the registry key, we can't do any more.
+ if (!mUserFontKey) {
+ return;
+ }
+
+ // We don't want the wild-card for comparisons.
+ windowsUserFontDir.SetLength(windowsUserFontDir.Length() - 1);
+
+ // Windows user's profile dir with trailing slash for comparisons.
+ nsAutoString winUserProfile(mWinUserProfile);
+ winUserProfile += L'\\';
+
+ // We will add rules for fonts outside of the User's directory at the end, in
+ // case we run out of space.
+ Vector<nsString> nonUserDirFonts;
+
+ result = AddRulesForKey(mUserFontKey, windowsUserFontDir, winUserProfile,
+ aConfig, nonUserDirFonts);
+ if (result == sandbox::SBOX_ERROR_NO_SPACE) {
+ CrashReporter::RecordAnnotationCString(
+ CrashReporter::Annotation::UserFontRulesExhausted, "inside");
+ return;
+ }
+
// Finally add rules for fonts outside the user's dir. These are less likely
// to have access blocked by USER_LIMITED.
for (const auto& fontPath : nonUserDirFonts) {
diff --git a/security/sandbox/win/src/sandboxbroker/ConfigHelpers.h b/security/sandbox/win/src/sandboxbroker/ConfigHelpers.h
@@ -40,7 +40,7 @@ MOZ_RAII class UserFontConfigHelper final {
const nsString& aLocalAppData);
~UserFontConfigHelper();
- void AddRules(sandboxing::SizeTrackingConfig& aPolicy) const;
+ void AddRules(sandboxing::SizeTrackingConfig& aConfig) const;
UserFontConfigHelper(const UserFontConfigHelper&) = delete;
UserFontConfigHelper& operator=(const UserFontConfigHelper&) = delete;
diff --git a/xpcom/base/nsWindowsHelpers.h b/xpcom/base/nsWindowsHelpers.h
@@ -341,6 +341,11 @@ using AutoDestroySecurityDescriptor =
mozilla::UniquePtr<SECURITY_DESCRIPTOR,
DestroyPrivateObjectSecurityDeleter>;
+struct RegCloseKeyDeleter {
+ using pointer = HKEY;
+ void operator()(HKEY aRegKey) { ::RegCloseKey(aRegKey); }
+};
+
// One caller of this function is early in startup and several others are not,
// so they have different ways of determining the two parameters. This function
// exists just so any future code that needs to determine whether the dynamic
