summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwolfbeast <mcwerewolf@gmail.com>2018-11-02 02:08:44 +0100
committerwolfbeast <mcwerewolf@gmail.com>2018-11-02 02:08:44 +0100
commit52b989d53553949c82e999b86e24f824e55bafbb (patch)
tree88ef201f67290ebeb697eb99919a73525c635d53
parent059397bdd2b8eaaa7f2bbacb9ce415aba8db91b0 (diff)
downloadUXP-52b989d53553949c82e999b86e24f824e55bafbb.tar
UXP-52b989d53553949c82e999b86e24f824e55bafbb.tar.gz
UXP-52b989d53553949c82e999b86e24f824e55bafbb.tar.lz
UXP-52b989d53553949c82e999b86e24f824e55bafbb.tar.xz
UXP-52b989d53553949c82e999b86e24f824e55bafbb.zip
Make sure nsNSSCertList handling checks for valid certs.
-rw-r--r--security/manager/ssl/nsNSSCertificate.cpp13
-rw-r--r--security/manager/ssl/tests/unit/test_cert_chains.js26
2 files changed, 36 insertions, 3 deletions
diff --git a/security/manager/ssl/nsNSSCertificate.cpp b/security/manager/ssl/nsNSSCertificate.cpp
index 12fca5065..f6685e89a 100644
--- a/security/manager/ssl/nsNSSCertificate.cpp
+++ b/security/manager/ssl/nsNSSCertificate.cpp
@@ -1208,6 +1208,10 @@ void nsNSSCertList::destructorSafeDestroyNSSReference()
NS_IMETHODIMP
nsNSSCertList::AddCert(nsIX509Cert* aCert)
{
+ if (!aCert) {
+ return NS_ERROR_INVALID_ARG;
+ }
+
nsNSSShutDownPreventionLock locker;
if (isAlreadyShutDown()) {
return NS_ERROR_NOT_AVAILABLE;
@@ -1369,17 +1373,20 @@ nsNSSCertList::Read(nsIObjectInputStream* aStream)
nsCOMPtr<nsISupports> certSupports;
rv = aStream->ReadObject(true, getter_AddRefs(certSupports));
if (NS_FAILED(rv)) {
- break;
+ return rv;
}
nsCOMPtr<nsIX509Cert> cert = do_QueryInterface(certSupports);
+ if (!cert) {
+ return NS_ERROR_UNEXPECTED;
+ }
rv = AddCert(cert);
if (NS_FAILED(rv)) {
- break;
+ return rv;
}
}
- return rv;
+ return NS_OK;
}
NS_IMETHODIMP
diff --git a/security/manager/ssl/tests/unit/test_cert_chains.js b/security/manager/ssl/tests/unit/test_cert_chains.js
index 8abcb4e65..dd1fc9369 100644
--- a/security/manager/ssl/tests/unit/test_cert_chains.js
+++ b/security/manager/ssl/tests/unit/test_cert_chains.js
@@ -31,9 +31,30 @@ function test_cert_equals() {
" should return false");
}
+function test_bad_cert_list_serialization() {
+ // Normally the serialization of an nsIX509CertList consists of some header
+ // junk (IIDs and whatnot), 4 bytes representing how many nsIX509Cert follow,
+ // and then the serialization of each nsIX509Cert. This serialization consists
+ // of the header junk for an nsIX509CertList with 1 "nsIX509Cert", but then
+ // instead of an nsIX509Cert, the subsequent bytes represent the serialization
+ // of another nsIX509CertList (with 0 nsIX509Cert). This test ensures that
+ // nsIX509CertList safely handles this unexpected input when deserializing.
+ const badCertListSerialization =
+ "lZ+xZWUXSH+rm9iRO+UxlwAAAAAAAAAAwAAAAAAAAEYAAAABlZ+xZWUXSH+rm9iRO+UxlwAAAAAA" +
+ "AAAAwAAAAAAAAEYAAAAA";
+ let serHelper = Cc["@mozilla.org/network/serialization-helper;1"]
+ .getService(Ci.nsISerializationHelper);
+ throws(() => serHelper.deserializeObject(badCertListSerialization),
+ /NS_ERROR_UNEXPECTED/,
+ "deserializing a bogus nsIX509CertList should throw NS_ERROR_UNEXPECTED");
+}
+
function test_cert_list_serialization() {
let certList = build_cert_chain(['default-ee', 'expired-ee']);
+ throws(() => certList.addCert(null), /NS_ERROR_ILLEGAL_VALUE/,
+ "trying to add a null cert to an nsIX509CertList should throw");
+
// Serialize the cert list to a string
let serHelper = Cc["@mozilla.org/network/serialization-helper;1"]
.getService(Ci.nsISerializationHelper);
@@ -78,6 +99,11 @@ function run_test() {
// Test serialization of nsIX509CertList
add_test(function() {
+ test_bad_cert_list_serialization();
+ run_next_test();
+ });
+
+ add_test(function() {
test_cert_list_serialization();
run_next_test();
});