net: fix crash when failing to import a client-side cert into NSS.

net/socket/ssl_client_socket_nss.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This file includes code SSLClientSocketNSS::DoVerifyCertComplete() derived
 // from AuthCertificateCallback() in
 // mozilla/security/manager/ssl/src/nsNSSCallbacks.cpp.
 
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
@@ skipping 2148 matching lines @@
         SECItem der_cert;
         der_cert.type = siDERCertBuffer;
         der_cert.data = cert_context->pbCertEncoded;
         der_cert.len  = cert_context->cbCertEncoded;
 
         // TODO(rsleevi): Error checking for NSS allocation errors.
         *result_certs = CERT_NewCertList();
         CERTCertDBHandle* db_handle = CERT_GetDefaultCertDB();
         CERTCertificate* user_cert = CERT_NewTempCertificate(
             db_handle, &der_cert, NULL, PR_FALSE, PR_TRUE);
+        if (!user_cert) {
+          // Importing the certificate can fail for reasons including a serial
+          // number collision. See crbug.com/97355.
+          CERT_DestroyCertList(*result_certs);
+          *result_certs = NULL;
+          return SECFailure;
+        }
         CERT_AddCertToListTail(*result_certs, user_cert);
 
         // Add the intermediates.
         X509Certificate::OSCertHandles intermediates =
             that->ssl_config_.client_cert->GetIntermediateCertificates();
         for (X509Certificate::OSCertHandles::const_iterator it =
             intermediates.begin(); it != intermediates.end(); ++it) {
           der_cert.data = (*it)->pbCertEncoded;
           der_cert.len = (*it)->cbCertEncoded;
 
           CERTCertificate* intermediate = CERT_NewTempCertificate(
               db_handle, &der_cert, NULL, PR_FALSE, PR_TRUE);
+          if (!intermediate) {
+            CERT_DestroyCertList(*result_certs);
+            *result_certs = NULL;
+            return SECFailure;
+          }
           CERT_AddCertToListTail(*result_certs, intermediate);
         }
         *result_private_key = key_context;
         return SECSuccess;
       }
       PORT_Free(key_context);
       LOG(WARNING) << "Client cert found without private key";
     }
     // Send no client certificate.
     return SECFailure;
@@ skipping 122 matching lines @@
           os_error = SecCertificateGetData(cert_ref, &cert_data);
           if (os_error != noErr)
             break;
 
           SECItem der_cert;
           der_cert.type = siDERCertBuffer;
           der_cert.data = cert_data.Data;
           der_cert.len = cert_data.Length;
           CERTCertificate* nss_cert = CERT_NewTempCertificate(
               CERT_GetDefaultCertDB(), &der_cert, NULL, PR_FALSE, PR_TRUE);
+          if (!nss_cert) {
+            // In the event of an NSS error we make up an OS error and reuse
+            // the error handling, below.
+            os_error = errKCCreateChainFailed;

[mattm, 2011/09/22 21:31:19]

Do we need a break here?

[agl, 2011/09/22 22:07:09]

I don't think so but, on the other hand, why not? Done.

+          }
           CERT_AddCertToListTail(*result_certs, nss_cert);
         }
       }
       if (os_error == noErr) {
         CFRelease(chain);
         return SECSuccess;
       }
       LOG(WARNING) << "Client cert found, but could not be used: "
                    << os_error;
       if (*result_certs) {
@@ skipping 165 matching lines @@
   valid_thread_id_ = base::PlatformThread::CurrentId();
 }
 
 bool SSLClientSocketNSS::CalledOnValidThread() const {
   EnsureThreadIdAssigned();
   base::AutoLock auto_lock(lock_);
   return valid_thread_id_ == base::PlatformThread::CurrentId();
 }
 
 }  // namespace net