From ad9887ee0dc016a0131f0518cf602bb8dd380799 Mon Sep 17 00:00:00 2001
From: Mark Mentovai <mark@chromium.org>
Date: Wed, 28 Oct 2015 16:42:34 -0400
Subject: [PATCH] win: Don't attempt to read a nonexistent
 IMAGE_DIRECTORY_ENTRY_DEBUG

BUG=crashpad:1
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1411123011 .
---
 snapshot/win/pe_image_reader.cc | 28 ++++++++++++++++++++--------
 snapshot/win/pe_image_reader.h  | 19 +++++++++++++++++--
 2 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/snapshot/win/pe_image_reader.cc b/snapshot/win/pe_image_reader.cc
index 7268358a..6f5bb610 100644
--- a/snapshot/win/pe_image_reader.cc
+++ b/snapshot/win/pe_image_reader.cc
@@ -143,11 +143,20 @@ template <class NtHeadersType>
 bool PEImageReader::ReadDebugDirectoryInformation(UUID* uuid,
                                                   DWORD* age,
                                                   std::string* pdbname) const {
-  WinVMAddress nt_headers_address;
   NtHeadersType nt_headers;
-  if (!ReadNtHeaders(&nt_headers_address, &nt_headers))
+  if (!ReadNtHeaders(&nt_headers, nullptr))
     return false;
 
+  if (nt_headers.FileHeader.SizeOfOptionalHeader <
+          offsetof(decltype(nt_headers.OptionalHeader),
+                   DataDirectory[IMAGE_DIRECTORY_ENTRY_DEBUG]) +
+              sizeof(nt_headers.OptionalHeader
+                         .DataDirectory[IMAGE_DIRECTORY_ENTRY_DEBUG]) ||
+      nt_headers.OptionalHeader.NumberOfRvaAndSizes <=
+          IMAGE_DIRECTORY_ENTRY_DEBUG) {
+    return false;
+  }
+
   const IMAGE_DATA_DIRECTORY& data_directory =
       nt_headers.OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_DEBUG];
   if (data_directory.VirtualAddress == 0 || data_directory.Size == 0)
@@ -202,8 +211,8 @@ bool PEImageReader::ReadDebugDirectoryInformation(UUID* uuid,
 }
 
 template <class NtHeadersType>
-bool PEImageReader::ReadNtHeaders(WinVMAddress* nt_headers_address,
-                                  NtHeadersType* nt_headers) const {
+bool PEImageReader::ReadNtHeaders(NtHeadersType* nt_headers,
+                                  WinVMAddress* nt_headers_address) const {
   IMAGE_DOS_HEADER dos_header;
   if (!CheckedReadMemory(Address(), sizeof(IMAGE_DOS_HEADER), &dos_header)) {
     LOG(WARNING) << "could not read dos header of " << module_name_;
@@ -215,9 +224,9 @@ bool PEImageReader::ReadNtHeaders(WinVMAddress* nt_headers_address,
     return false;
   }
 
-  *nt_headers_address = Address() + dos_header.e_lfanew;
+  WinVMAddress local_nt_headers_address = Address() + dos_header.e_lfanew;
   if (!CheckedReadMemory(
-          *nt_headers_address, sizeof(NtHeadersType), nt_headers)) {
+          local_nt_headers_address, sizeof(NtHeadersType), nt_headers)) {
     LOG(WARNING) << "could not read nt headers of " << module_name_;
     return false;
   }
@@ -227,6 +236,9 @@ bool PEImageReader::ReadNtHeaders(WinVMAddress* nt_headers_address,
     return false;
   }
 
+  if (nt_headers_address)
+    *nt_headers_address = local_nt_headers_address;
+
   return true;
 }
 
@@ -238,9 +250,9 @@ bool PEImageReader::GetSectionByName(const std::string& name,
     return false;
   }
 
-  WinVMAddress nt_headers_address;
   NtHeadersType nt_headers;
-  if (!ReadNtHeaders(&nt_headers_address, &nt_headers))
+  WinVMAddress nt_headers_address;
+  if (!ReadNtHeaders(&nt_headers, &nt_headers_address))
     return false;
 
   WinVMAddress first_section_address =
diff --git a/snapshot/win/pe_image_reader.h b/snapshot/win/pe_image_reader.h
index 73a9e382..f6364d99 100644
--- a/snapshot/win/pe_image_reader.h
+++ b/snapshot/win/pe_image_reader.h
@@ -109,15 +109,30 @@ class PEImageReader {
  private:
   //! \brief Implementation helper for DebugDirectoryInformation() templated by
   //!     `IMAGE_NT_HEADERS` type for different bitnesses.
+  //!
+  //! \return `true` on success, with the parameters set appropriately. `false`
+  //!     on failure. This method may return `false` without logging anything in
+  //!     the case of a module that does not contain relevant debugging
+  //!     information but is otherwise properly structured.
   template <class NtHeadersType>
   bool ReadDebugDirectoryInformation(UUID* uuid,
                                      DWORD* age,
                                      std::string* pdbname) const;
 
   //! \brief Reads the `IMAGE_NT_HEADERS` from the beginning of the image.
+  //!
+  //! \param[out] nt_headers The contents of the templated NtHeadersType
+  //!     structure read from the remote process.
+  //! \param[out] nt_headers_address The address of the templated NtHeadersType
+  //!     structure in the remote process’ address space. If this information is
+  //!     not needed, this parameter may be `nullptr`.
+  //!
+  //! \return `true` on success, with \a nt_headers and optionally \a
+  //!     nt_headers_address set appropriately. `false` on failure, with a
+  //!     message logged.
   template <class NtHeadersType>
-  bool ReadNtHeaders(WinVMAddress* nt_header_address,
-                     NtHeadersType* nt_headers) const;
+  bool ReadNtHeaders(NtHeadersType* nt_headers,
+                     WinVMAddress* nt_headers_address) const;
 
   //! \brief Finds a given section by name in the image.
   template <class NtHeadersType>