Make the deps_log parser report corrupted files instead of crashing.

src/deps_log.cc

@@ -15,8 +15,9 @@
 #include "deps_log.h"
 
 #include <assert.h>
-#include <stdio.h>
 #include <errno.h>
+#include <stdint.h>
+#include <stdio.h>
 #include <string.h>
 #ifndef _WIN32
 #include <unistd.h>
@@ -34,12 +35,14 @@ using namespace std;
 
 // The version is stored as 4 bytes after the signature and also serves as a
 // byte order mark. Signature and version combined are 16 bytes long.
-const char kFileSignature[] = "# ninjadeps\n";
-const int kCurrentVersion = 4;
+static const char kFileSignature[] = "# ninjadeps\n";
+static const size_t kFileSignatureSize = sizeof(kFileSignature) - 1u;
+
+static const int32_t kCurrentVersion = 4;
 
 // Record size is currently limited to less than the full 32 bit, due to
 // internal buffers having to have this size.
-const unsigned kMaxRecordSize = (1 << 19) - 1;
+static constexpr size_t kMaxRecordSize = (1 << 19) - 1;
 
 DepsLog::~DepsLog() {
   Close();
@@ -160,16 +163,18 @@ LoadStatus DepsLog::Load(const string& path, State* state, string* err) {
     return LOAD_ERROR;
   }
 
-  bool valid_header = true;
-  int version = 0;
-  if (!fgets(buf, sizeof(buf), f) || fread(&version, 4, 1, f) < 1)
-    valid_header = false;
+  bool valid_header = fread(buf, kFileSignatureSize, 1, f) == 1 &&
+                      !memcmp(buf, kFileSignature, kFileSignatureSize);
+
+  int32_t version = 0;
+  bool valid_version =
+      fread(&version, 4, 1, f) == 1 && version == kCurrentVersion;
+
   // Note: For version differences, this should migrate to the new format.
   // But the v1 format could sometimes (rarely) end up with invalid data, so
   // don't migrate v1 to v3 to force a rebuild. (v2 only existed for a few days,
   // and there was no release with it, so pretend that it never happened.)
-  if (!valid_header || strcmp(buf, kFileSignature) != 0 ||
-      version != kCurrentVersion) {
+  if (!valid_header || !valid_version) {
     if (version == 1)
       *err = "deps log version change; rebuilding";
     else
@@ -203,7 +208,10 @@ LoadStatus DepsLog::Load(const string& path, State* state, string* err) {
     }
 
     if (is_deps) {
-      assert(size % 4 == 0);
+      if ((size % 4) != 0) {
+        read_failed = true;
+        break;
+      }
       int* deps_data = reinterpret_cast<int*>(buf);
       int out_id = deps_data[0];
       TimeStamp mtime;
@@ -212,10 +220,18 @@ LoadStatus DepsLog::Load(const string& path, State* state, string* err) {
       deps_data += 3;
       int deps_count = (size / 4) - 3;
 
+      for (int i = 0; i < deps_count; ++i) {
+        int node_id = deps_data[i];
+        if (node_id >= (int)nodes_.size() || !nodes_[node_id]) {
+          read_failed = true;
+          break;
+        }
+      }
+      if (read_failed)
+        break;
+
       Deps* deps = new Deps(mtime, deps_count);
       for (int i = 0; i < deps_count; ++i) {
-        assert(deps_data[i] < (int)nodes_.size());
-        assert(nodes_[deps_data[i]]);
         deps->nodes[i] = nodes_[deps_data[i]];
       }
 
@@ -224,7 +240,10 @@ LoadStatus DepsLog::Load(const string& path, State* state, string* err) {
         ++unique_dep_record_count;
     } else {
       int path_size = size - 4;
-      assert(path_size > 0);  // CanonicalizePath() rejects empty paths.
+      if (path_size <= 0) {
+        read_failed = true;
+        break;
+      }
       // There can be up to 3 bytes of padding.
       if (buf[path_size - 1] == '\0') --path_size;
       if (buf[path_size - 1] == '\0') --path_size;
@@ -244,12 +263,10 @@ LoadStatus DepsLog::Load(const string& path, State* state, string* err) {
       unsigned checksum = *reinterpret_cast<unsigned*>(buf + size - 4);
       int expected_id = ~checksum;
       int id = nodes_.size();
-      if (id != expected_id) {
+      if (id != expected_id || node->id() >= 0) {
         read_failed = true;
         break;
       }
-
-      assert(node->id() < 0);
       node->set_id(id);
       nodes_.push_back(node);
     }
@@ -384,6 +401,7 @@ bool DepsLog::UpdateDeps(int out_id, Deps* deps) {
 
 bool DepsLog::RecordId(Node* node) {
   int path_size = node->path().size();
+  assert(path_size > 0 && "Trying to record empty path Node!");
   int padding = (4 - path_size % 4) % 4;  // Pad path to 4 byte boundary.
 
   unsigned size = path_size + padding + 4;
@@ -398,7 +416,6 @@ bool DepsLog::RecordId(Node* node) {
   if (fwrite(&size, 4, 1, file_) < 1)
     return false;
   if (fwrite(node->path().data(), path_size, 1, file_) < 1) {
-    assert(!node->path().empty());
     return false;
   }
   if (padding && fwrite("\0\0", padding, 1, file_) < 1)

src/deps_log_test.cc

@@ -19,6 +19,7 @@
 #include <unistd.h>
 #endif
 
+#include "disk_interface.h"
 #include "graph.h"
 #include "util.h"
 #include "test.h"
@@ -34,9 +35,7 @@ struct DepsLogTest : public testing::Test {
     // In case a crashing test left a stale file behind.
     unlink(kTestFilename);
   }
-  virtual void TearDown() {
-    unlink(kTestFilename);
-  }
+  virtual void TearDown() { unlink(kTestFilename); }
 };
 
 TEST_F(DepsLogTest, WriteRead) {
@@ -542,4 +541,139 @@ TEST_F(DepsLogTest, ReverseDepsNodes) {
   EXPECT_TRUE(rev_deps == state.GetNode("out.o", 0));
 }
 
+TEST_F(DepsLogTest, MalformedDepsLog) {
+  std::string err;
+  {
+    State state;
+    DepsLog log;
+    EXPECT_TRUE(log.OpenForWrite(kTestFilename, &err));
+    ASSERT_EQ("", err);
+
+    // First, create a valid log file.
+    std::vector<Node*> deps;
+    deps.push_back(state.GetNode("foo.hh", 0));
+    deps.push_back(state.GetNode("bar.hpp", 0));
+    log.RecordDeps(state.GetNode("out.o", 0), 1, deps);
+    log.Close();
+  }
+
+  // Now read its value, validate it a little.
+  RealDiskInterface disk;
+
+  std::string original_contents;
+  ASSERT_EQ(FileReader::Okay, disk.ReadFile(kTestFilename,
+                                          &original_contents, &err));
+
+  const size_t version_offset = 12;
+  ASSERT_EQ("# ninjadeps\n", original_contents.substr(0, version_offset));
+  ASSERT_EQ('\x04', original_contents[version_offset + 0]);
+  ASSERT_EQ('\x00', original_contents[version_offset + 1]);
+  ASSERT_EQ('\x00', original_contents[version_offset + 2]);
+  ASSERT_EQ('\x00', original_contents[version_offset + 3]);
+
+  // clang-format off
+  static const uint8_t kFirstRecord[] = {
+    // size field == 0x0000000c
+    0x0c, 0x00, 0x00, 0x00,
+    // name field = 'out.o' + 3 bytes of padding.
+    'o', 'u', 't', '.', 'o', 0x00, 0x00, 0x00,
+    // checksum = ~0
+    0xff, 0xff, 0xff, 0xff,
+  };
+  // clang-format on
+  const size_t kFirstRecordLen = sizeof(kFirstRecord);
+  const size_t first_offset = version_offset + 4;
+
+#define COMPARE_RECORD(start_pos, reference, len)  \
+  ASSERT_EQ(original_contents.substr(start_pos, len), std::string(reinterpret_cast<const char*>(reference), len))
+
+  COMPARE_RECORD(first_offset, kFirstRecord, kFirstRecordLen);
+
+  const size_t second_offset = first_offset + kFirstRecordLen;
+  // clang-format off
+  static const uint8_t kSecondRecord[] = {
+    // size field == 0x0000000c
+    0x0c, 0x00, 0x00, 0x00,
+    // name field = 'foo.hh' + 2 bytes of padding.
+    'f', 'o', 'o', '.', 'h', 'h', 0x00, 0x00,
+    // checksum = ~1
+    0xfe, 0xff, 0xff, 0xff,
+  };
+  // clang-format on
+  const size_t kSecondRecordLen = sizeof(kSecondRecord);
+  COMPARE_RECORD(second_offset, kSecondRecord, kSecondRecordLen);
+
+  // Then start generating corrupted versions and trying to load them.
+  const char kBadLogFile[] = "DepsLogTest-corrupted.tempfile";
+
+  // Helper lambda to rewrite the bad log file with new content.
+  auto write_bad_log_file =
+      [&disk, &kBadLogFile](const std::string& bad_contents) -> bool {
+    (void)disk.RemoveFile(kBadLogFile);
+    return disk.WriteFile(kBadLogFile, bad_contents);
+  };
+
+  // First, corrupt the header.
+  std::string bad_contents = original_contents;
+  bad_contents[0] = '@';
+
+  ASSERT_TRUE(write_bad_log_file(bad_contents)) << strerror(errno);
+  {
+    State state;
+    DepsLog log;
+    err.clear();
+    ASSERT_EQ(LOAD_SUCCESS, log.Load(kBadLogFile, &state, &err));
+    ASSERT_EQ("bad deps log signature or version; starting over", err);
+  }
+
+  // Second, truncate the version.
+  bad_contents = original_contents.substr(0, version_offset + 3);
+  ASSERT_TRUE(write_bad_log_file(bad_contents)) << strerror(errno);
+  {
+    State state;
+    DepsLog log;
+    err.clear();
+    ASSERT_EQ(LOAD_SUCCESS, log.Load(kBadLogFile, &state, &err));
+    ASSERT_EQ("bad deps log signature or version; starting over", err);
+  }
+
+  // Truncate first record's |size| field. The loader should recover.
+  bad_contents = original_contents.substr(0, version_offset + 4 + 3);
+  ASSERT_TRUE(write_bad_log_file(bad_contents)) << strerror(errno);
+  {
+    State state;
+    DepsLog log;
+    err.clear();
+    ASSERT_EQ(LOAD_SUCCESS, log.Load(kBadLogFile, &state, &err));
+    ASSERT_EQ("", err);
+  }
+
+  // Corrupt first record |size| value.
+  bad_contents = original_contents;
+  bad_contents[first_offset + 0] = '\x55';
+  bad_contents[first_offset + 1] = '\xaa';
+  bad_contents[first_offset + 2] = '\xff';
+  bad_contents[first_offset + 3] = '\xff';
+  ASSERT_TRUE(write_bad_log_file(bad_contents)) << strerror(errno);
+  {
+    State state;
+    DepsLog log;
+    err.clear();
+    ASSERT_EQ(LOAD_SUCCESS, log.Load(kBadLogFile, &state, &err));
+    ASSERT_EQ("premature end of file; recovering", err);
+  }
+
+  // Make first record |size| less than 4.
+  bad_contents = original_contents;
+  bad_contents[first_offset] = '\x01';
+  ASSERT_TRUE(write_bad_log_file(bad_contents)) << strerror(errno);
+  {
+    State state;
+    DepsLog log;
+    err.clear();
+    ASSERT_EQ(LOAD_SUCCESS, log.Load(kBadLogFile, &state, &err));
+    ASSERT_EQ("premature end of file; recovering", err);
+  }
+}
+
 }  // anonymous namespace

src/disk_interface.cc

@@ -246,7 +246,7 @@ TimeStamp RealDiskInterface::Stat(const string& path, string* err) const {
 }
 
 bool RealDiskInterface::WriteFile(const string& path, const string& contents) {
-  FILE* fp = fopen(path.c_str(), "w");
+  FILE* fp = fopen(path.c_str(), "wb");
   if (fp == NULL) {
     Error("WriteFile(%s): Unable to create file. %s",
           path.c_str(), strerror(errno));