Enable prevent_unsafe_narrowing for //sandbox/win:sandbox

Updates remaining smaller sites to compile under
prevent_unsafe_narrowing and turns on for //sandbox/win:sandbox.

* NTSTATUS_ codes added to nt_internals.h where their definition
  would otherwise conflict with DWORD STATUS_ codes.
* casting of some pointer arithmetic results to size_t.
* size_t or DWORD for counts/nums where appropriate.

Bug: 1385207
Change-Id: Ia1474316c8a3e193c78030a2894e5e2a9d62c5cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4029619
Commit-Queue: Alex Gough <ajgo@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1073717}

sandbox/win/BUILD.gn

@@ -155,7 +155,10 @@ static_library("sandbox") {
   configs +=
       [ "//build/config/sanitizers:default_sanitizer_flags_but_coverage" ]
 
-  configs += [ "//build/config:precompiled_headers" ]
+  configs += [
+    "//build/config:precompiled_headers",
+    "//build/config/compiler:prevent_unsafe_narrowing",
+  ]
 
   public_deps = [
     ":common",

sandbox/win/src/acl.cc

@@ -76,19 +76,19 @@ SE_OBJECT_TYPE ConvertObjectType(SecurityObjectType object_type) {
 absl::optional<DWORD> GetIntegrityLevelValue(IntegrityLevel integrity_level) {
   switch (integrity_level) {
     case INTEGRITY_LEVEL_SYSTEM:
-      return SECURITY_MANDATORY_SYSTEM_RID;
+      return DWORD{SECURITY_MANDATORY_SYSTEM_RID};
     case INTEGRITY_LEVEL_HIGH:
-      return SECURITY_MANDATORY_HIGH_RID;
+      return DWORD{SECURITY_MANDATORY_HIGH_RID};
     case INTEGRITY_LEVEL_MEDIUM:
-      return SECURITY_MANDATORY_MEDIUM_RID;
+      return DWORD{SECURITY_MANDATORY_MEDIUM_RID};
     case INTEGRITY_LEVEL_MEDIUM_LOW:
-      return SECURITY_MANDATORY_MEDIUM_RID - 2048;
+      return DWORD{SECURITY_MANDATORY_MEDIUM_RID - 2048};
     case INTEGRITY_LEVEL_LOW:
-      return SECURITY_MANDATORY_LOW_RID;
+      return DWORD{SECURITY_MANDATORY_LOW_RID};
     case INTEGRITY_LEVEL_BELOW_LOW:
-      return SECURITY_MANDATORY_LOW_RID - 2048;
+      return DWORD{SECURITY_MANDATORY_LOW_RID - 2048};
     case INTEGRITY_LEVEL_UNTRUSTED:
-      return SECURITY_MANDATORY_UNTRUSTED_RID;
+      return DWORD{SECURITY_MANDATORY_UNTRUSTED_RID};
     case INTEGRITY_LEVEL_LAST:
       return absl::nullopt;
   }

sandbox/win/src/eat_resolver.cc

@@ -7,6 +7,7 @@
 #include <stddef.h>
 
 #include "base/win/pe_image.h"
+#include "sandbox/win/src/nt_internals.h"
 #include "sandbox/win/src/sandbox_nt_util.h"
 
 namespace sandbox {
@@ -26,7 +27,7 @@ NTSTATUS EatResolverThunk::Setup(const void* target_module,
     return ret;
 
   if (!eat_entry_)
-    return STATUS_INVALID_PARAMETER;
+    return NTSTATUS_INVALID_PARAMETER;
 
 #if defined(_WIN64)
   // We have two thunks, in order: the return path and the forward path.
@@ -61,7 +62,7 @@ NTSTATUS EatResolverThunk::ResolveTarget(const void* module,
                                          void** address) {
   DCHECK_NT(address);
   if (!module)
-    return STATUS_INVALID_PARAMETER;
+    return NTSTATUS_INVALID_PARAMETER;
 
   base::win::PEImage pe(module);
   if (!pe.VerifyMagic())

sandbox/win/src/handle_closer.cc

@@ -10,6 +10,7 @@
 
 #include "base/check_op.h"
 #include "base/memory/free_deleter.h"
+#include "base/numerics/checked_math.h"
 #include "base/ranges/algorithm.h"
 #include "sandbox/win/src/win_utils.h"
 
@@ -141,8 +142,8 @@ bool HandleCloser::SetupHandleList(void* buffer, size_t buffer_bytes) {
     i->first.copy(output, i->first.size());
     *(output += i->first.size()) = L'\0';
     output++;
-    list_entry->offset_to_names =
-        reinterpret_cast<char*>(output) - reinterpret_cast<char*>(list_entry);
+    list_entry->offset_to_names = base::checked_cast<size_t>(
+        reinterpret_cast<char*>(output) - reinterpret_cast<char*>(list_entry));
     list_entry->name_count = i->second.size();
 
     // Copy the handle names.
@@ -153,8 +154,8 @@ bool HandleCloser::SetupHandleList(void* buffer, size_t buffer_bytes) {
 
     // Round up to the nearest multiple of sizeof(size_t).
     output = RoundUpToWordSize(output);
-    list_entry->record_bytes =
-        reinterpret_cast<char*>(output) - reinterpret_cast<char*>(list_entry);
+    list_entry->record_bytes = static_cast<size_t>(
+        reinterpret_cast<char*>(output) - reinterpret_cast<char*>(list_entry));
   }
 
   DCHECK_EQ(reinterpret_cast<size_t>(output), reinterpret_cast<size_t>(end));

sandbox/win/src/nt_internals.h

@@ -36,6 +36,9 @@ typedef LONG NTSTATUS;
 #define STATUS_NO_TOKEN               ((NTSTATUS)0xC000007CL)
 #define STATUS_NOT_SUPPORTED          ((NTSTATUS)0xC00000BBL)
 #define STATUS_INVALID_IMAGE_HASH     ((NTSTATUS)0xC0000428L)
+
+#define NTSTATUS_INVALID_PARAMETER    ((NTSTATUS)0xC000000DL)
+#define NTSTATUS_NO_MEMORY            ((NTSTATUS)0xC0000017L)
 // clang-format on
 
 #define CURRENT_PROCESS ((HANDLE)-1)

sandbox/win/src/process_thread_dispatcher.cc

@@ -130,7 +130,7 @@ bool ThreadProcessDispatcher::CreateThread(IPCInfo* ipc,
   }
 
   HANDLE handle;
-  DWORD ret = ProcessPolicy::CreateThreadAction(
+  NTSTATUS ret = ProcessPolicy::CreateThreadAction(
       *ipc->client_info, stack_size, start_address, parameter, creation_flags,
       nullptr, &handle);
 

sandbox/win/src/process_thread_policy.cc

@@ -10,6 +10,7 @@
 #include <string>
 
 #include "base/memory/free_deleter.h"
+#include "base/win/nt_status.h"
 #include "sandbox/win/src/ipc_tags.h"
 #include "sandbox/win/src/nt_internals.h"
 #include "sandbox/win/src/policy_engine_opcodes.h"
@@ -118,7 +119,7 @@ NTSTATUS ProcessPolicy::OpenProcessTokenExAction(const ClientInfo& client_info,
   return status;
 }
 
-DWORD ProcessPolicy::CreateThreadAction(
+NTSTATUS ProcessPolicy::CreateThreadAction(
     const ClientInfo& client_info,
     const SIZE_T stack_size,
     const LPTHREAD_START_ROUTINE start_address,
@@ -131,7 +132,7 @@ DWORD ProcessPolicy::CreateThreadAction(
       ::CreateRemoteThread(client_info.process, nullptr, stack_size,
                            start_address, parameter, creation_flags, thread_id);
   if (!local_handle) {
-    return ::GetLastError();
+    return base::win::GetLastNtStatus();
   }
   if (!::DuplicateHandle(::GetCurrentProcess(), local_handle,
                          client_info.process, handle, 0, false,

sandbox/win/src/process_thread_policy.h

@@ -53,13 +53,13 @@ class ProcessPolicy {
 
   // Processes a 'CreateThread()' request from the target.
   // 'client_info' : the target process that is making the request.
-  static DWORD CreateThreadAction(const ClientInfo& client_info,
-                                  SIZE_T stack_size,
-                                  LPTHREAD_START_ROUTINE start_address,
-                                  PVOID parameter,
-                                  DWORD creation_flags,
-                                  LPDWORD thread_id,
-                                  HANDLE* handle);
+  static NTSTATUS CreateThreadAction(const ClientInfo& client_info,
+                                     SIZE_T stack_size,
+                                     LPTHREAD_START_ROUTINE start_address,
+                                     PVOID parameter,
+                                     DWORD creation_flags,
+                                     LPDWORD thread_id,
+                                     HANDLE* handle);
 };
 
 }  // namespace sandbox

sandbox/win/src/resolver.cc

@@ -7,6 +7,7 @@
 #include <stddef.h>
 
 #include "base/win/pe_image.h"
+#include "sandbox/win/src/nt_internals.h"
 #include "sandbox/win/src/sandbox_nt_util.h"
 
 namespace sandbox {
@@ -19,7 +20,7 @@ NTSTATUS ResolverThunk::Init(const void* target_module,
                              void* thunk_storage,
                              size_t storage_bytes) {
   if (!thunk_storage || 0 == storage_bytes || !target_module || !target_name)
-    return STATUS_INVALID_PARAMETER;
+    return NTSTATUS_INVALID_PARAMETER;
 
   if (storage_bytes < GetThunkSize())
     return STATUS_BUFFER_TOO_SMALL;
@@ -46,7 +47,7 @@ NTSTATUS ResolverThunk::ResolveInterceptor(const void* interceptor_module,
                                            const void** address) {
   DCHECK_NT(address);
   if (!interceptor_module)
-    return STATUS_INVALID_PARAMETER;
+    return NTSTATUS_INVALID_PARAMETER;
 
   base::win::PEImage pe(interceptor_module);
   if (!pe.VerifyMagic())

sandbox/win/src/sandbox_nt_util.cc

@@ -12,6 +12,7 @@
 #include "base/compiler_specific.h"
 #include "base/win/pe_image.h"
 #include "sandbox/win/src/internal_types.h"
+#include "sandbox/win/src/nt_internals.h"
 #include "sandbox/win/src/sandbox_factory.h"
 #include "sandbox/win/src/target_services.h"
 
@@ -283,7 +284,7 @@ NTSTATUS CopyData(void* destination, const void* source, size_t bytes) {
   __try {
     GetNtExports()->memcpy(destination, source, bytes);
   } __except (EXCEPTION_EXECUTE_HANDLER) {
-    ret = GetExceptionCode();
+    ret = (NTSTATUS)GetExceptionCode();
   }
   return ret;
 }
@@ -294,7 +295,7 @@ NTSTATUS CopyNameAndAttributes(
     size_t* out_name_len,
     uint32_t* attributes) {
   if (!InitHeap())
-    return STATUS_NO_MEMORY;
+    return NTSTATUS_NO_MEMORY;
 
   DCHECK_NT(out_name);
   DCHECK_NT(out_name_len);
@@ -326,7 +327,7 @@ NTSTATUS CopyNameAndAttributes(
       ret = STATUS_SUCCESS;
     } while (false);
   } __except (EXCEPTION_EXECUTE_HANDLER) {
-    ret = GetExceptionCode();
+    ret = (NTSTATUS)GetExceptionCode();
   }
 
   if (!NT_SUCCESS(ret) && *out_name)
@@ -542,30 +543,27 @@ UNICODE_STRING* ExtractModuleName(const UNICODE_STRING* module_path) {
   if ((!module_path) || (!module_path->Buffer))
     return nullptr;
 
-  wchar_t* sep = nullptr;
-  int start_pos = module_path->Length / sizeof(wchar_t) - 1;
-  int ix = start_pos;
-
-  for (; ix >= 0; --ix) {
-    if (module_path->Buffer[ix] == L'\\') {
-      sep = &module_path->Buffer[ix];
-      break;
+  wchar_t* start_ptr = &module_path->Buffer[0];
+  if (module_path->Length > 0) {
+    size_t last_char = module_path->Length / sizeof(wchar_t) - 1;
+    // Ends with path separator. Not a valid module name.
+    if (module_path->Buffer[last_char] == L'\\')
+      return nullptr;
+    // Search backwards for path separator.
+    for (size_t i = 0; i <= last_char; ++i) {
+      if (module_path->Buffer[last_char - i] == L'\\') {
+        start_ptr = &module_path->Buffer[last_char - i + 1];
+        break;
+      }
     }
   }
 
-  // Ends with path separator. Not a valid module name.
-  if ((ix == start_pos) && sep)
-    return nullptr;
-
-  // No path separator found. Use the entire name.
-  if (!sep) {
-    sep = &module_path->Buffer[-1];
-  }
-
-  // Add one to the size so we can null terminate the string.
-  size_t size_bytes = (start_pos - ix + 1) * sizeof(wchar_t);
+  size_t skip_bytes = reinterpret_cast<uintptr_t>(start_ptr) -
+                      reinterpret_cast<uintptr_t>(&module_path->Buffer[0]);
+  // We add a nul wchar to the buffer.
+  size_t size_bytes = module_path->Length - skip_bytes + sizeof(wchar_t);
 
-  // Based on the code above, size_bytes should always be small enough
+  // Because module_path is a UNICODE_STRING, size_bytes will be small enough
   // to make the static_cast below safe.
   DCHECK_NT(UINT16_MAX > size_bytes);
   char* str_buffer = new (NT_ALLOC) char[size_bytes + sizeof(UNICODE_STRING)];
@@ -577,7 +575,7 @@ UNICODE_STRING* ExtractModuleName(const UNICODE_STRING* module_path) {
   out_string->Length = static_cast<USHORT>(size_bytes - sizeof(wchar_t));
   out_string->MaximumLength = static_cast<USHORT>(size_bytes);
 
-  NTSTATUS ret = CopyData(out_string->Buffer, &sep[1], out_string->Length);
+  NTSTATUS ret = CopyData(out_string->Buffer, start_ptr, out_string->Length);
   if (!NT_SUCCESS(ret)) {
     operator delete(out_string, NT_ALLOC);
     return nullptr;

sandbox/win/src/sharedmem_ipc_client.cc

@@ -28,7 +28,7 @@ DWORD SignalObjectAndWaitWrapper(HANDLE object_to_signal,
       millis == INFINITE ? nullptr : &timeout);
   if (!NT_SUCCESS(status))
     return WAIT_FAILED;
-  return status;
+  return static_cast<DWORD>(status);
 }
 
 DWORD WaitForSingleObjectWrapper(HANDLE handle, DWORD millis) {
@@ -38,7 +38,7 @@ DWORD WaitForSingleObjectWrapper(HANDLE handle, DWORD millis) {
       handle, FALSE, millis == INFINITE ? nullptr : &timeout);
   if (!NT_SUCCESS(status))
     return WAIT_FAILED;
-  return status;
+  return static_cast<DWORD>(status);
 }
 
 }  // namespace
@@ -172,7 +172,8 @@ size_t SharedMemIPCClient::LockFreeChannel(bool* severe_failure) {
 // Find out which channel we are from the pointer returned by GetBuffer.
 size_t SharedMemIPCClient::ChannelIndexFromBuffer(const void* buffer) {
   ptrdiff_t d = reinterpret_cast<const char*>(buffer) - first_base_;
-  size_t num = d / kIPCChannelSize;
+  DCHECK_GE(d, 0);
+  size_t num = static_cast<size_t>(d) / kIPCChannelSize;
   DCHECK_LT(num, control_->channels_count);
   return (num);
 }

sandbox/win/src/startup_information_helper.cc

@@ -80,8 +80,8 @@ void StartupInformationHelper::AddJobToAssociate(HANDLE job_handle) {
   job_handle_list_.push_back(job_handle);
 }
 
-int StartupInformationHelper::CountAttributes() {
-  int attribute_count = 0;
+DWORD StartupInformationHelper::CountAttributes() {
+  DWORD attribute_count = 0;
   if (mitigations_[0] || mitigations_[1])
     ++attribute_count;
 

sandbox/win/src/startup_information_helper.h

@@ -64,7 +64,7 @@ class StartupInformationHelper {
   void operator=(const StartupInformationHelper&) = delete;
   StartupInformationHelper(const StartupInformationHelper&) = delete;
 
-  int CountAttributes();
+  DWORD CountAttributes();
 
   // Fields that are not passed into CreateProcessAsUserW().
   scoped_refptr<AppContainer> app_container_;