CreateThread interception, to use CreateRemoteThread

sandbox/win/src/process_thread_policy.cc

Index: sandbox/win/src/process_thread_policy.cc
diff --git a/sandbox/win/src/process_thread_policy.cc b/sandbox/win/src/process_thread_policy.cc
index b58a287ed2d5937917b480fd30bbfe4c7acc5d97..dd97ae47307e3b5f068a26d95655a2e434864654 100644
--- a/sandbox/win/src/process_thread_policy.cc
+++ b/sandbox/win/src/process_thread_policy.cc
@@ -97,6 +97,9 @@ bool ProcessPolicy::GenerateRules(const wchar_t* name,
   if (!policy->AddRule(IPC_CREATEPROCESSW_TAG, process.get())) {
     return false;
   }
+  if (!policy->AddRule(IPC_CREATETHREAD_TAG, process.get())) {
+    return false;
+  }
   return true;
 }
 
@@ -236,4 +239,38 @@ DWORD ProcessPolicy::CreateProcessWAction(EvalResult eval_result,
   return ERROR_SUCCESS;
 }
 
+DWORD ProcessPolicy::CreateThreadAction(EvalResult eval_result,
+  const ClientInfo& client_info,
+  const LPSECURITY_ATTRIBUTES thread_attributes,
+  const SIZE_T stack_size,
+  const LPTHREAD_START_ROUTINE start_address,
+  const PVOID parameter,
+  const DWORD creation_flags,
+  LPDWORD thread_id,
+  HANDLE *handle) {
+
+  // The only action supported is ASK_BROKER which means create the process.
+  if (GIVE_ALLACCESS != eval_result && GIVE_READONLY != eval_result) {

[Will Harris, 2015/09/04 02:41:01]

eval_result will always be GIVE_ALLACCESS

I wonder if we even need to pass the eval through unless we plan to change this
to allow policy based blocking for CreateThread

[liamjm (20p), 2015/09/04 21:30:39]

As discussed, this is more of a stylistic thing - do we match the existing
functions, or remove the redundant checks.
Will discuss with jschuh@ when he's back.

+    return ERROR_ACCESS_DENIED;
+  }
+  HANDLE local_handle = CreateRemoteThread(
+    client_info.process,
+    thread_attributes,
+    stack_size,
+    start_address,
+    parameter,
+    creation_flags,
+    thread_id);
+  if (!local_handle) {
+    return ERROR_ACCESS_DENIED;
+  }
+  if (!::DuplicateHandle(::GetCurrentProcess(), local_handle,
+    client_info.process, handle, 0, FALSE,
+    DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) {
+    // TODO(liamjm): what do we do on error with original handle??

[Will Harris, 2015/09/04 02:41:01]

I don't know but we probably need a DCHECK here

[liamjm (20p), 2015/09/04 21:30:39]

Added a DCHECK()

+    return ERROR_ACCESS_DENIED;
+  }
+  return ERROR_SUCCESS;
+}
+
 }  // namespace sandbox