win: make CrashpadInfo retrievable

snapshot/win/pe_image_reader.cc

+// Copyright 2015 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "snapshot/win/pe_image_reader.h"
+
+#include "client/crashpad_info.h"
+#include "snapshot/win/process_reader_win.h"
+
+namespace crashpad {
+
+PEImageReader::PEImageReader()
+    : process_reader_(nullptr), address_(0), module_name_(), initialized_() {
+}
+
+PEImageReader::~PEImageReader() {
+}
+
+bool PEImageReader::Initialize(ProcessReaderWin* process_reader,
+                               uintptr_t address,
+                               const std::string& name) {
+  INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
+
+  process_reader_ = process_reader;
+  address_ = address;
+  module_name_ = name;
+
+  INITIALIZATION_STATE_SET_VALID(initialized_);
+  return true;
+}
+
+bool PEImageReader::GetCrashpadInfo(
+    process_types::CrashpadInfo* crashpad_info) const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+
+  IMAGE_SECTION_HEADER section;
+  if (!GetSectionByName("CPADinfo", &section))
+    return false;
+
+  if (section.Misc.VirtualSize < sizeof(process_types::CrashpadInfo)) {

[Mark Mentovai, 2015/04/29 19:37:15]

The documentation says “This field is valid only for executable images and
should be set to 0 for object files.” In this instance, executable images means
modules loaded into a process and thus includes DLLs, right? This is just a
sanity check, and there’s no need to rewrite this or add a comment as long as
the answer is “yes.”

[scottmg, 2015/04/30 03:31:31]

That's right. The same structures are used for .obj and for PEs, and "executable
images" means all of EXE, DLL, CPL, OCX, etc.

+    LOG(WARNING) << "small crashpad info section size "

[Mark Mentovai, 2015/04/29 19:37:15]

"base/logging.h"

[scottmg, 2015/04/30 03:31:32]

Done.

+                 << section.Misc.VirtualSize << ", " << module_name_;
+    return false;
+  }
+
+  uintptr_t crashpad_info_address = address_ + section.VirtualAddress;

[Mark Mentovai, 2015/04/29 19:37:14]

PE modules load completely contiguously in memory on Windows, right? You should
check that the address and end are within the range of the module. See
CheckedMachAddressRange from util/mac/checked_mach_address_range.h. (We don’t
use it in exactly this way in the Mach-O reader on Mac because Mach-O modules
can load discontiguously, but we use it for similar range checks.)

If you can know the size of the image for GetSectionByName, it’d be good to
check the reads in there to ensure that they’re in-range as well.

[scottmg, 2015/04/30 03:31:31]

Done. (I moved most of util/mac/checked_mach_address_range to util/numeric/ in
doing so.)

+  // TODO(scottmg): process_types for cross-bitness.
+  if (!process_reader_->ReadMemory(crashpad_info_address,
+                                   sizeof(process_types::CrashpadInfo),
+                                   crashpad_info)) {
+    LOG(WARNING) << "could not read crashpad info" << module_name_;

[Mark Mentovai, 2015/04/29 19:37:15]

Space before module_name. Same on line 67.

[scottmg, 2015/04/30 03:31:31]

Done.

+    return false;
+  }
+
+  if (crashpad_info->signature != CrashpadInfo::kSignature ||
+      crashpad_info->version < 1) {
+    LOG(WARNING) << "unexpected crashpad info data" << module_name_;
+    return false;
+  }
+
+  return true;
+}
+
+bool PEImageReader::GetSectionByName(const std::string& name,
+                                     IMAGE_SECTION_HEADER* section) const {
+  IMAGE_DOS_HEADER dos_header;
+  if (!process_reader_->ReadMemory(
+          address_, sizeof(IMAGE_DOS_HEADER), &dos_header)) {
+    LOG(WARNING) << "could not read dos header";

[Mark Mentovai, 2015/04/29 19:37:14]

Might want to send module_name_ to the log stream on the messages in this
method.

[scottmg, 2015/04/30 03:31:32]

Done.

+    return false;
+  }
+
+  // TODO(scottmg): This is reading a same-bitness sized structure.
+  IMAGE_NT_HEADERS nt_headers;
+  uintptr_t nt_headers_address = address_ + dos_header.e_lfanew;

[Mark Mentovai, 2015/04/29 19:37:15]

Validate dos_header.e_magic before proceeding. And after reading this, validate
nt_headers.Signature.

[scottmg, 2015/04/30 03:31:32]

Done.

+  if (!process_reader_->ReadMemory(
+          nt_headers_address, sizeof(IMAGE_NT_HEADERS), &nt_headers)) {
+    LOG(WARNING) << "could not read nt headers";
+    return false;
+  }
+
+  uintptr_t first_section_address =
+      nt_headers_address + FIELD_OFFSET(IMAGE_NT_HEADERS, OptionalHeader) +

[Mark Mentovai, 2015/04/29 19:37:14]

If this is the same as offsetof, that’s more standard (although maybe not in
Windows code?)

[scottmg, 2015/04/30 03:31:32]

Done.

+      nt_headers.FileHeader.SizeOfOptionalHeader;
+  for (DWORD i = 0; i < nt_headers.FileHeader.NumberOfSections; ++i) {
+    uintptr_t section_address =
+        first_section_address + sizeof(IMAGE_SECTION_HEADER) * i;
+    if (!process_reader_->ReadMemory(
+            section_address, sizeof(IMAGE_SECTION_HEADER), section)) {
+      LOG(WARNING) << "could not read section " << i;
+      return false;
+    }
+    if (strncmp(reinterpret_cast<const char*>(section->Name),

[Mark Mentovai, 2015/04/29 19:37:15]

<string.h>

[scottmg, 2015/04/30 03:31:32]

Done.

+                name.c_str(),

[Mark Mentovai, 2015/04/29 19:37:15]

The problem with using strncmp in this way: it also matches a prefix of a
too-long search string. Ideally this whole method would fail if name.size() >=
sizeof(section->Name) instead of trying for a partial match.

[scottmg, 2015/04/30 03:31:31]

Good point, validated `name` at function entry. > rather than >= as it's nul
padded, but not necessarily nul terminated.

+                IMAGE_SIZEOF_SHORT_NAME) == 0) {

[Mark Mentovai, 2015/04/29 19:37:14]

sizeof(section->Name) is more descriptive.

[scottmg, 2015/04/30 03:31:32]

Done.

+      return true;
+    }
+  }
+
+  return false;
+}
+
+}  // namespace crashpad