diff options
| -rw-r--r-- | security/manager/ssl/nsNSSCertificate.cpp | 13 | ||||
| -rw-r--r-- | security/manager/ssl/tests/unit/test_cert_chains.js | 26 |
2 files changed, 36 insertions, 3 deletions
diff --git a/security/manager/ssl/nsNSSCertificate.cpp b/security/manager/ssl/nsNSSCertificate.cpp index 12fca5065f..f6685e89af 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 8abcb4e65c..dd1fc93697 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(); }); |