Record Code Signature of Downloaded DMG files

chrome/utility/safe_browsing/mac/udif.cc

 // Copyright 2015 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 "chrome/utility/safe_browsing/mac/udif.h"
 
 #include <CoreFoundation/CoreFoundation.h>
 #include <bzlib.h>
 #include <libkern/OSByteOrder.h>
 #include <uuid/uuid.h>
@@ skipping 330 matching lines @@
 
   DISALLOW_COPY_AND_ASSIGN(UDIFBlockChunkReadStream);
 };
 
 }  // namespace
 
 UDIFParser::UDIFParser(ReadStream* stream)
     : stream_(stream),
       partition_names_(),
       blocks_(),
-      block_size_(kSectorSize) {
-}
+      block_size_(kSectorSize),
+      signature_blob_data_(nullptr),
+      signature_blob_length_(0) {}
 
 UDIFParser::~UDIFParser() {}
 
 bool UDIFParser::Parse() {
   if (!ParseBlkx())
     return false;
 
   return true;
 }
 
+uint64_t UDIFParser::GetDmgSignatureLength() {
+  return signature_blob_length_;
+}
+
+uint8_t* UDIFParser::GetDmgSignatureData() {
+  return signature_blob_data_;
+}
+
 size_t UDIFParser::GetNumberOfPartitions() {
   return blocks_.size();
 }
 
 std::string UDIFParser::GetPartitionName(size_t part_number) {
   DCHECK_LT(part_number, partition_names_.size());
   return partition_names_[part_number];
 }
 
 std::string UDIFParser::GetPartitionType(size_t part_number) {
@@ skipping 177 matching lines @@
                  << ", SectorCount = " << chunk->sector_count
                  << ", CompressOffset = " << chunk->compressed_offset
                  << ", CompressLen = " << chunk->compressed_length;
       }
     }
 
     blocks_.push_back(std::move(block));
     partition_names_.push_back(partition_name);
   }
 
+  // The offsets in the trailer could be garbage in DMGs that aren't signed.
+  // Need a sanity check that the DMG has legit values for these fields.
+  if (trailer.code_signature_length != 0 && trailer_start > 0 &&
+      trailer.code_signature_offset + trailer.code_signature_length <=

[Robert Sesek, 2017/06/26 18:56:33]

Use safe_numerics for this math. There are examples in this file.

[mortonm, 2017/06/27 16:36:23]

Done.

+          (uint64_t)trailer_start) {

[Robert Sesek, 2017/06/26 18:56:33]

C-style casts are banned by the styleguide.

[mortonm, 2017/06/27 16:36:23]

Done.

+    uint64_t temp_size = trailer.code_signature_length;
+    uint8_t* temp_data = new uint8_t[temp_size];

[Robert Sesek, 2017/06/26 18:56:33]

This is leaked. Rather than tracking size and bytes as two separate fields, use
a std::vector. That will also fix the ownership issue.

[mortonm, 2017/06/27 16:36:23]

Done.

+
+    if (temp_data == nullptr) {

[Robert Sesek, 2017/06/26 18:56:33]

This doesn't happen in Chromium (failure to allocate is fatal).

[mortonm, 2017/06/27 16:36:23]

So do you think I should put a cap on the size of signature field specified by
file metadata for which we try to allocate a buffer? Otherwise our analysis
could be evaded by simply setting the value at offset
trailer.code_signature_length to be arbitrarily large.

On the other hand this may make the DMG un-openable on Mac, so maybe we don't
care.

[Robert Sesek, 2017/06/28 18:21:06]

I could see putting a cap on the signature_length, but I don't know what value
to use. 10K, 1M, etc? It wouldn't totally thwart analysis, though, right? Just
the outer code signature extraction.

[mortonm, 2017/06/28 23:07:08]

As of now, if the metadata in trailer.code_signature_length has a huge value
(e.g. 5GB), the underlying memory allocator for the vector will likely fail for
the call to signature_blob_.resize(trailer.code_signature_length). I'm guessing
the exception thrown (http://www.cplusplus.com/reference/vector/vector/resize/)
will crash the utility process. Of course this necessitates the file being 5GB
as well.

I'm just gonna leave the code as is for now since this evasion would require a
very large DMG, which in a soon-to-come CL would not be unpacked by the DMG
analyzer anyway.

+      // If indicated signature size was too large for allocation, still want to
+      // continue processing DMG.
+      return true;
+    }
+
+    off_t code_signature_start =
+        stream_->Seek(trailer.code_signature_offset, SEEK_SET);
+    if (code_signature_start == -1)
+      return false;
+
+    size_t bytes_read = 0;
+
+    if (!stream_->Read(temp_data, temp_size, &bytes_read)) {

[Robert Sesek, 2017/06/26 18:56:33]

… and then using a vector, you std::vector<unit8_t>::resize to the
code_signature_length and then read directly into the vector member.

[mortonm, 2017/06/27 16:36:23]

Done.

+      DLOG(ERROR) << "Failed to read raw signature bytes";
+      return false;
+    }
+
+    if (bytes_read != temp_size)
+      return false;
+
+    signature_blob_length_ = temp_size;
+    signature_blob_data_ = temp_data;
+  }
+
   return true;
 }
 
 namespace {
 
 UDIFPartitionReadStream::UDIFPartitionReadStream(
     ReadStream* stream,
     uint16_t block_size,
     const UDIFBlock* partition_block)
     : stream_(stream),
@@ skipping 295 matching lines @@
                 << chunk_->compressed_offset;
     return false;
   }
   return true;
 }
 
 }  // namespace
 
 }  // namespace dmg
 }  // namespace safe_browsing