bug 712914 - add some sanity checking when (de)serializing security info in the disk cache. r=michal
authorNick Hurley <hurley@todesschaf.org>
Fri, 30 Mar 2012 11:21:01 +0200
changeset 93995 461760e5dbd591abf0b943e89ca9cabad2485ecf
parent 93994 38d50ee4b9bac84f0ccc7a2f09e297fe864e16b9
child 93996 7bfb26afd19b6ac4ab99b646e7972969954441a5
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmichal
bugs712914
milestone14.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 712914 - add some sanity checking when (de)serializing security info in the disk cache. r=michal
netwerk/cache/nsDiskCacheDevice.cpp
netwerk/cache/nsDiskCacheEntry.cpp
netwerk/cache/nsDiskCacheMap.cpp
netwerk/test/unit/test_bug712914_secinfo_validation.js
netwerk/test/unit/xpcshell.ini
--- a/netwerk/cache/nsDiskCacheDevice.cpp
+++ b/netwerk/cache/nsDiskCacheDevice.cpp
@@ -544,22 +544,27 @@ nsDiskCacheDevice::FindEntry(nsCString *
     
     // compare key to be sure
     if (!key->Equals(diskEntry->Key())) {
         *collision = true;
         return nsnull;
     }
     
     nsCacheEntry * entry = diskEntry->CreateCacheEntry(this);
-    if (!entry)  return nsnull;
-    
-    binding = mBindery.CreateBinding(entry, &record);
-    if (!binding) {
-        delete entry;
-        return nsnull;
+    if (entry) {
+        binding = mBindery.CreateBinding(entry, &record);
+        if (!binding) {
+            delete entry;
+            entry = nsnull;
+        }
+    }
+
+    if (!entry) {
+      (void) mCacheMap.DeleteStorage(&record);
+      (void) mCacheMap.DeleteRecord(&record);
     }
     
     return entry;
 }
 
 
 /**
  *  NOTE: called while holding the cache service lock
--- a/netwerk/cache/nsDiskCacheEntry.cpp
+++ b/netwerk/cache/nsDiskCacheEntry.cpp
@@ -82,17 +82,22 @@ nsDiskCacheEntry::CreateCacheEntry(nsCac
         delete entry;
         return nsnull;
     }
 
     // Restore security info, if present
     const char* info = entry->GetMetaDataElement("security-info");
     if (info) {
         nsCOMPtr<nsISupports> infoObj;
-        NS_DeserializeObject(nsDependentCString(info), getter_AddRefs(infoObj));
+        rv = NS_DeserializeObject(nsDependentCString(info),
+                                  getter_AddRefs(infoObj));
+        if (NS_FAILED(rv)) {
+            delete entry;
+            return nsnull;
+        }
         entry->SetSecurityInfo(infoObj);
     }
 
     return entry;                      
 }
 
 
 /******************************************************************************
--- a/netwerk/cache/nsDiskCacheMap.cpp
+++ b/netwerk/cache/nsDiskCacheMap.cpp
@@ -782,22 +782,25 @@ nsDiskCacheMap::ReadDiskCacheEntry(nsDis
 nsDiskCacheEntry *
 nsDiskCacheMap::CreateDiskCacheEntry(nsDiskCacheBinding *  binding,
                                      PRUint32 * aSize)
 {
     nsCacheEntry * entry = binding->mCacheEntry;
     if (!entry)  return nsnull;
     
     // Store security info, if it is serializable
-    nsCOMPtr<nsISerializable> serializable =
-        do_QueryInterface(entry->SecurityInfo());
+    nsCOMPtr<nsISupports> infoObj = entry->SecurityInfo();
+    nsCOMPtr<nsISerializable> serializable = do_QueryInterface(infoObj);
+    if (infoObj && !serializable) return nsnull;
     if (serializable) {
         nsCString info;
-        NS_SerializeToString(serializable, info);
-        entry->SetMetaDataElement("security-info", info.get());
+        nsresult rv = NS_SerializeToString(serializable, info);
+        if (NS_FAILED(rv)) return nsnull;
+        rv = entry->SetMetaDataElement("security-info", info.get());
+        if (NS_FAILED(rv)) return nsnull;
     }
 
     PRUint32  keySize  = entry->Key()->Length() + 1;
     PRUint32  metaSize = entry->MetaDataSize();
     PRUint32  size     = sizeof(nsDiskCacheEntry) + keySize + metaSize;
     
     if (aSize) *aSize = size;
     
new file mode 100644
--- /dev/null
+++ b/netwerk/test/unit/test_bug712914_secinfo_validation.js
@@ -0,0 +1,152 @@
+var Ci = Components.interfaces;
+var Cc = Components.classes;
+var Cr = Components.results;
+
+var _cacheSvc;
+var _ios;
+
+var ACCESS_WRITE = Ci.nsICache.ACCESS_WRITE;
+var ACCESS_READ = Ci.nsICache.ACCESS_READ;
+
+var KEY_CORRUPT_SECINFO = "corruptSecurityInfo";
+var ENTRY_DATA = "foobar";
+
+function create_scriptable_input(unscriptable_input) {
+  var istream = Cc["@mozilla.org/scriptableinputstream;1"].
+                createInstance(Ci.nsIScriptableInputStream);
+  istream.init(unscriptable_input);
+  return istream;
+}
+
+function CacheListener(cb) {
+  this._cb = cb;
+}
+
+CacheListener.prototype = {
+  _cb: null,
+
+  QueryInterface: function (iid) {
+    if (iid.equals(Ci.nsISupports) ||
+        iid.equals(Ci.nsICacheListener))
+      return this;
+    throw Cr.NS_ERROR_NO_INTERFACE;
+  },
+
+  onCacheEntryAvailable: function (descriptor, accessGranted, status) {
+    this._cb(descriptor, accessGranted, status);
+  },
+
+  onCacheEntryDoomed: function (status) {
+    // Nothing to do here
+  }
+};
+
+function CacheVisitor() {
+}
+
+CacheVisitor.prototype = {
+  QueryInterface: function (iid) {
+    if (iid.equals(Ci.nsISupports) ||
+        iid.equals(Ci.nsICacheVisitor))
+      return this;
+    throw Cr.NS_ERROR_NO_INTERFACE;
+  },
+
+  visitDevice: function (deviceID, deviceInfo) {
+    if (deviceID == "disk") {
+      // We only care about the disk device, since that's the only place we
+      // actually serialize security info
+      do_check_eq(deviceInfo.entryCount, 0);
+      do_check_eq(deviceInfo.totalSize, 0);
+    }
+
+    // We don't care about visiting entries since we get all the info we need
+    // from the device itself
+    return false;
+  },
+
+  visitEntry: function (deviceID, entryInfo) {
+    // Just in case we somehow got here, just skip on
+    return false;
+  }
+};
+
+function get_cache_service() {
+  if (!_cacheSvc) {
+    _cacheSvc = Cc["@mozilla.org/network/cache-service;1"].
+                getService(Ci.nsICacheService);
+  }
+
+  return _cacheSvc
+}
+
+function get_io_service() {
+  if (!_ios) {
+    _ios = Cc["@mozilla.org/network/io-service;1"].
+           getService(Ci.nsIIOService);
+  }
+
+  return _ios;
+}
+
+function open_entry(key, access, cb) {
+  var cache = get_cache_service();
+  var session = cache.createSession("HTTP", Ci.nsICache.STORE_ON_DISK,
+                                    Ci.nsICache.STREAM_BASED);
+  var listener = new CacheListener(cb);
+  session.asyncOpenCacheEntry(key, access, listener);
+}
+
+function write_data(entry) {
+  var ostream = entry.openOutputStream(0);
+  if (ostream.write(ENTRY_DATA, ENTRY_DATA.length) != ENTRY_DATA.length) {
+    do_throw("Could not write all data!");
+  }
+  ostream.close();
+}
+
+function continue_failure(descriptor, accessGranted, status) {
+  // Make sure we couldn't open this for reading
+  do_check_eq(status, Cr.NS_ERROR_CACHE_KEY_NOT_FOUND);
+
+  // Make sure the cache is empty
+  var cache = get_cache_service();
+  var visitor = new CacheVisitor();
+  cache.visitEntries(visitor);
+  run_next_test();
+}
+
+function try_read_corrupt_secinfo() {
+  open_entry(KEY_CORRUPT_SECINFO, ACCESS_READ, continue_failure);
+}
+
+function write_corrupt_secinfo(entry, accessGranted, status) {
+  write_data(entry);
+  entry.setMetaDataElement("security-info", "blablabla");
+  try {
+    entry.close();
+  } catch (e) {
+    do_throw("Unexpected exception closing corrupt entry: " + e);
+  }
+
+  try_read_corrupt_secinfo();
+}
+
+function test_corrupt_secinfo() {
+  open_entry(KEY_CORRUPT_SECINFO, ACCESS_WRITE, write_corrupt_secinfo);
+}
+
+function run_test() {
+  // Make sure we have a cache to use
+  do_get_profile();
+
+  // Make sure the cache is empty
+  var cache = get_cache_service();
+  cache.evictEntries(Ci.nsICache.STORE_ANYWHERE);
+
+  // Add new tests at the end of this section
+  add_test(test_corrupt_secinfo);
+
+  // Let's get going!
+  run_next_test();
+}
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -79,16 +79,17 @@ fail-if = os == "android"
 [test_bug654926_test_seek.js]
 # Bug 675049: test fails consistently on Android
 fail-if = os == "android"  
 [test_bug659569.js]
 [test_bug660066.js]
 [test_bug667907.js]
 [test_bug667818.js]
 [test_bug669001.js]
+[test_bug712914_secinfo_validation.js]
 [test_doomentry.js]
 [test_cacheflags.js]
 [test_channel_close.js]
 [test_compareURIs.js]
 [test_compressappend.js]
 [test_content_encoding_gzip.js]
 [test_content_sniffer.js]
 [test_cookie_header.js]