Bug 1281955 - Don't Adopt() NSS allocated strings in PSM to avoid using the wrong deallocator. r=dkeeler
authorCykesiopka <cykesiopka.bmo@gmail.com>
Wed, 29 Jun 2016 18:42:37 -0700
changeset 303598 b8e3ba3674f77187daa76b865921e6c923be3d91
parent 303597 469a7975c195cdb8436bd4ef5b72e84a3c816b09
child 303599 516c82acb163c7f75048b46118d5414db5cec558
push id79126
push usercbook@mozilla.com
push dateTue, 05 Jul 2016 03:52:44 +0000
treeherdermozilla-inbound@516c82acb163 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdkeeler
bugs1281955, 1281564
milestone50.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 1281955 - Don't Adopt() NSS allocated strings in PSM to avoid using the wrong deallocator. r=dkeeler There are a few places in PSM where the result of an NSS function returning char* is adopted by e.g. an nsXPIDLCString, which will use the wrong deallocator when the string eventually gets destroyed. This is basically Bug 1281564, but the free() call is buried within the Mozilla string code instead. MozReview-Commit-ID: HVSMyRpLnjS
security/manager/ssl/nsNSSCertHelper.cpp
security/manager/ssl/nsNSSCertificateDB.cpp
--- a/security/manager/ssl/nsNSSCertHelper.cpp
+++ b/security/manager/ssl/nsNSSCertHelper.cpp
@@ -111,41 +111,46 @@ ProcessVersion(SECItem* versionItem, nsI
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   printableItem.forget(retItem);
   return NS_OK;
 }
 
-static nsresult 
-ProcessSerialNumberDER(SECItem         *serialItem, 
-                       nsINSSComponent *nssComponent,
-                       nsIASN1PrintableItem **retItem)
+static nsresult
+ProcessSerialNumberDER(const SECItem& serialItem,
+                       NotNull<nsINSSComponent*> nssComponent,
+               /*out*/ nsCOMPtr<nsIASN1PrintableItem>& retItem)
 {
-  nsresult rv;
   nsAutoString text;
-  nsCOMPtr<nsIASN1PrintableItem> printableItem = new nsNSSASN1PrintableItem();
-
-  rv = nssComponent->GetPIPNSSBundleString("CertDumpSerialNo", text); 
-  if (NS_FAILED(rv))
+  nsresult rv = nssComponent->GetPIPNSSBundleString("CertDumpSerialNo", text);
+  if (NS_FAILED(rv)) {
     return rv;
+  }
 
+  nsCOMPtr<nsIASN1PrintableItem> printableItem = new nsNSSASN1PrintableItem();
   rv = printableItem->SetDisplayName(text);
-  if (NS_FAILED(rv))
+  if (NS_FAILED(rv)) {
     return rv;
+  }
 
-  nsXPIDLCString serialNumber;
-  serialNumber.Adopt(CERT_Hexify(serialItem, 1));
-  if (!serialNumber)
+  UniquePORTString serialNumber(
+    CERT_Hexify(const_cast<SECItem*>(&serialItem), 1));
+  if (!serialNumber) {
     return NS_ERROR_OUT_OF_MEMORY;
+  }
 
-  rv = printableItem->SetDisplayValue(NS_ConvertASCIItoUTF16(serialNumber));
-  printableItem.forget(retItem);
-  return rv;
+  rv = printableItem->SetDisplayValue(NS_ConvertASCIItoUTF16(serialNumber.get()));
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  retItem = printableItem.forget();
+  return NS_OK;
 }
 
 static nsresult
 GetDefaultOIDFormat(SECItem *oid,
                     nsINSSComponent *nssComponent,
                     nsAString &outString,
                     char separator)
 {
@@ -1810,16 +1815,19 @@ static SECStatus RegisterDynamicOids()
   registered = true;
   return rv;
 }
 
 nsresult
 nsNSSCertificate::CreateTBSCertificateASN1Struct(nsIASN1Sequence **retSequence,
                                                  nsINSSComponent *nssComponent)
 {
+  MOZ_ASSERT(nssComponent);
+  NS_ENSURE_ARG(nssComponent);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   if (RegisterDynamicOids() != SECSuccess)
     return NS_ERROR_FAILURE;
 
   //
@@ -1853,20 +1861,19 @@ nsNSSCertificate::CreateTBSCertificateAS
   sequence->GetASN1Objects(getter_AddRefs(asn1Objects));
 
   nsresult rv = ProcessVersion(&mCert->version, nssComponent,
                                getter_AddRefs(printableItem));
   if (NS_FAILED(rv))
     return rv;
 
   asn1Objects->AppendElement(printableItem, false);
-  
-  rv = ProcessSerialNumberDER(&mCert->serialNumber, nssComponent,
-                              getter_AddRefs(printableItem));
 
+  rv = ProcessSerialNumberDER(mCert->serialNumber, WrapNotNull(nssComponent),
+                              printableItem);
   if (NS_FAILED(rv))
     return rv;
   asn1Objects->AppendElement(printableItem, false);
 
   nsCOMPtr<nsIASN1Sequence> algID;
   rv = ProcessSECAlgorithmID(&mCert->signature,
                              nssComponent, getter_AddRefs(algID));
   if (NS_FAILED(rv))
--- a/security/manager/ssl/nsNSSCertificateDB.cpp
+++ b/security/manager/ssl/nsNSSCertificateDB.cpp
@@ -375,18 +375,17 @@ nsNSSCertificateDB::handleCACertDownload
   rv = dialogs->ConfirmDownloadCACert(ctx, certToShow, &trustBits, &allows);
   if (NS_FAILED(rv))
     return rv;
 
   if (!allows)
     return NS_ERROR_NOT_AVAILABLE;
 
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("trust is %d\n", trustBits));
-  nsXPIDLCString nickname;
-  nickname.Adopt(CERT_MakeCANickname(tmpCert.get()));
+  UniquePORTString nickname(CERT_MakeCANickname(tmpCert.get()));
 
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("Created nick \"%s\"\n", nickname.get()));
 
   nsNSSCertTrust trust;
   trust.SetValidCA();
   trust.AddCATrust(!!(trustBits & nsIX509CertDB::TRUSTED_SSL),
                    !!(trustBits & nsIX509CertDB::TRUSTED_EMAIL),
                    !!(trustBits & nsIX509CertDB::TRUSTED_OBJSIGN));
@@ -1402,18 +1401,17 @@ NS_IMETHODIMP nsNSSCertificateDB::AddCer
   }
 
    // If there's already a certificate that matches this one in the database,
    // we still want to set its trust to the given value.
   if (tmpCert->isperm) {
     return SetCertTrustFromString(newCert, aTrust);
   }
 
-  nsXPIDLCString nickname;
-  nickname.Adopt(CERT_MakeCANickname(tmpCert.get()));
+  UniquePORTString nickname(CERT_MakeCANickname(tmpCert.get()));
 
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("Created nick \"%s\"\n", nickname.get()));
 
   rv = attemptToLogInWithDefaultPassword();
   if (NS_WARN_IF(rv != NS_OK)) {
     return rv;
   }