Fix CFI bad casts in ServiceResolverTest.

Change the class hierarchy used in this test in order to
eliminate two static_casts that are invalid under CFI.

While here, change the tests to use std::unique_ptr in order
to simplify memory management (and fix a memory leak).

Bug: 507755
Change-Id: Ibb077fee1f4286d46c9da3fbe0ce296f30e11c96
Reviewed-on: https://chromium-review.googlesource.com/818531Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523649}

sandbox/win/src/service_resolver_unittest.cc

@@ -19,17 +19,30 @@
 
 namespace {
 
+class ResolverThunkTest {
+ public:
+  virtual ~ResolverThunkTest() {}
+
+  virtual sandbox::ServiceResolverThunk* resolver() = 0;
+
+  // Sets the interception target to the desired address.
+  void set_target(void* target) { fake_target_ = target; }
+
+ protected:
+  // Holds the address of the fake target.
+  void* fake_target_;
+};
+
 // This is the concrete resolver used to perform service-call type functions
 // inside ntdll.dll.
 template <typename T>
-class ResolverThunkTest : public T {
+class ResolverThunkTestImpl : public T, public ResolverThunkTest {
  public:
   // The service resolver needs a child process to write to.
-  explicit ResolverThunkTest(bool relaxed)
+  explicit ResolverThunkTestImpl(bool relaxed)
       : T(::GetCurrentProcess(), relaxed) {}
 
-  // Sets the interception target to the desired address.
-  void set_target(void* target) { fake_target_ = target; }
+  sandbox::ServiceResolverThunk* resolver() { return this; }
 
  protected:
   // Overrides Resolver::Init
@@ -51,20 +64,18 @@ class ResolverThunkTest : public T {
     return ret;
   };
 
- private:
-  // Holds the address of the fake target.
-  void* fake_target_;
-
-  DISALLOW_COPY_AND_ASSIGN(ResolverThunkTest);
+  DISALLOW_COPY_AND_ASSIGN(ResolverThunkTestImpl);
 };
 
-typedef ResolverThunkTest<sandbox::ServiceResolverThunk> WinXpResolverTest;
+typedef ResolverThunkTestImpl<sandbox::ServiceResolverThunk> WinXpResolverTest;
 
 #if !defined(_WIN64)
-typedef ResolverThunkTest<sandbox::Win8ResolverThunk> Win8ResolverTest;
-typedef ResolverThunkTest<sandbox::Wow64ResolverThunk> Wow64ResolverTest;
-typedef ResolverThunkTest<sandbox::Wow64W8ResolverThunk> Wow64W8ResolverTest;
-typedef ResolverThunkTest<sandbox::Wow64W10ResolverThunk> Wow64W10ResolverTest;
+typedef ResolverThunkTestImpl<sandbox::Win8ResolverThunk> Win8ResolverTest;
+typedef ResolverThunkTestImpl<sandbox::Wow64ResolverThunk> Wow64ResolverTest;
+typedef ResolverThunkTestImpl<sandbox::Wow64W8ResolverThunk>
+    Wow64W8ResolverTest;
+typedef ResolverThunkTestImpl<sandbox::Wow64W10ResolverThunk>
+    Wow64W10ResolverTest;
 #endif
 
 const BYTE kJump32 = 0xE9;
@@ -92,7 +103,7 @@ void CheckJump(void* source, void* target) {
 
 NTSTATUS PatchNtdllWithResolver(const char* function,
                                 bool relaxed,
-                                sandbox::ServiceResolverThunk* resolver) {
+                                ResolverThunkTest* thunk_test) {
   HMODULE ntdll_base = ::GetModuleHandle(L"ntdll.dll");
   EXPECT_TRUE(ntdll_base);
 
@@ -104,8 +115,9 @@ NTSTATUS PatchNtdllWithResolver(const char* function,
   BYTE service[50];
   memcpy(service, target, sizeof(service));
 
-  static_cast<WinXpResolverTest*>(resolver)->set_target(service);
+  thunk_test->set_target(service);
 
+  sandbox::ServiceResolverThunk* resolver = thunk_test->resolver();
   // Any pointer will do as an interception_entry_point
   void* function_entry = resolver;
   size_t thunk_size = resolver->GetThunkSize();
@@ -134,32 +146,29 @@ NTSTATUS PatchNtdllWithResolver(const char* function,
   return ret;
 }
 
-sandbox::ServiceResolverThunk* GetTestResolver(bool relaxed) {
+std::unique_ptr<ResolverThunkTest> GetTestResolver(bool relaxed) {
 #if defined(_WIN64)
-  return new WinXpResolverTest(relaxed);
+  return std::make_unique<WinXpResolverTest>(relaxed);
 #else
   base::win::OSInfo* os_info = base::win::OSInfo::GetInstance();
   if (os_info->wow64_status() == base::win::OSInfo::WOW64_ENABLED) {
     if (os_info->version() >= base::win::VERSION_WIN10)
-      return new Wow64W10ResolverTest(relaxed);
+      return std::make_unique<Wow64W10ResolverTest>(relaxed);
     if (os_info->version() >= base::win::VERSION_WIN8)
-      return new Wow64W8ResolverTest(relaxed);
-    return new Wow64ResolverTest(relaxed);
+      return std::make_unique<Wow64W8ResolverTest>(relaxed);
+    return std::make_unique<Wow64ResolverTest>(relaxed);
   }
 
   if (os_info->version() >= base::win::VERSION_WIN8)
-    return new Win8ResolverTest(relaxed);
+    return std::make_unique<Win8ResolverTest>(relaxed);
 
-  return new WinXpResolverTest(relaxed);
+  return std::make_unique<WinXpResolverTest>(relaxed);
 #endif
 }
 
 NTSTATUS PatchNtdll(const char* function, bool relaxed) {
-  sandbox::ServiceResolverThunk* resolver = GetTestResolver(relaxed);
-
-  NTSTATUS ret = PatchNtdllWithResolver(function, relaxed, resolver);
-  delete resolver;
-  return ret;
+  std::unique_ptr<ResolverThunkTest> thunk_test = GetTestResolver(relaxed);
+  return PatchNtdllWithResolver(function, relaxed, thunk_test.get());
 }
 
 TEST(ServiceResolverTest, PatchesServices) {
@@ -210,27 +219,26 @@ TEST(ServiceResolverTest, PatchesPatchedServices) {
 TEST(ServiceResolverTest, MultiplePatchedServices) {
 // We don't support "relaxed mode" for Win64 apps.
 #if !defined(_WIN64)
-  sandbox::ServiceResolverThunk* resolver = GetTestResolver(true);
-  NTSTATUS ret = PatchNtdllWithResolver("NtClose", true, resolver);
+  std::unique_ptr<ResolverThunkTest> thunk_test = GetTestResolver(true);
+  NTSTATUS ret = PatchNtdllWithResolver("NtClose", true, thunk_test.get());
   EXPECT_EQ(STATUS_SUCCESS, ret) << "NtClose, last error: " << ::GetLastError();
 
-  ret = PatchNtdllWithResolver("NtCreateFile", true, resolver);
+  ret = PatchNtdllWithResolver("NtCreateFile", true, thunk_test.get());
   EXPECT_EQ(STATUS_SUCCESS, ret)
       << "NtCreateFile, last error: " << ::GetLastError();
 
-  ret = PatchNtdllWithResolver("NtCreateMutant", true, resolver);
+  ret = PatchNtdllWithResolver("NtCreateMutant", true, thunk_test.get());
   EXPECT_EQ(STATUS_SUCCESS, ret)
       << "NtCreateMutant, last error: " << ::GetLastError();
 
-  ret = PatchNtdllWithResolver("NtMapViewOfSection", true, resolver);
+  ret = PatchNtdllWithResolver("NtMapViewOfSection", true, thunk_test.get());
   EXPECT_EQ(STATUS_SUCCESS, ret)
       << "NtMapViewOfSection, last error: " << ::GetLastError();
-  delete resolver;
 #endif
 }
 
 TEST(ServiceResolverTest, LocalPatchesAllowed) {
-  sandbox::ServiceResolverThunk* resolver = GetTestResolver(true);
+  std::unique_ptr<ResolverThunkTest> thunk_test = GetTestResolver(true);
 
   HMODULE ntdll_base = ::GetModuleHandle(L"ntdll.dll");
   ASSERT_TRUE(ntdll_base);
@@ -242,8 +250,9 @@ TEST(ServiceResolverTest, LocalPatchesAllowed) {
 
   BYTE service[50];
   memcpy(service, target, sizeof(service));
-  static_cast<WinXpResolverTest*>(resolver)->set_target(service);
+  thunk_test->set_target(service);
 
+  sandbox::ServiceResolverThunk* resolver = thunk_test->resolver();
   // Any pointer will do as an interception_entry_point
   void* function_entry = resolver;
   size_t thunk_size = resolver->GetThunkSize();