commit 098c7a3d157218dab4eed595e8f2fbe5a20a0bae
author Jack Hou <jackhou@chromium.org> Tue Oct 14 00:28:06 2014
committer Jack Hou <jackhou@chromium.org> Tue Oct 14 02:44:34 2014
tree 1daaf705713227da3f8740bc6f561a8379273fbb
parent e83f154a8bab4d8b8d6846f3f5b5e094fac61084

Update base/extractor.[cc|h] for security review.

Change-Id: I58430f3609301ec6b07610917345c958c67cc26a
Reviewed-on: https://chromium-review.googlesource.com/223041
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Jack Hou <jackhou@chromium.org>
Tested-by: Jack Hou <jackhou@chromium.org>

base/extractor.cc

diff --git a/base/extractor.cc b/base/extractor.cc
index 9e7dcaa..32fe12e 100644
--- a/base/extractor.cc
+++ b/base/extractor.cc
@@ -121,6 +121,12 @@
   if (NULL == asn1_signature_pointer) {
     return false;
   }
+  // Check that the file is long enough to contain |asn1_signature_pointer| plus
+  // some more for GetASN1SignatureLength to read the length.
+  if (static_cast<const char*>(asn1_signature_pointer) + 4 >=
+      file_buffer + file_length_) {
+    return false;
+  }
   DWORD asn1_signature_length =
     GetASN1SignatureLength(asn1_signature_pointer);
   if (0 == asn1_signature_length) {
@@ -136,6 +142,9 @@
 bool TagExtractor::InternalExtractTag(const char* file_buffer,
                                       char* tag_buffer,
                                       int* tag_buffer_len) {
+  if (tag_buffer_len == NULL) {
+    return false;
+  }
   if (!file_buffer) {
     return false;
   }
@@ -191,6 +200,9 @@
   const char* image_base = reinterpret_cast<const char*>(base);
 
   // Is this a PEF?
+  if (file_length_ < sizeof(IMAGE_DOS_HEADER)) {
+    return NULL;
+  }
   const IMAGE_DOS_HEADER* dos_header =
     reinterpret_cast<const IMAGE_DOS_HEADER *>(image_base);
   if (dos_header->e_magic != IMAGE_DOS_SIGNATURE) {
@@ -198,6 +210,9 @@
   }
 
   // Get PE header.
+  if (file_length_ < dos_header->e_lfanew + sizeof(IMAGE_NT_HEADERS)) {
+    return NULL;
+  }
   const IMAGE_NT_HEADERS* nt_headers = reinterpret_cast<const IMAGE_NT_HEADERS*>
       (image_base + dos_header->e_lfanew);
 
@@ -207,11 +222,27 @@
     return NULL;
   }
 
+  // Check that we're operating no an image of the same bitness.
+  // OptionalHeader.DataDirectory location in structure depends on the image's
+  // bitness. See IMAGE_OPTIONAL_HEADER:
+  // http://msdn.microsoft.com/en-us/library/windows/desktop/ms680339.aspx
+  if (nt_headers->FileHeader.Machine == IMAGE_FILE_MACHINE_I386) {
+#if !defined(ARCH_CPU_X86)
+    return NULL;
+#endif
+  } else if (nt_headers->FileHeader.Machine == IMAGE_FILE_MACHINE_AMD64) {
+#if !defined(ARCH_CPU_X86_64)
+    return NULL;
+#endif
+  }
   const IMAGE_DATA_DIRECTORY* idd =
     reinterpret_cast<const IMAGE_DATA_DIRECTORY *>
     (&nt_headers->
     OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_SECURITY]);
   if (idd->VirtualAddress != NULL) {
+    if (file_length_ < idd->VirtualAddress + sizeof(WIN_CERTIFICATE)) {
+      return NULL;
+    }
     return image_base + idd->VirtualAddress;
   }
   return NULL;