corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1433636 - Put a limit on the length of Safe Browsing metadata values. r=gcp

Browse Source 
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
This commit is contained in:
Francois Marier
2018-01-30 14:21:33 -08:00
parent a3b1297377
commit 2fcfa5bb32
4 changed files with 25 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
toolkit/components/telemetry/Histograms.json
View File

@@ -4879,12 +4879,21 @@
"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"],

2
toolkit/components/url-classifier/LookupCache.cpp
View File

@@ -79,7 +79,7 @@ LookupCache::LookupCache(const nsACString& aTableName,
nsresult
LookupCache::Open()
{
LOG(("Loading PrefixSet"));
LOG(("Loading PrefixSet for %s", mTableName.get()));
nsresult rv = LoadPrefixSet();
NS_ENSURE_SUCCESS(rv, rv);

13
toolkit/components/url-classifier/LookupCacheV4.cpp
View File

@@ -19,6 +19,7 @@ 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.
@@ -167,6 +168,8 @@ LookupCacheV4::LoadFromFile(nsIFile* aFile)
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;
}
@@ -403,6 +406,8 @@ 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; }
@@ -435,6 +440,8 @@ 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);
@@ -467,6 +474,12 @@ ReadValue(nsIInputStream* aInputStream, T& aValue)
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);

1
toolkit/components/url-classifier/LookupCacheV4.h
View File

@@ -47,6 +47,7 @@ public:
nsresult LoadMetadata(nsACString& aState, nsACString& aChecksum);
static const int VER;
static const uint32_t MAX_METADATA_VALUE_LENGTH;
protected:
virtual nsresult ClearPrefixes() override;