Mac Archive Type Sniffing

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),
+      trailer_(new UDIFResourceFile),
+      trailer_successfully_parsed_(false) {}
 
 UDIFParser::~UDIFParser() {}
 
-bool UDIFParser::Parse() {
-  if (!ParseBlkx())
+bool UDIFParser::ParseTrailer() {
+  // Makes sure that trailer is only ever parsed single time during execution of
+  // constuctor, so position of ReadStream is not moved while parsing blocks.
+  if (trailer_successfully_parsed_)
+    return true;
+
+  if (trailer_.get() == nullptr)

[Robert Sesek, 2017/06/12 20:52:34]

This will never happen (Chromium terminates on OOM). But you could lazily
allocate the trailer_ member and use the presence of the object to indicate
whether or not the trailer has been parsed.

[mortonm, 2017/06/12 21:39:49]

I removed the null pointer check, but unless anyone has a strong preference will
leave the UDIFResourceFile struct initialization the way it is, since I think
that's a little more clear.

     return false;
 
+  off_t trailer_start = stream_->Seek(-sizeof(*trailer_), SEEK_END);
+  if (trailer_start == -1)
+    return false;
+
+  if (!stream_->ReadType(trailer_.get())) {
+    DLOG(ERROR) << "Failed to read UDIFResourceFile";
+    return false;
+  }
+  ConvertBigEndian(trailer_.get());
+
+  if (trailer_->signature != trailer_->kSignature) {
+    DLOG(ERROR) << "blkx signature does not match, is 0x" << std::hex
+                << trailer_->signature;
+    return false;
+  }
+  if (trailer_->version != trailer_->kVersion) {
+    DLOG(ERROR) << "blkx version does not match, is " << trailer_->version;
+    return false;
+  }
+
+  auto plist_end = base::CheckedNumeric<size_t>(trailer_->plist_offset) +
+                   trailer_->plist_length;
+  if (!plist_end.IsValid() ||
+      plist_end.ValueOrDie() > base::checked_cast<size_t>(trailer_start)) {
+    DLOG(ERROR) << "blkx plist extends past UDIF trailer";
+    return false;
+  }
+
   return true;
 }
 
+bool UDIFParser::Parse() {
+  // First parses trailer and then parses blocks.
+  trailer_successfully_parsed_ = ParseTrailer();
+  return trailer_successfully_parsed_ && ParseBlkx();
+}
+
 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 27 matching lines @@
 }
 
 std::unique_ptr<ReadStream> UDIFParser::GetPartitionReadStream(
     size_t part_number) {
   DCHECK_LT(part_number, blocks_.size());
   return base::MakeUnique<UDIFPartitionReadStream>(stream_, block_size_,
                                                    blocks_[part_number].get());
 }
 
 bool UDIFParser::ParseBlkx() {
-  UDIFResourceFile trailer;
-  off_t trailer_start = stream_->Seek(-sizeof(trailer), SEEK_END);
-  if (trailer_start == -1)
+  if (!trailer_successfully_parsed_)
     return false;
 
-  if (!stream_->ReadType(&trailer)) {
-    DLOG(ERROR) << "Failed to read UDIFResourceFile";
-    return false;
-  }
-  ConvertBigEndian(&trailer);
+  std::vector<uint8_t> plist_bytes(trailer_->plist_length, 0);
 
-  if (trailer.signature != trailer.kSignature) {
-    DLOG(ERROR) << "blkx signature does not match, is 0x"
-                << std::hex << trailer.signature;
-    return false;
-  }
-  if (trailer.version != trailer.kVersion) {
-    DLOG(ERROR) << "blkx version does not match, is " << trailer.version;
-    return false;
-  }
-
-  auto plist_end = base::CheckedNumeric<size_t>(trailer.plist_offset) +
-                   trailer.plist_length;
-  if (!plist_end.IsValid() ||
-      plist_end.ValueOrDie() > base::checked_cast<size_t>(trailer_start)) {
-    DLOG(ERROR) << "blkx plist extends past UDIF trailer";
-    return false;
-  }
-
-  std::vector<uint8_t> plist_bytes(trailer.plist_length, 0);
-
-  if (stream_->Seek(trailer.plist_offset, SEEK_SET) == -1)
+  if (stream_->Seek(trailer_->plist_offset, SEEK_SET) == -1)
     return false;
 
-  if (!stream_->ReadExact(&plist_bytes[0], trailer.plist_length)) {
+  if (!stream_->ReadExact(&plist_bytes[0], trailer_->plist_length)) {
     DLOG(ERROR) << "Failed to read blkx plist data";
     return false;
   }
 
   base::ScopedCFTypeRef<CFDataRef> plist_data(
       CFDataCreateWithBytesNoCopy(kCFAllocatorDefault,
           &plist_bytes[0], plist_bytes.size(), kCFAllocatorNull));
   if (!plist_data) {
     DLOG(ERROR) << "Failed to create data from bytes";
     return false;
@@ skipping 410 matching lines @@
                 << chunk_->compressed_offset;
     return false;
   }
   return true;
 }
 
 }  // namespace
 
 }  // namespace dmg
 }  // namespace safe_browsing