Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3299)

Unified Diff: chrome/browser/safe_browsing/safe_browsing_store_file.cc

Issue 10093004: Double-check safe-browsing database validity on update failure. (Closed) Base URL: http://git.chromium.org/chromium/src.git@master
Patch Set: Check size before reading. Created 8 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/safe_browsing/safe_browsing_store_file.cc
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_file.cc b/chrome/browser/safe_browsing/safe_browsing_store_file.cc
index 7f1924dba3e6d6f3ed8bc672a3bafb76f247d0c6..025ea2fdf6e33705b650b87394cfd1814ec4f542 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file.cc
@@ -243,6 +243,59 @@ bool SafeBrowsingStoreFile::Delete() {
return true;
}
+bool SafeBrowsingStoreFile::CheckValidity() {
+ if (empty_)
+ return true;
+
+ // If the file was not empty, it should be open.
+ DCHECK(file_.get());
+
+ if (!FileRewind(file_.get()))
+ return OnCorruptDatabase();
+
+ int64 size = 0;
+ if (!file_util::GetFileSize(filename_, &size))
+ return OnCorruptDatabase();
+
+ base::MD5Context context;
+ base::MD5Init(&context);
+
+ // Read everything except the final digest.
+ size_t bytes_left = static_cast<size_t>(size);
+ CHECK(size == static_cast<int64>(bytes_left));
+ if (bytes_left < sizeof(base::MD5Digest))
+ return OnCorruptDatabase();
+ bytes_left -= sizeof(base::MD5Digest);
+
+ // Fold the contents of the file into the checksum.
+ while (bytes_left > 0) {
+ char buf[4096];
+ const size_t c = std::min(sizeof(buf), bytes_left);
+ const size_t ret = fread(buf, 1, c, file_.get());
+
+ // The file's size changed while reading, give up.
+ if (ret != c)
+ return OnCorruptDatabase();
+ base::MD5Update(&context, base::StringPiece(buf, c));
+ bytes_left -= c;
+ }
+
+ // Calculate the digest to this point.
+ base::MD5Digest calculated_digest;
+ base::MD5Final(&calculated_digest, &context);
+
+ // Read the stored digest and verify it.
+ base::MD5Digest file_digest;
+ if (!ReadItem(&file_digest, file_.get(), NULL))
+ return OnCorruptDatabase();
+ if (0 != memcmp(&file_digest, &calculated_digest, sizeof(file_digest))) {
+ RecordFormatEvent(FORMAT_EVENT_VALIDITY_CHECKSUM_FAILURE);
+ return OnCorruptDatabase();
+ }
+
+ return true;
+}
+
void SafeBrowsingStoreFile::Init(
const FilePath& filename,
const base::Closure& corruption_callback
@@ -503,8 +556,10 @@ bool SafeBrowsingStoreFile::DoUpdate(
if (!ReadItem(&file_digest, file_.get(), NULL))
return OnCorruptDatabase();
- if (0 != memcmp(&file_digest, &calculated_digest, sizeof(file_digest)))
+ if (0 != memcmp(&file_digest, &calculated_digest, sizeof(file_digest))) {
+ RecordFormatEvent(FORMAT_EVENT_UPDATE_CHECKSUM_FAILURE);
return OnCorruptDatabase();
+ }
// Close the file so we can later rename over it.
file_.reset();

Powered by Google App Engine
This is Rietveld 408576698