Bug 1572791 - Check for nulls in SSLExp_DelegateCredential and its tests r=kjacobs
authorJ.C. Jones <jjones@mozilla.com>
Tue, 13 Aug 2019 17:32:31 +0000
changeset 15249 ed50678575639fdf32c7d461b343aaf5f5b4ea77
parent 15248 cef2aa7f3b8ce52571ac4b82242ba75cdeb756ef
child 15250 ec113de50cdd1d4ccb160870099165a0f4d2b99e
push id3460
push userjjones@mozilla.com
push dateTue, 13 Aug 2019 17:33:17 +0000
reviewerskjacobs
bugs1572791
Bug 1572791 - Check for nulls in SSLExp_DelegateCredential and its tests r=kjacobs This particularly catches test errors in tls_subcerts_unittest when the profile is stale. Differential Revision: https://phabricator.services.mozilla.com/D41429
gtests/ssl_gtest/tls_agent.cc
gtests/ssl_gtest/tls_subcerts_unittest.cc
lib/ssl/tls13subcerts.c
--- a/gtests/ssl_gtest/tls_agent.cc
+++ b/gtests/ssl_gtest/tls_agent.cc
@@ -161,17 +161,19 @@ void TlsAgent::SetState(State s) {
 
 void TlsAgent::DelegateCredential(const std::string& name,
                                   const ScopedSECKEYPublicKey& dc_pub,
                                   SSLSignatureScheme dc_cert_verify_alg,
                                   PRUint32 dc_valid_for, PRTime now,
                                   SECItem* dc) {
   ScopedCERTCertificate cert;
   ScopedSECKEYPrivateKey cert_priv;
-  EXPECT_TRUE(TlsAgent::LoadCertificate(name, &cert, &cert_priv));
+  EXPECT_TRUE(TlsAgent::LoadCertificate(name, &cert, &cert_priv))
+    << "Could not load delegate certificate: " << name << "; test db corrupt?";
+
   EXPECT_EQ(SECSuccess,
             SSL_DelegateCredential(cert.get(), cert_priv.get(), dc_pub.get(),
                                    dc_cert_verify_alg, dc_valid_for, now, dc));
 }
 
 void TlsAgent::EnableDelegatedCredentials() {
   ASSERT_TRUE(EnsureTlsSetup());
   SetOption(SSL_ENABLE_DELEGATED_CREDENTIALS, PR_TRUE);
--- a/gtests/ssl_gtest/tls_subcerts_unittest.cc
+++ b/gtests/ssl_gtest/tls_subcerts_unittest.cc
@@ -235,16 +235,17 @@ TEST_P(TlsConnectTls13, DCAbortBadSignat
 
   ScopedSECKEYPublicKey pub;
   ScopedSECKEYPrivateKey priv;
   EXPECT_TRUE(TlsAgent::LoadKeyPairFromCert(kDCId, &pub, &priv));
 
   StackSECItem dc;
   TlsAgent::DelegateCredential(kDelegatorId, pub, kDCScheme, kDCValidFor, now(),
                                &dc);
+  ASSERT_TRUE(dc.data != nullptr);
 
   // Flip the first bit of the DC so that the signature is invalid.
   dc.data[0] ^= 0x01;
 
   SSLExtraServerCertData extra_data = {ssl_auth_null, nullptr, nullptr,
                                        nullptr,       &dc,     priv.get()};
   EXPECT_TRUE(server_->ConfigServerCert(kDelegatorId, true, &extra_data));
 
--- a/lib/ssl/tls13subcerts.c
+++ b/lib/ssl/tls13subcerts.c
@@ -660,16 +660,21 @@ SSLExp_DelegateCredential(const CERTCert
 {
     SECStatus rv;
     SSL3Hashes hash;
     CERTSubjectPublicKeyInfo *spki = NULL;
     SECKEYPrivateKey *tmpPriv = NULL;
     sslDelegatedCredential *dc = NULL;
     sslBuffer dcBuf = SSL_BUFFER_EMPTY;
 
+    if (!cert || !certPriv || !dcPub || !out) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
+        return SECFailure;
+    }
+
     dc = PORT_ZNew(sslDelegatedCredential);
     if (!dc) {
         PORT_SetError(SEC_ERROR_NO_MEMORY);
         goto loser;
     }
 
     /* Serialize the DC parameters. */
     PRTime start;