SimpleCache: Fix DCHECK on some kinds of format errors in index file

(Those would generally fail cleanly in release code on CRC checking)

Bug: 851912
Change-Id: I9c4301893c917b6a51f3d9532080ae7098829b12
Reviewed-on: https://chromium-review.googlesource.com/1099275Reviewed-by: Bence Béky <bnc@chromium.org>
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566953}

net/disk_cache/simple/simple_index_file.cc

@@ -106,6 +106,19 @@ void UmaRecordStaleIndexQuality(int missed_entry_count,
                    STALE_INDEX_MAX);
 }
 
+struct PickleHeader : public base::Pickle::Header {
+  uint32_t crc;
+};
+
+class SimpleIndexPickle : public base::Pickle {
+ public:
+  SimpleIndexPickle() : base::Pickle(sizeof(PickleHeader)) {}
+  SimpleIndexPickle(const char* data, int data_len)
+      : base::Pickle(data, data_len) {}
+
+  bool HeaderValid() const { return header_size() == sizeof(PickleHeader); }
+};
+
 bool WritePickleFile(base::Pickle* pickle, const base::FilePath& file_name) {
   File file(
       file_name,
@@ -264,7 +277,7 @@ void SimpleIndexFile::IndexMetadata::Serialize(base::Pickle* pickle) const {
 void SimpleIndexFile::SerializeFinalData(base::Time cache_modified,
                                          base::Pickle* pickle) {
   pickle->WriteInt64(cache_modified.ToInternalValue());
-  SimpleIndexFile::PickleHeader* header_p = pickle->headerT<PickleHeader>();
+  PickleHeader* header_p = pickle->headerT<PickleHeader>();
   header_p->crc = CalculatePickleCRC(*pickle);
 }
 
@@ -503,8 +516,7 @@ void SimpleIndexFile::SyncLoadFromDisk(const base::FilePath& index_filename,
 std::unique_ptr<base::Pickle> SimpleIndexFile::Serialize(
     const SimpleIndexFile::IndexMetadata& index_metadata,
     const SimpleIndex::EntrySet& entries) {
-  std::unique_ptr<base::Pickle> pickle(
-      new base::Pickle(sizeof(SimpleIndexFile::PickleHeader)));
+  std::unique_ptr<base::Pickle> pickle = std::make_unique<SimpleIndexPickle>();
 
   index_metadata.Serialize(pickle.get());
   for (SimpleIndex::EntrySet::const_iterator it = entries.begin();
@@ -524,15 +536,14 @@ void SimpleIndexFile::Deserialize(const char* data, int data_len,
   out_result->Reset();
   SimpleIndex::EntrySet* entries = &out_result->entries;
 
-  base::Pickle pickle(data, data_len);
-  if (!pickle.data()) {
+  SimpleIndexPickle pickle(data, data_len);
+  if (!pickle.data() || !pickle.HeaderValid()) {
     LOG(WARNING) << "Corrupt Simple Index File.";
     return;
   }
 
   base::PickleIterator pickle_it(pickle);
-  SimpleIndexFile::PickleHeader* header_p =
-      pickle.headerT<SimpleIndexFile::PickleHeader>();
+  PickleHeader* header_p = pickle.headerT<PickleHeader>();
   const uint32_t crc_read = header_p->crc;
   const uint32_t crc_calculated = CalculatePickleCRC(pickle);
 

net/disk_cache/simple/simple_index_file.h

@@ -183,10 +183,6 @@ class NET_EXPORT_PRIVATE SimpleIndexFile {
   static bool LegacyIsIndexFileStale(base::Time cache_last_modified,
                                      const base::FilePath& index_file_path);
 
-  struct PickleHeader : public base::Pickle::Header {
-    uint32_t crc;
-  };
-
   const scoped_refptr<base::SequencedTaskRunner> cache_runner_;
   const scoped_refptr<base::TaskRunner> worker_pool_;
   const net::CacheType cache_type_;

net/disk_cache/simple/simple_index_file_unittest.cc

@@ -365,6 +365,38 @@ TEST_F(SimpleIndexFileTest, LoadCorruptIndex) {
   EXPECT_TRUE(load_index_result.flush_required);
 }
 
+TEST_F(SimpleIndexFileTest, LoadCorruptIndex2) {
+  // Variant where the index looks like a pickle, but not one with right
+  // header size --- that used to hit a DCHECK on debug builds.
+  base::ScopedTempDir cache_dir;
+  ASSERT_TRUE(cache_dir.CreateUniqueTempDir());
+
+  WrappedSimpleIndexFile simple_index_file(cache_dir.GetPath());
+  ASSERT_TRUE(simple_index_file.CreateIndexFileDirectory());
+  const base::FilePath& index_path = simple_index_file.GetIndexFilePath();
+  base::Pickle bad_payload;
+  bad_payload.WriteString("nothing to be seen here");
+
+  EXPECT_EQ(
+      static_cast<int>(bad_payload.size()),
+      base::WriteFile(index_path, static_cast<const char*>(bad_payload.data()),
+                      bad_payload.size()));
+  base::Time fake_cache_mtime;
+  ASSERT_TRUE(simple_util::GetMTime(simple_index_file.GetIndexFilePath(),
+                                    &fake_cache_mtime));
+  EXPECT_FALSE(WrappedSimpleIndexFile::LegacyIsIndexFileStale(fake_cache_mtime,
+                                                              index_path));
+  SimpleIndexLoadResult load_index_result;
+  net::TestClosure closure;
+  simple_index_file.LoadIndexEntries(fake_cache_mtime, closure.closure(),
+                                     &load_index_result);
+  closure.WaitForResult();
+
+  EXPECT_FALSE(base::PathExists(index_path));
+  EXPECT_TRUE(load_index_result.did_load);
+  EXPECT_TRUE(load_index_result.flush_required);
+}
+
 // Tests that after an upgrade the backend has the index file put in place.
 TEST_F(SimpleIndexFileTest, SimpleCacheUpgrade) {
   base::ScopedTempDir cache_dir;