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

--HG--
extra : transplant_source : Msmc%DB%16%23%87%00%A1%05%ABB%0BD%97%3B%A1%E7x

security/manager/ssl/nsNSSCertHelper.cpp

@@ -116,31 +116,36 @@ ProcessVersion(SECItem* versionItem, nsINSSComponent* nssComponent,
   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;
+  nsresult rv = nssComponent->GetPIPNSSBundleString("CertDumpSerialNo", text);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
   nsCOMPtr<nsIASN1PrintableItem> printableItem = new nsNSSASN1PrintableItem();
-
-  rv = nssComponent->GetPIPNSSBundleString("CertDumpSerialNo", text); 
-  if (NS_FAILED(rv))
-    return rv;
-
   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
@@ -1815,6 +1820,9 @@ nsresult
 nsNSSCertificate::CreateTBSCertificateASN1Struct(nsIASN1Sequence **retSequence,
                                                  nsINSSComponent *nssComponent)
 {
+  MOZ_ASSERT(nssComponent);
+  NS_ENSURE_ARG(nssComponent);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
@@ -1858,10 +1866,9 @@ nsNSSCertificate::CreateTBSCertificateASN1Struct(nsIASN1Sequence **retSequence,
     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);

security/manager/ssl/nsNSSCertificateDB.cpp

@@ -380,8 +380,7 @@ nsNSSCertificateDB::handleCACertDownload(nsIArray *x509Certs,
     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()));
 
@@ -1407,8 +1406,7 @@ NS_IMETHODIMP nsNSSCertificateDB::AddCertFromBase64(const char* aBase64,
     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()));