Bug 1433636 - Put a limit on the length of Safe Browsing metadata values. r?gcp
Disk corruption can lead to the stored length of a value to be
unreasonably large and trigger an OOM.
Since values are all currently <= 32 bytes, we can safely enforce
a 256-byte upper bound.
MozReview-Commit-ID: XygReOpEK3
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -4874,22 +4874,31 @@
"high": 5000,
"n_buckets": 15,
"bug_numbers": [1336865],
"description": "Time spent constructing Variable-Length PrefixSet from file (ms)"
},
"URLCLASSIFIER_VLPS_LOAD_CORRUPT": {
"record_in_processes": ["main", "content"],
"alert_emails": ["francois@mozilla.com", "safebrowsing-telemetry@mozilla.org"],
- "expires_in_version": "63",
+ "expires_in_version": "never",
"releaseChannelCollection": "opt-out",
"kind": "boolean",
"bug_numbers": [1305581],
"description": "Whether or not a variable-length prefix set loaded from disk is corrupted (true = file corrupted)."
},
+ "URLCLASSIFIER_VLPS_METADATA_CORRUPT": {
+ "record_in_processes": ["main", "content"],
+ "alert_emails": ["francois@mozilla.com", "safebrowsing-telemetry@mozilla.org"],
+ "expires_in_version": "never",
+ "releaseChannelCollection": "opt-out",
+ "kind": "boolean",
+ "bug_numbers": [1433636],
+ "description": "Whether or not the metadata for a variable-length prefix set loaded from disk is corrupted (true = file corrupted)."
+ },
"URLCLASSIFIER_VLPS_LONG_PREFIXES": {
"record_in_processes": ["main", "content"],
"alert_emails": ["francois@mozilla.com", "safebrowsing-telemetry@mozilla.org"],
"expires_in_version": "63",
"releaseChannelCollection": "opt-out",
"kind": "enumerated",
"n_values": 32,
"bug_numbers": [1322523],
--- a/toolkit/components/url-classifier/LookupCache.cpp
+++ b/toolkit/components/url-classifier/LookupCache.cpp
@@ -74,17 +74,17 @@ LookupCache::LookupCache(const nsACStrin
, mRootStoreDirectory(aRootStoreDir)
{
UpdateRootDirHandle(mRootStoreDirectory);
}
nsresult
LookupCache::Open()
{
- LOG(("Loading PrefixSet"));
+ LOG(("Loading PrefixSet for %s", mTableName.get()));
nsresult rv = LoadPrefixSet();
NS_ENSURE_SUCCESS(rv, rv);
return NS_OK;
}
nsresult
LookupCache::UpdateRootDirHandle(nsIFile* aNewRootStoreDirectory)
--- a/toolkit/components/url-classifier/LookupCacheV4.cpp
+++ b/toolkit/components/url-classifier/LookupCacheV4.cpp
@@ -14,16 +14,17 @@ extern mozilla::LazyLogModule gUrlClassi
#define LOG_ENABLED() MOZ_LOG_TEST(gUrlClassifierDbServiceLog, mozilla::LogLevel::Debug)
#define METADATA_SUFFIX NS_LITERAL_CSTRING(".metadata")
namespace mozilla {
namespace safebrowsing {
const int LookupCacheV4::VER = 4;
+const uint32_t LookupCacheV4::MAX_METADATA_VALUE_LENGTH = 256;
// Prefixes coming from updates and VLPrefixSet are both stored in the HashTable
// where the (key, value) pair is a prefix size and a lexicographic-sorted string.
// The difference is prefixes from updates use std:string(to avoid additional copies)
// and prefixes from VLPrefixSet use nsCString.
// This class provides a common interface for the partial update algorithm to make it
// easier to operate on two different kind prefix string map..
class VLPrefixSet
@@ -162,16 +163,18 @@ LookupCacheV4::LoadFromFile(nsIFile* aFi
{
nsresult rv = mVLPrefixSet->LoadFromFile(aFile);
if (NS_FAILED(rv)) {
return rv;
}
nsCString state, checksum;
rv = LoadMetadata(state, checksum);
+ Telemetry::Accumulate(Telemetry::URLCLASSIFIER_VLPS_METADATA_CORRUPT,
+ rv == NS_ERROR_FILE_CORRUPTED);
if (NS_FAILED(rv)) {
return rv;
}
rv = VerifyChecksum(checksum);
Telemetry::Accumulate(Telemetry::URLCLASSIFIER_VLPS_LOAD_CORRUPT,
rv == NS_ERROR_FILE_CORRUPTED);
@@ -398,16 +401,18 @@ LookupCacheV4::VerifyChecksum(const nsAC
//////////////////////////////////////////////////////////////////////////
// A set of lightweight functions for reading/writing value from/to file.
namespace {
template<typename T>
struct ValueTraits
{
+ static_assert(sizeof(T) <= LookupCacheV4::MAX_METADATA_VALUE_LENGTH,
+ "LookupCacheV4::MAX_METADATA_VALUE_LENGTH is too small.");
static uint32_t Length(const T& aValue) { return sizeof(T); }
static char* WritePtr(T& aValue, uint32_t aLength) { return (char*)&aValue; }
static const char* ReadPtr(const T& aValue) { return (char*)&aValue; }
static bool IsFixedLength() { return true; }
};
template<>
struct ValueTraits<nsACString>
@@ -430,16 +435,18 @@ struct ValueTraits<nsACString>
return aValue.BeginReading();
}
};
template<typename T> static nsresult
WriteValue(nsIOutputStream *aOutputStream, const T& aValue)
{
uint32_t writeLength = ValueTraits<T>::Length(aValue);
+ MOZ_ASSERT(writeLength <= LookupCacheV4::MAX_METADATA_VALUE_LENGTH,
+ "LookupCacheV4::MAX_METADATA_VALUE_LENGTH is too small.");
if (!ValueTraits<T>::IsFixedLength()) {
// We need to write out the variable value length.
nsresult rv = WriteValue(aOutputStream, writeLength);
NS_ENSURE_SUCCESS(rv, rv);
}
// Write out the value.
auto valueReadPtr = ValueTraits<T>::ReadPtr(aValue);
@@ -462,16 +469,22 @@ ReadValue(nsIInputStream* aInputStream,
if (ValueTraits<T>::IsFixedLength()) {
readLength = ValueTraits<T>::Length(aValue);
} else {
// Read the variable value length from file.
nsresult rv = ReadValue(aInputStream, readLength);
NS_ENSURE_SUCCESS(rv, rv);
}
+ // Sanity-check the readLength in case of disk corruption
+ // (see bug 1433636).
+ if (readLength > LookupCacheV4::MAX_METADATA_VALUE_LENGTH) {
+ return NS_ERROR_FILE_CORRUPTED;
+ }
+
// Read the value.
uint32_t read;
auto valueWritePtr = ValueTraits<T>::WritePtr(aValue, readLength);
rv = aInputStream->Read(valueWritePtr, readLength, &read);
if (NS_FAILED(rv) || read != readLength) {
LOG(("Failed to read the value."));
return NS_FAILED(rv) ? rv : NS_ERROR_FAILURE;
}
--- a/toolkit/components/url-classifier/LookupCacheV4.h
+++ b/toolkit/components/url-classifier/LookupCacheV4.h
@@ -42,16 +42,17 @@ public:
PrefixStringMap& aOutputMap);
nsresult AddFullHashResponseToCache(const FullHashResponseMap& aResponseMap);
nsresult WriteMetadata(TableUpdateV4* aTableUpdate);
nsresult LoadMetadata(nsACString& aState, nsACString& aChecksum);
static const int VER;
+ static const uint32_t MAX_METADATA_VALUE_LENGTH;
protected:
virtual nsresult ClearPrefixes() override;
virtual nsresult StoreToFile(nsIFile* aFile) override;
virtual nsresult LoadFromFile(nsIFile* aFile) override;
virtual size_t SizeOfPrefixSet() override;
private: