Make sure Chrome_elf.dll imports are correct and that it the first import of chrome.exe

chrome_elf/elf_imports_unittest.cc

Index: chrome_elf/elf_imports_unittest.cc
diff --git a/chrome_elf/elf_imports_unittest.cc b/chrome_elf/elf_imports_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ea874544b36020ce6cbdf05c4ec36d81826417cc
--- /dev/null
+++ b/chrome_elf/elf_imports_unittest.cc
@@ -0,0 +1,72 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <stdint.h>
+#include <windows.h>
+
+#include <algorithm>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/scoped_native_library.h"
+#include "base/strings/string_util.h"
+#include "base/win/pe_image.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+const char kKernel32[] = "KERNEL32.dll";
+const char kMSVC[] = "MSVC*";
+
+class ELFImportsTest : public testing::Test {
+ protected:
+  void GetImports(wchar_t* module_name, std::vector<std::string>* imports) {
+    HMODULE module_handle = ::LoadLibrary(module_name);

[Sigurður Ásgeirsson, 2013/12/17 14:52:37]

You should be able to do this without executing any code in the image by loading
it as a data file, see LoadLibraryEx.

[Cait (Slow), 2013/12/18 20:59:03]

Done.

+    base::ScopedNativeLibrary library(module_handle);

[robertshield, 2013/12/17 04:02:03]

This looks odd, but I guess it's done since SNL's interface doesn't actually
expose the module handle short of calling Release? bummer.

[Cait (Slow), 2013/12/18 20:59:03]

Not needed anymore.

+
+    if (!module_handle || !imports)

[robertshield, 2013/12/17 04:02:03]

checking imports first would allow you to abort early before loading the library

[Cait (Slow), 2013/12/18 20:59:03]

Done.

+      return;
+
+    base::win::PEImage pe_image(module_handle);

[Sigurður Ásgeirsson, 2013/12/17 14:52:37]

PeImageAsDatafile, given the above.

[Cait (Slow), 2013/12/18 20:59:03]

Done.

+    PIMAGE_IMPORT_DESCRIPTOR image_descriptor = pe_image.GetFirstImportChunk();
+
+    // The list of Import Address Tables ends with an empty {0} descriptor.
+    while (image_descriptor->Characteristics) {

[Sigurður Ásgeirsson, 2013/12/17 14:52:37]

You can also do this by using one of the PEImage enumeration functions, see
PEImage::EnumImportChunks. 
This has the advantage that it'll do the image parsing for you - it's better to
concentrate that sort of stuff in one place IMHO.

[Cait (Slow), 2013/12/18 20:59:03]

Done.

+      DWORD name =
+          reinterpret_cast<DWORD>(module_handle) + image_descriptor->Name;

[Sigurður Ásgeirsson, 2013/12/17 14:52:37]

This is what RVAToAddr does.

[Cait (Slow), 2013/12/18 20:59:03]

Not needed anymore with EnumImportChunks

+      imports->push_back(reinterpret_cast<char*>(name));
+      image_descriptor += sizeof(image_descriptor) / sizeof(uintptr_t);
+    }
+  }
+};
+
+TEST_F(ELFImportsTest, ChromeElfSanityCheck) {
+  std::vector<std::string> elf_imports;
+  GetImports(L"chrome_elf.dll", &elf_imports);
+
+  // Check that ELF has imports.
+  ASSERT_TRUE(elf_imports.size() > 0);

[Sigurður Ásgeirsson, 2013/12/17 14:52:37]

ASSERT_LE or GE

[Cait (Slow), 2013/12/18 20:59:03]

Done.

+
+  std::vector<std::string>::iterator it = elf_imports.begin();
+
+  // Make sure all of ELF's imports are in are valid imports list.
+  for (; it != elf_imports.end(); it++) {
+    if (!MatchPattern(*it, kMSVC) && !MatchPattern(*it, kKernel32))

[robertshield, 2013/12/17 04:02:03]

elf is now (or about to) import a few functions from advapi32 as well, could we
add these here?

also, can the patterns that need to be matched against be expressed as a list to
make it easier to add / remove new ones?

[Cait (Slow), 2013/12/18 20:59:03]

Done.

+      FAIL() << "Illegal import in chrome_elf.dll.";
+  }
+}
+
+TEST_F(ELFImportsTest, ChromeExeSanityCheck) {
+  std::vector<std::string> exe_imports;
+  GetImports(L"chrome.exe", &exe_imports);

[Sigurður Ásgeirsson, 2013/12/17 14:52:37]

oooh, I don't know what happens when you LoadLibrary an EXE. Better to load it
as a data file.

[Cait (Slow), 2013/12/18 20:59:03]

Switched to loading the file using CreateFile (as is done here:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/installer/t...)
since LOAD_LIBRARY_AS_DATAFILE gives a messed up module handle.

+
+  // Check that chrome.exe has imports.
+  ASSERT_TRUE(exe_imports.size() > 0);

[Sigurður Ásgeirsson, 2013/12/17 14:52:37]

ASSERT_GE or LE?

[Cait (Slow), 2013/12/18 20:59:03]

Done.

+
+  // Chrome.exe's first import must be ELF.
+  EXPECT_EQ("chrome_elf.dll", exe_imports[0]);
+}
+
+}  // namespace