tor-browser

commit bb7b5955cc0ddc9234cec40dd6b4d8932106170d
parent 567eaa35d8bc0f0f08a0db19177a613a47cc8ec3
Author: Yannis Juglaret <yjuglaret@mozilla.com>
Date:   Mon, 13 Oct 2025 08:06:46 +0000

Bug 1992678 - Use a more resilient initialization for sandboxed children. r=gstoll

If commiting RWX memory fails, we fall back to RW instead, which is
enough to initialize the thunks. In that case, we make sure that the
later transition to RX permission succeeds.

Differential Revision: https://phabricator.services.mozilla.com/D268123

Diffstat:
A
security/sandbox/chromium-shim/patches/50_more_resilient_initialization_for_sandboxed_children.patch
 | 
45
+++++++++++++++++++++++++++++++++++++++++++++
M
security/sandbox/chromium/sandbox/win/src/interception.cc
 | 
16
+++++++++++++---

2 files changed, 58 insertions(+), 3 deletions(-)
diff --git a/security/sandbox/chromium-shim/patches/50_more_resilient_initialization_for_sandboxed_children.patch b/security/sandbox/chromium-shim/patches/50_more_resilient_initialization_for_sandboxed_children.patch
@@ -0,0 +1,45 @@
+Use a more resilient initialization for sandboxed children.
+
+If commiting RWX memory fails, we fall back to RW instead, which is
+enough to initialize the thunks. In that case, we make sure that the
+later transition to RX permission succeeds.
+---
+ sandbox/win/src/interception.cc | 16 +++++++++++++---
+ 1 file changed, 13 insertions(+), 3 deletions(-)
+
+diff --git a/security/sandbox/chromium/sandbox/win/src/interception.cc b/security/sandbox/chromium/sandbox/win/src/interception.cc
+index 04f32b099646..c78d56e17abc 100644
+--- a/security/sandbox/chromium/sandbox/win/src/interception.cc
++++ b/security/sandbox/chromium/sandbox/win/src/interception.cc
+@@ -386,9 +386,16 @@ ResultCode InterceptionManager::PatchNtdll(bool hot_patch_needed) {
+ 
+   // Make an aligned, padded allocation, and move the pointer to our chunk.
+   size_t thunk_bytes_padded = base::bits::AlignUp(thunk_bytes, kPageSize);
+-  thunk_base = reinterpret_cast<BYTE*>(
++  BYTE* thunk_base_rwx = reinterpret_cast<BYTE*>(
+       ::VirtualAllocEx(child, thunk_base, thunk_bytes_padded, MEM_COMMIT,
+                        PAGE_EXECUTE_READWRITE));
++  bool rwx_commit_succeeded = static_cast<bool>(thunk_base_rwx);
++  if (rwx_commit_succeeded) {
++    thunk_base = thunk_base_rwx;
++  } else {
++    thunk_base = reinterpret_cast<BYTE*>(::VirtualAllocEx(
++        child, thunk_base, thunk_bytes_padded, MEM_COMMIT, PAGE_READWRITE));
++  }
+   CHECK(thunk_base);  // If this fails we'd crash anyway on an invalid access.
+   DllInterceptionData* thunks =
+       reinterpret_cast<DllInterceptionData*>(thunk_base + thunk_offset);
+@@ -418,8 +425,11 @@ ResultCode InterceptionManager::PatchNtdll(bool hot_patch_needed) {
+ 
+   // Attempt to protect all the thunks, but ignore failure
+   DWORD old_protection;
+-  ::VirtualProtectEx(child, thunks, thunk_bytes, PAGE_EXECUTE_READ,
+-                     &old_protection);
++  bool rx_update_succeeded = static_cast<bool>(::VirtualProtectEx(
++      child, thunks, thunk_bytes, PAGE_EXECUTE_READ, &old_protection));
++
++  // We need the EXECUTE permission on the thunks, one way or the other
++  CHECK(rwx_commit_succeeded || rx_update_succeeded);
+ 
+   ResultCode ret =
+       child_->TransferVariable("g_originals", g_originals, sizeof(g_originals));
diff --git a/security/sandbox/chromium/sandbox/win/src/interception.cc b/security/sandbox/chromium/sandbox/win/src/interception.cc
@@ -386,9 +386,16 @@ ResultCode InterceptionManager::PatchNtdll(bool hot_patch_needed) {
 
   // Make an aligned, padded allocation, and move the pointer to our chunk.
   size_t thunk_bytes_padded = base::bits::AlignUp(thunk_bytes, kPageSize);
-  thunk_base = reinterpret_cast<BYTE*>(
+  BYTE* thunk_base_rwx = reinterpret_cast<BYTE*>(
       ::VirtualAllocEx(child, thunk_base, thunk_bytes_padded, MEM_COMMIT,
                        PAGE_EXECUTE_READWRITE));
+  bool rwx_commit_succeeded = static_cast<bool>(thunk_base_rwx);
+  if (rwx_commit_succeeded) {
+    thunk_base = thunk_base_rwx;
+  } else {
+    thunk_base = reinterpret_cast<BYTE*>(::VirtualAllocEx(
+        child, thunk_base, thunk_bytes_padded, MEM_COMMIT, PAGE_READWRITE));
+  }
   CHECK(thunk_base);  // If this fails we'd crash anyway on an invalid access.
   DllInterceptionData* thunks =
       reinterpret_cast<DllInterceptionData*>(thunk_base + thunk_offset);
@@ -418,8 +425,11 @@ ResultCode InterceptionManager::PatchNtdll(bool hot_patch_needed) {
 
   // Attempt to protect all the thunks, but ignore failure
   DWORD old_protection;
-  ::VirtualProtectEx(child, thunks, thunk_bytes, PAGE_EXECUTE_READ,
-                     &old_protection);
+  bool rx_update_succeeded = static_cast<bool>(::VirtualProtectEx(
+      child, thunks, thunk_bytes, PAGE_EXECUTE_READ, &old_protection));
+
+  // We need the EXECUTE permission on the thunks, one way or the other
+  CHECK(rwx_commit_succeeded || rx_update_succeeded);
 
   ResultCode ret =
       child_->TransferVariable("g_originals", g_originals, sizeof(g_originals));