Bug 1247847 - Use smart pointers in nsNSSCertHelper.cpp to manage NSS resources. r=keeler draft
authorCykesiopka <cykesiopka.bmo@gmail.com>
Tue, 16 Feb 2016 16:25:09 -0800
changeset 331405 27ffd2746a4c499b35657021b7fa9208140712c6
parent 331404 58b7b92c3ad92a3bb9a59df7d944eb43a7ee0f9a
child 514376 9fb21b0df5b5df4b197e85ecacf17c1bdaf27b64
push id10977
push usercykesiopka.bmo@gmail.com
push dateWed, 17 Feb 2016 01:27:23 +0000
reviewerskeeler
bugs1247847
milestone47.0a1
Bug 1247847 - Use smart pointers in nsNSSCertHelper.cpp to manage NSS resources. r=keeler This lets us remove things like gotos in the code, and makes resource ownership slightly clearer. MozReview-Commit-ID: Kucn7exhLd7
security/manager/ssl/ScopedNSSTypes.h
security/manager/ssl/nsNSSCertHelper.cpp
--- a/security/manager/ssl/ScopedNSSTypes.h
+++ b/security/manager/ssl/ScopedNSSTypes.h
@@ -4,36 +4,38 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_ScopedNSSTypes_h
 #define mozilla_ScopedNSSTypes_h
 
 #include <limits>
 
+#include "cert.h"
+#include "cms.h"
+#include "cryptohi.h"
+#include "keyhi.h"
+#include "mozilla/Likely.h"
+#include "mozilla/Scoped.h"
+#include "mozilla/UniquePtr.h"
+#include "nsDebug.h"
+#include "nsError.h"
 #include "NSSErrorsService.h"
-#include "mozilla/Likely.h"
+#include "pk11pub.h"
+#include "pkcs12.h"
+#include "prerror.h"
+#include "prio.h"
+#include "sechash.h"
+#include "secmod.h"
+#include "secpkcs7.h"
+#include "secport.h"
+
 #ifndef MOZ_NO_MOZALLOC
 #include "mozilla/mozalloc_oom.h"
 #endif
-#include "mozilla/Scoped.h"
-#include "nsError.h"
-#include "nsDebug.h"
-#include "prio.h"
-#include "cert.h"
-#include "cms.h"
-#include "keyhi.h"
-#include "cryptohi.h"
-#include "pk11pub.h"
-#include "pkcs12.h"
-#include "sechash.h"
-#include "secpkcs7.h"
-#include "secport.h"
-#include "prerror.h"
-#include "secmod.h"
 
 namespace mozilla {
 
 // It is very common to cast between char* and uint8_t* when doing crypto stuff.
 // Here, we provide more type-safe wrappers around reinterpret_cast so you don't
 // shoot yourself in the foot by reinterpret_casting completely unrelated types.
 
 inline char *
@@ -44,19 +46,19 @@ char_ptr_cast(const uint8_t * p) { retur
 
 inline uint8_t *
 uint8_t_ptr_cast(char * p) { return reinterpret_cast<uint8_t*>(p); }
 
 inline const uint8_t *
 uint8_t_ptr_cast(const char * p) { return reinterpret_cast<const uint8_t*>(p); }
 
 // NSPR APIs use PRStatus/PR_GetError and NSS APIs use SECStatus/PR_GetError to
-// report success/failure. This funtion makes it more convenient and *safer*
+// report success/failure. This function makes it more convenient and *safer*
 // to translate NSPR/NSS results to nsresult. It is safer because it
-// refuses to traslate any bad PRStatus/SECStatus into an NS_OK, even when the
+// refuses to translate any bad PRStatus/SECStatus into an NS_OK, even when the
 // NSPR/NSS function forgot to call PR_SetError. The actual enforcement of
 // this happens in mozilla::psm::GetXPCOMFromNSSError.
 // IMPORTANT: This must be called immediately after the function returning the
 // SECStatus result. The recommended usage is:
 //    nsresult rv = MapSECStatus(f(x, y, z));
 inline nsresult
 MapSECStatus(SECStatus rv)
 {
@@ -337,12 +339,37 @@ MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLAT
                                           SECKEYPublicKey,
                                           SECKEY_DestroyPublicKey)
 MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE(ScopedSECAlgorithmID,
                                           SECAlgorithmID,
                                           internal::SECOID_DestroyAlgorithmID_true)
 MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE(ScopedSECMODModule, SECMODModule,
                                           SECMOD_DestroyModule)
 
+// Emulates MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE, but for UniquePtrs.
+#define MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(name, Type, Deleter) \
+struct name##DeletePolicy \
+{ \
+  void operator()(Type* aValue) { Deleter(aValue); } \
+}; \
+typedef UniquePtr<Type, name##DeletePolicy> name;
 
+MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniqueCERTCertificatePolicies,
+                                      CERTCertificatePolicies,
+                                      CERT_DestroyCertificatePoliciesExtension)
+MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniqueCERTOidSequence,
+                                      CERTOidSequence,
+                                      CERT_DestroyOidSequence)
+MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniqueCERTUserNotice,
+                                      CERTUserNotice,
+                                      CERT_DestroyUserNotice)
+MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniquePLArenaPool,
+                                      PLArenaPool,
+                                      internal::PORT_FreeArena_false)
+MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniqueSECItem,
+                                      SECItem,
+                                      internal::SECITEM_FreeItem_true)
+MOZ_TYPE_SPECIFIC_UNIQUE_PTR_TEMPLATE(UniqueSECKEYPublicKey,
+                                      SECKEYPublicKey,
+                                      SECKEY_DestroyPublicKey)
 } // namespace mozilla
 
 #endif // mozilla_ScopedNSSTypes_h
--- a/security/manager/ssl/nsNSSCertHelper.cpp
+++ b/security/manager/ssl/nsNSSCertHelper.cpp
@@ -1,30 +1,31 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include "nsNSSCertHelper.h"
+
+#include <algorithm>
+
 #include "mozilla/UniquePtr.h"
-
+#include "nsComponentManagerUtils.h"
+#include "nsCOMPtr.h"
+#include "nsDateTimeFormatCID.h"
+#include "nsIDateTimeFormat.h"
+#include "nsNSSASN1Object.h"
+#include "nsNSSCertificate.h"
+#include "nsNSSCertTrust.h"
+#include "nsNSSCertValidity.h"
+#include "nsNSSComponent.h"
+#include "nsServiceManagerUtils.h"
 #include "prerror.h"
 #include "prprf.h"
-
-#include "nsNSSCertHelper.h"
-#include "nsCOMPtr.h"
-#include "nsNSSCertificate.h"
+#include "ScopedNSSTypes.h"
 #include "secder.h"
-#include "nsComponentManagerUtils.h"
-#include "nsNSSCertValidity.h"
-#include "nsNSSASN1Object.h"
-#include "nsNSSComponent.h"
-#include "nsNSSCertTrust.h"
-#include "nsIDateTimeFormat.h"
-#include "nsDateTimeFormatCID.h"
-#include "nsServiceManagerUtils.h"
-#include <algorithm>
 
 using namespace mozilla;
  
 /* Object Identifier constants */
 #define CONST_OID static const unsigned char
 #define MICROSOFT_OID 0x2b, 0x6, 0x1, 0x4, 0x1, 0x82, 0x37
 #define PKIX_OID 0x2b, 0x6, 0x01, 0x05, 0x05, 0x07
 CONST_OID msCertExtCerttype[]      = { MICROSOFT_OID, 20, 2};
@@ -749,24 +750,24 @@ ProcessBasicConstraints(SECItem  *extDat
 }
 
 static nsresult
 ProcessExtKeyUsage(SECItem  *extData, 
                    nsAString &text,
                    nsINSSComponent *nssComponent)
 {
   nsAutoString local;
-  CERTOidSequence *extKeyUsage = nullptr;
   SECItem **oids;
   SECItem *oid;
   nsresult rv;
-  
-  extKeyUsage = CERT_DecodeOidSequence(extData);
-  if (!extKeyUsage)
+
+  UniqueCERTOidSequence extKeyUsage(CERT_DecodeOidSequence(extData));
+  if (!extKeyUsage) {
     return NS_ERROR_FAILURE;
+  }
 
   oids = extKeyUsage->oids;
   while (oids && *oids) {
     // For each OID, try to find a bundle string
     // of the form CertDumpEKU_<underlined-OID>
     nsAutoString oidname;
     oid = *oids;
     rv = GetDefaultOIDFormat(oid, nssComponent, oidname, '_');
@@ -788,62 +789,59 @@ ProcessExtKeyUsage(SECItem  *extData,
     } else
       // If there is no bundle string, just display the OID itself
       text.Append(oidname);
 
     text.AppendLiteral(SEPARATOR);
     oids++;
   }
 
-  CERT_DestroyOidSequence(extKeyUsage);
   return NS_OK;
 }
 
 static nsresult
 ProcessRDN(CERTRDN* rdn, nsAString &finalString, nsINSSComponent *nssComponent)
 {
   nsresult rv;
   CERTAVA** avas;
   CERTAVA* ava;
-  SECItem *decodeItem = nullptr;
   nsString avavalue;
   nsString type;
   nsAutoString temp;
   const char16_t *params[2];
 
   avas = rdn->avas;
   while ((ava = *avas++) != 0) {
     rv = GetOIDText(&ava->type, nssComponent, type);
-    if (NS_FAILED(rv))
+    if (NS_FAILED(rv)) {
       return rv;
-    
+    }
+
     //This function returns a string in UTF8 format.
-    decodeItem = CERT_DecodeAVAValue(&ava->value);
-    if(!decodeItem) {
+    UniqueSECItem decodeItem(CERT_DecodeAVAValue(&ava->value));
+    if (!decodeItem) {
       return NS_ERROR_FAILURE;
     }
 
     // We know we can fit buffer of this length. CERT_RFC1485_EscapeAndQuote
     // will fail if we provide smaller buffer then the result can fit to.
     int escapedValueCapacity = decodeItem->len * 3 + 3;
     UniquePtr<char[]> escapedValue = MakeUnique<char[]>(escapedValueCapacity);
 
     SECStatus status = CERT_RFC1485_EscapeAndQuote(
           escapedValue.get(),
           escapedValueCapacity, 
           (char*)decodeItem->data, 
           decodeItem->len);
     if (SECSuccess != status) {
-      SECITEM_FreeItem(decodeItem, true);
       return NS_ERROR_FAILURE;
     }
 
     avavalue = NS_ConvertUTF8toUTF16(escapedValue.get());
-    
-    SECITEM_FreeItem(decodeItem, true);
+
     params[0] = type.get();
     params[1] = avavalue.get();
     nssComponent->PIPBundleFormatStringFromName("AVATemplate",
                                                   params, 2, temp);
     finalString += temp + NS_LITERAL_STRING("\n");
   }
   return NS_OK;
 }
@@ -900,63 +898,56 @@ ProcessIA5String(SECItem  *extData,
     return NS_ERROR_FAILURE;
   local.AssignASCII((char*)item.data, item.len);
   free(item.data);
   text.Append(local);
   return NS_OK;
 }
 
 static nsresult
-AppendBMPtoUTF16(PLArenaPool *arena,
-		 unsigned char* data, unsigned int len,
-		 nsAString& text)
+AppendBMPtoUTF16(const UniquePLArenaPool& arena, unsigned char* data,
+                 unsigned int len, nsAString& text)
 {
-  unsigned int   utf8ValLen;
-  unsigned char *utf8Val;
-
-  if (len % 2 != 0)
+  if (len % 2 != 0) {
     return NS_ERROR_FAILURE;
+  }
 
   /* XXX instead of converting to and from UTF-8, it would
      be sufficient to just swap bytes, or do nothing */
-  utf8ValLen = len * 3 + 1;
-  utf8Val = (unsigned char*)PORT_ArenaZAlloc(arena, utf8ValLen);
-  if (!PORT_UCS2_UTF8Conversion(false, data, len,
-				utf8Val, utf8ValLen, &utf8ValLen))
+  unsigned int utf8ValLen = len * 3 + 1;
+  unsigned char* utf8Val = (unsigned char*)PORT_ArenaZAlloc(arena.get(),
+                                                            utf8ValLen);
+  if (!PORT_UCS2_UTF8Conversion(false, data, len, utf8Val, utf8ValLen,
+                                &utf8ValLen)) {
     return NS_ERROR_FAILURE;
+  }
   AppendUTF8toUTF16((char*)utf8Val, text);
   return NS_OK;
 }
 
 static nsresult
-ProcessBMPString(SECItem  *extData, 
-		 nsAString &text,
-		 nsINSSComponent *nssComponent)
+ProcessBMPString(SECItem* extData, nsAString& text)
 {
+  UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  if (!arena) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+
   SECItem item;
-  PLArenaPool *arena;
-  nsresult rv = NS_ERROR_FAILURE;
-  
-  arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-  if (!arena)
+  if (SEC_ASN1DecodeItem(arena.get(), &item, SEC_ASN1_GET(SEC_BMPStringTemplate),
+                         extData) != SECSuccess) {
     return NS_ERROR_FAILURE;
+  }
 
-  if (SECSuccess == SEC_ASN1DecodeItem(arena, &item, 
-				       SEC_ASN1_GET(SEC_BMPStringTemplate),
-				       extData))
-    rv = AppendBMPtoUTF16(arena, item.data, item.len, text);
-  PORT_FreeArena(arena, false);
-  return rv;
+  return AppendBMPtoUTF16(arena, item.data, item.len, text);
 }
 
 static nsresult
-ProcessGeneralName(PLArenaPool *arena,
-		   CERTGeneralName *current,
-		   nsAString &text,
-		   nsINSSComponent *nssComponent)
+ProcessGeneralName(const UniquePLArenaPool& arena, CERTGeneralName* current,
+                   nsAString& text, nsINSSComponent* nssComponent)
 {
   NS_ENSURE_ARG_POINTER(current);
 
   nsAutoString key;
   nsXPIDLString value;
   nsresult rv = NS_OK;
 
   switch (current->type) {
@@ -964,47 +955,48 @@ ProcessGeneralName(PLArenaPool *arena,
     SECOidTag oidTag = SECOID_FindOIDTag(&current->name.OthName.oid);
     if (oidTag == SEC_OID(MS_NT_PRINCIPAL_NAME)) {
 	/* The type of this name is apparently nowhere explicitly
 	   documented. However, in the generated templates, it is always
 	   UTF-8. So try to decode this as UTF-8; if that fails, dump the
 	   raw data. */
 	SECItem decoded;
 	nssComponent->GetPIPNSSBundleString("CertDumpMSNTPrincipal", key);
-	if (SEC_ASN1DecodeItem(arena, &decoded, 
+        if (SEC_ASN1DecodeItem(arena.get(), &decoded,
 			       SEC_ASN1_GET(SEC_UTF8StringTemplate), 
 			       &current->name.OthName.name) == SECSuccess) {
 	  AppendUTF8toUTF16(nsAutoCString((char*)decoded.data, decoded.len),
 			    value);
 	} else {
 	  ProcessRawBytes(nssComponent, &current->name.OthName.name, value);
 	}
 	break;
     } else if (oidTag == SEC_OID(MS_NTDS_REPLICATION)) {
 	/* This should be a 16-byte GUID */
 	SECItem guid;
 	nssComponent->GetPIPNSSBundleString("CertDumpMSDomainGUID", key);
-	if (SEC_ASN1DecodeItem(arena, &guid,
+        if (SEC_ASN1DecodeItem(arena.get(), &guid,
 			       SEC_ASN1_GET(SEC_OctetStringTemplate),
 			       &current->name.OthName.name) == SECSuccess
 	    && guid.len == 16) {
 	  char buf[40];
 	  unsigned char *d = guid.data;
 	  PR_snprintf(buf, sizeof(buf), 
 		      "{%.2x%.2x%.2x%.2x-%.2x%.2x-%.2x%.2x-%.2x%.2x-%.2x%.2x%.2x%.2x%.2x%.2x}",
 		      d[3], d[2], d[1], d[0], d[5], d[4], d[7], d[6],
 		      d[8], d[9], d[10], d[11], d[12], d[13], d[14], d[15]);
 	  value.AssignASCII(buf);
 	} else {
 	  ProcessRawBytes(nssComponent, &current->name.OthName.name, value);
 	}
     } else {
       rv = GetDefaultOIDFormat(&current->name.OthName.oid, nssComponent, key, ' ');
-      if (NS_FAILED(rv))
-	goto finish;
+      if (NS_FAILED(rv)) {
+        return rv;
+      }
       ProcessRawBytes(nssComponent, &current->name.OthName.name, value);
     }
     break;
   }
   case certRFC822Name:
     nssComponent->GetPIPNSSBundleString("CertDumpRFC822Name", key);
     value.AssignASCII((char*)current->name.other.data, current->name.other.len);
     break;
@@ -1015,18 +1007,19 @@ ProcessGeneralName(PLArenaPool *arena,
   case certX400Address:
     nssComponent->GetPIPNSSBundleString("CertDumpX400Address", key);
     ProcessRawBytes(nssComponent, &current->name.other, value);
     break;
   case certDirectoryName:
     nssComponent->GetPIPNSSBundleString("CertDumpDirectoryName", key);
     rv = ProcessName(&current->name.directoryName, nssComponent, 
 		     getter_Copies(value));
-    if (NS_FAILED(rv))
-      goto finish;
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
     break;
   case certEDIPartyName:
     nssComponent->GetPIPNSSBundleString("CertDumpEDIPartyName", key);
     ProcessRawBytes(nssComponent, &current->name.other, value);
     break;
   case certURI:
     nssComponent->GetPIPNSSBundleString("CertDumpURI", key);
     value.AssignASCII((char*)current->name.other.data, current->name.other.len);
@@ -1053,167 +1046,148 @@ ProcessGeneralName(PLArenaPool *arena,
         /* invalid IP address */
         ProcessRawBytes(nssComponent, &current->name.other, value);
       }
       break;
     }
   case certRegisterID:
     nssComponent->GetPIPNSSBundleString("CertDumpRegisterID", key);
     rv = GetDefaultOIDFormat(&current->name.other, nssComponent, value, '.');
-    if (NS_FAILED(rv))
-      goto finish;
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
     break;
   }
   text.Append(key);
   text.AppendLiteral(": ");
   text.Append(value);
   text.AppendLiteral(SEPARATOR);
- finish:
-    return rv;
+
+  return rv;
 }
 
 static nsresult
-ProcessGeneralNames(PLArenaPool *arena,
-		    CERTGeneralName *nameList,
-		    nsAString &text,
-		    nsINSSComponent *nssComponent)
+ProcessGeneralNames(const UniquePLArenaPool& arena, CERTGeneralName* nameList,
+                    nsAString& text, nsINSSComponent* nssComponent)
 {
-  CERTGeneralName *current = nameList;
+  CERTGeneralName* current = nameList;
   nsresult rv;
 
   do {
     rv = ProcessGeneralName(arena, current, text, nssComponent);
-    if (NS_FAILED(rv))
+    if (NS_FAILED(rv)) {
       break;
+    }
     current = CERT_GetNextGeneralName(current);
   } while (current != nameList);
   return rv;
 }
 
 static nsresult
-ProcessAltName(SECItem  *extData, 
-	       nsAString &text,
-	       nsINSSComponent *nssComponent)
+ProcessAltName(SECItem* extData, nsAString& text, nsINSSComponent* nssComponent)
 {
-  nsresult rv = NS_OK;
-  PLArenaPool *arena;
-  CERTGeneralName *nameList;
+  UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  if (!arena) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
 
-  arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-  if (!arena)
-    return NS_ERROR_FAILURE;
+  CERTGeneralName* nameList = CERT_DecodeAltNameExtension(arena.get(), extData);
+  if (!nameList) {
+    return NS_OK;
+  }
 
-  nameList = CERT_DecodeAltNameExtension(arena, extData);
-  if (!nameList)
-    goto finish;
-
-  rv = ProcessGeneralNames(arena, nameList, text, nssComponent);
-
- finish:
-  PORT_FreeArena(arena, false);
-  return rv;
+  return ProcessGeneralNames(arena, nameList, text, nssComponent);
 }
 
 static nsresult
 ProcessSubjectKeyId(SECItem  *extData, 
 		    nsAString &text,
 		    nsINSSComponent *nssComponent)
 {
-  PLArenaPool *arena;
-  nsresult rv = NS_OK;
   SECItem decoded;
   nsAutoString local;
 
-  arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-  if (!arena)
-    return NS_ERROR_FAILURE;
+  UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  if (!arena) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
 
-  if (SEC_QuickDERDecodeItem(arena, &decoded, 
+  if (SEC_QuickDERDecodeItem(arena.get(), &decoded,
 			     SEC_ASN1_GET(SEC_OctetStringTemplate), 
 			     extData) != SECSuccess) {
-    rv = NS_ERROR_FAILURE;
-    goto finish;
+    return NS_ERROR_FAILURE;
   }
-  
+
   nssComponent->GetPIPNSSBundleString("CertDumpKeyID", local);
   text.Append(local);
   text.AppendLiteral(": ");
   ProcessRawBytes(nssComponent, &decoded, text);
 
- finish:
-  PORT_FreeArena(arena, false);
-  return rv;
+  return NS_OK;
 }
 
 static nsresult
 ProcessAuthKeyId(SECItem  *extData, 
 		 nsAString &text,
 		 nsINSSComponent *nssComponent)
 {
-  CERTAuthKeyID *ret;
-  PLArenaPool *arena;
   nsresult rv = NS_OK;
   nsAutoString local;
 
-  arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-  if (!arena)
+  UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  if (!arena) {
     return NS_ERROR_OUT_OF_MEMORY;
+  }
 
-  ret = CERT_DecodeAuthKeyID (arena, extData);
+  CERTAuthKeyID* ret = CERT_DecodeAuthKeyID(arena.get(), extData);
   if (!ret) {
-    rv = NS_ERROR_FAILURE;
-    goto finish;
+    return NS_ERROR_FAILURE;
   }
 
   if (ret->keyID.len > 0) {
     nssComponent->GetPIPNSSBundleString("CertDumpKeyID", local);
     text.Append(local);
     text.AppendLiteral(": ");
     ProcessRawBytes(nssComponent, &ret->keyID, text);
     text.AppendLiteral(SEPARATOR);
   }
 
   if (ret->authCertIssuer) {
     nssComponent->GetPIPNSSBundleString("CertDumpIssuer", local);
     text.Append(local);
     text.AppendLiteral(": ");
     rv = ProcessGeneralNames(arena, ret->authCertIssuer, text, nssComponent);
-    if (NS_FAILED(rv))
-      goto finish;
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
   }
 
   if (ret->authCertSerialNumber.len > 0) {
     nssComponent->GetPIPNSSBundleString("CertDumpSerialNo", local);
     text.Append(local);
     text.AppendLiteral(": ");
     ProcessRawBytes(nssComponent, &ret->authCertSerialNumber, text);
   }
 
- finish:
-  PORT_FreeArena(arena, false);
   return rv;
 }
 
 static nsresult
-ProcessUserNotice(SECItem *der_notice,
-		  nsAString &text,
-		  nsINSSComponent *nssComponent)
+ProcessUserNotice(SECItem* derNotice, nsAString& text,
+                  nsINSSComponent* nssComponent)
 {
-  CERTUserNotice *notice = nullptr;
-  SECItem **itemList;
-  PLArenaPool *arena;
+  UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  if (!arena) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
 
-  arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-  if (!arena)
-    return NS_ERROR_FAILURE;
-
-  notice = CERT_DecodeUserNotice(der_notice);
+  UniqueCERTUserNotice notice(CERT_DecodeUserNotice(derNotice));
   if (!notice) {
-    ProcessRawBytes(nssComponent, der_notice, text);
-    goto finish;
+    ProcessRawBytes(nssComponent, derNotice, text);
+    return NS_OK;
   }
 
   if (notice->noticeReference.organization.len != 0) {
     switch (notice->noticeReference.organization.type) {
     case siAsciiString:
     case siVisibleString:
     case siUTF8String:
       text.Append(NS_ConvertUTF8toUTF16(
@@ -1223,17 +1197,17 @@ ProcessUserNotice(SECItem *der_notice,
     case siBMPString:
       AppendBMPtoUTF16(arena, notice->noticeReference.organization.data,
                        notice->noticeReference.organization.len, text);
       break;
     default:
       break;
     }
     text.AppendLiteral(" - ");
-    itemList = notice->noticeReference.noticeNumbers;
+    SECItem** itemList = notice->noticeReference.noticeNumbers;
     while (*itemList) {
       unsigned long number;
       char buffer[60];
       if (SEC_ASN1DecodeInteger(*itemList, &number) == SECSuccess) {
         PR_snprintf(buffer, sizeof(buffer), "#%d", number);
         if (itemList != notice->noticeReference.noticeNumbers)
           text.AppendLiteral(", ");
         AppendASCIItoUTF16(buffer, text);
@@ -1254,38 +1228,36 @@ ProcessUserNotice(SECItem *der_notice,
     case siBMPString:
       AppendBMPtoUTF16(arena, notice->displayText.data, notice->displayText.len,
 		       text);
       break;
     default:
       break;
     }
   }
- finish:
-  if (notice)
-    CERT_DestroyUserNotice(notice);
-  PORT_FreeArena(arena, false);
+
   return NS_OK;
 }
 
 static nsresult
 ProcessCertificatePolicies(SECItem  *extData, 
 			   nsAString &text,
                            SECOidTag ev_oid_tag, // SEC_OID_UNKNOWN means: not EV
 			   nsINSSComponent *nssComponent)
 {
-  CERTCertificatePolicies *policies;
   CERTPolicyInfo **policyInfos, *policyInfo;
   CERTPolicyQualifier **policyQualifiers, *policyQualifier;
   nsAutoString local;
   nsresult rv = NS_OK;
 
-  policies = CERT_DecodeCertificatePoliciesExtension(extData);
-  if (!policies)
+  UniqueCERTCertificatePolicies policies(
+    CERT_DecodeCertificatePoliciesExtension(extData));
+  if (!policies) {
     return NS_ERROR_FAILURE;
+  }
 
   policyInfos = policies->policyInfos;
   while (*policyInfos) {
     policyInfo = *policyInfos++;
     switch (policyInfo->oid) {
     case SEC_OID_VERISIGN_USER_NOTICES:
       nssComponent->GetPIPNSSBundleString("CertDumpVerisignNotices", local);
       text.Append(local);
@@ -1325,18 +1297,19 @@ ProcessCertificatePolicies(SECItem  *ext
 	  text.Append(local);
 	  text.Append(':');
 	  text.AppendLiteral(SEPARATOR);
 	  text.AppendLiteral("    ");
 	  /* The CPS pointer ought to be the cPSuri alternative
 	     of the Qualifier choice. */
 	  rv = ProcessIA5String(&policyQualifier->qualifierValue,
 				text, nssComponent);
-	  if (NS_FAILED(rv))
-	    goto finish;
+          if (NS_FAILED(rv)) {
+            return rv;
+          }
 	  break;
 	case SEC_OID_PKIX_USER_NOTICE_QUALIFIER:
 	  nssComponent->GetPIPNSSBundleString("CertDumpUserNotice", local);
 	  text.Append(local);
 	  text.AppendLiteral(": ");
 	  rv = ProcessUserNotice(&policyQualifier->qualifierValue,
 				 text, nssComponent);
 	  break;
@@ -1347,156 +1320,160 @@ ProcessCertificatePolicies(SECItem  *ext
 	  ProcessRawBytes(nssComponent, &policyQualifier->qualifierValue, text);
 	}
 	text.AppendLiteral(SEPARATOR);
       } /* while policyQualifiers */
     } /* if policyQualifiers */
     text.AppendLiteral(SEPARATOR);
   }
 
- finish:
-  CERT_DestroyCertificatePoliciesExtension(policies);
   return rv;
 }
 
 static nsresult
 ProcessCrlDistPoints(SECItem  *extData, 
 		     nsAString &text,
 		     nsINSSComponent *nssComponent)
 {
-  CERTCrlDistributionPoints *crldp;
-  CRLDistributionPoint **points, *point;
-  PLArenaPool *arena;
   nsresult rv = NS_OK;
   nsAutoString local;
-  int reasons, comma;
 
-  arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-  if (!arena)
-    return NS_ERROR_FAILURE;
-
-  crldp = CERT_DecodeCRLDistributionPoints(arena, extData);
-  if (!crldp || !crldp->distPoints) {
-    rv = NS_ERROR_FAILURE;
-    goto finish;
+  UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  if (!arena) {
+    return NS_ERROR_OUT_OF_MEMORY;
   }
 
-  for(points = crldp->distPoints; *points; points++) {
-    point = *points;
+  CERTCrlDistributionPoints* crldp =
+    CERT_DecodeCRLDistributionPoints(arena.get(), extData);
+  if (!crldp || !crldp->distPoints) {
+    return NS_ERROR_FAILURE;
+  }
+
+  for (CRLDistributionPoint** points = crldp->distPoints; *points; points++) {
+    CRLDistributionPoint* point = *points;
     switch (point->distPointType) {
     case generalName:
       rv = ProcessGeneralName(arena, point->distPoint.fullName,
 			      text, nssComponent);
-      if (NS_FAILED(rv))
-	goto finish;
+      if (NS_FAILED(rv)) {
+        return rv;
+      }
       break;
     case relativeDistinguishedName:
       rv = ProcessRDN(&point->distPoint.relativeName, 
 		      text, nssComponent);
-      if (NS_FAILED(rv))
-	goto finish;
+      if (NS_FAILED(rv)) {
+        return rv;
+      }
       break;
     }
     if (point->reasons.len) { 
-      reasons = point->reasons.data[0];
+      int reasons = point->reasons.data[0];
       text.Append(' ');
-      comma = 0;
+      bool comma = false;
       if (reasons & RF_UNUSED) {
 	nssComponent->GetPIPNSSBundleString("CertDumpUnused", local);
-	text.Append(local); comma = 1;
+        text.Append(local);
+        comma = true;
       }
       if (reasons & RF_KEY_COMPROMISE) {
 	if (comma) text.AppendLiteral(", ");
 	nssComponent->GetPIPNSSBundleString("CertDumpKeyCompromise", local);
-	text.Append(local); comma = 1;
+        text.Append(local);
+        comma = true;
       }
       if (reasons & RF_CA_COMPROMISE) {
 	if (comma) text.AppendLiteral(", ");
 	nssComponent->GetPIPNSSBundleString("CertDumpCACompromise", local);
-	text.Append(local); comma = 1;
+        text.Append(local);
+        comma = true;
       }
       if (reasons & RF_AFFILIATION_CHANGED) {
 	if (comma) text.AppendLiteral(", ");
 	nssComponent->GetPIPNSSBundleString("CertDumpAffiliationChanged", local);
-	text.Append(local); comma = 1;
+        text.Append(local);
+        comma = true;
       }
       if (reasons & RF_SUPERSEDED) {
 	if (comma) text.AppendLiteral(", ");
 	nssComponent->GetPIPNSSBundleString("CertDumpSuperseded", local);
-	text.Append(local); comma = 1;
+        text.Append(local);
+        comma = true;
       }
       if (reasons & RF_CESSATION_OF_OPERATION) {
 	if (comma) text.AppendLiteral(", ");
 	nssComponent->GetPIPNSSBundleString("CertDumpCessation", local);
-	text.Append(local); comma = 1;
+        text.Append(local);
+        comma = true;
       }
       if (reasons & RF_CERTIFICATE_HOLD) {
 	if (comma) text.AppendLiteral(", ");
 	nssComponent->GetPIPNSSBundleString("CertDumpHold", local);
-	text.Append(local); comma = 1;
+        text.Append(local);
+        comma = true;
       }
       text.AppendLiteral(SEPARATOR);
     }
     if (point->crlIssuer) {
       nssComponent->GetPIPNSSBundleString("CertDumpIssuer", local);
       text.Append(local);
       text.AppendLiteral(": ");
       rv = ProcessGeneralNames(arena, point->crlIssuer,
 			       text, nssComponent);
-      if (NS_FAILED(rv))
-	goto finish;
+      if (NS_FAILED(rv)) {
+        return rv;
+      }
     }
   }
-  
- finish:
-  PORT_FreeArena(arena, false);
+
   return NS_OK;
 }
 
 static nsresult
 ProcessAuthInfoAccess(SECItem  *extData, 
 		      nsAString &text,
 		      nsINSSComponent *nssComponent)
 {
-  CERTAuthInfoAccess **aia, *desc;
-  PLArenaPool *arena;
   nsresult rv = NS_OK;
   nsAutoString local;
 
-  arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-  if (!arena)
-    return NS_ERROR_FAILURE;
+  UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  if (!arena) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
 
-  aia = CERT_DecodeAuthInfoAccessExtension(arena, extData);
-  if (!aia)
-    goto finish;
+  CERTAuthInfoAccess** aia = CERT_DecodeAuthInfoAccessExtension(arena.get(),
+                                                                extData);
+  if (!aia) {
+    return NS_OK;
+  }
 
   while (*aia) {
-    desc = *aia++;
+    CERTAuthInfoAccess* desc = *aia++;
     switch (SECOID_FindOIDTag(&desc->method)) {
     case SEC_OID_PKIX_OCSP:
       nssComponent->GetPIPNSSBundleString("CertDumpOCSPResponder", local);
       break;
     case SEC_OID_PKIX_CA_ISSUERS:
       nssComponent->GetPIPNSSBundleString("CertDumpCAIssuers", local);
       break;
     default:
       rv = GetDefaultOIDFormat(&desc->method, nssComponent, local, '.');
-      if (NS_FAILED(rv))
-	goto finish;
+      if (NS_FAILED(rv)) {
+        return rv;
+      }
     }
     text.Append(local);
     text.AppendLiteral(": ");
     rv = ProcessGeneralName(arena, desc->location, text, nssComponent);
-    if (NS_FAILED(rv))
-      goto finish;
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
   }
 
- finish:
-  PORT_FreeArena(arena, false);
   return rv;
 }
 
 static nsresult
 ProcessMSCAVersion(SECItem  *extData, 
 		   nsAString &text,
 		   nsINSSComponent *nssComponent)
 {
@@ -1558,17 +1535,17 @@ ProcessExtensionData(SECOidTag oidTag, S
   case SEC_OID_X509_CRL_DIST_POINTS:
     rv = ProcessCrlDistPoints(extData, text, nssComponent);
     break;
   case SEC_OID_X509_AUTH_INFO_ACCESS:
     rv = ProcessAuthInfoAccess(extData, text, nssComponent);
     break;
   default:
     if (oidTag == SEC_OID(MS_CERT_EXT_CERTTYPE)) {
-      rv = ProcessBMPString(extData, text, nssComponent);
+      rv = ProcessBMPString(extData, text);
       break;
     }
     if (oidTag == SEC_OID(MS_CERTSERV_CA_VERSION)) {
       rv = ProcessMSCAVersion(extData, text, nssComponent);
       break;
     }
     rv = ProcessRawBytes(nssComponent, extData, text);
     break; 
@@ -1721,18 +1698,18 @@ ProcessSubjectPublicKeyInfo(CERTSubjectP
   sequenceItem->SetDisplayName(text);
   nsCOMPtr<nsIMutableArray> asn1Objects;
   spkiSequence->GetASN1Objects(getter_AddRefs(asn1Objects));
   asn1Objects->AppendElement(sequenceItem, false);
 
   nsCOMPtr<nsIASN1PrintableItem> printableItem = new nsNSSASN1PrintableItem();
 
   text.Truncate();
- 
-  SECKEYPublicKey *key = SECKEY_ExtractPublicKey(spki);
+
+  UniqueSECKEYPublicKey key(SECKEY_ExtractPublicKey(spki));
   bool displayed = false;
   if (key) {
       switch (key->keyType) {
       case rsaKey: {
          displayed = true;
          nsAutoString length1, length2, data1, data2;
          length1.AppendInt(key->u.rsa.modulus.len * 8);
          length2.AppendInt(key->u.rsa.publicExponent.len * 8);
@@ -1767,17 +1744,16 @@ ProcessSubjectPublicKeyInfo(CERTSubjectP
         nssComponent->PIPBundleFormatStringFromName("CertDumpECTemplate",
                                                     params, 3, text);
         break;
       }
       default:
          /* Algorithm unknown, or too rarely used to bother displaying it */
          break;
       }
-      SECKEY_DestroyPublicKey (key);
   }
   if (!displayed) {
       // Algorithm unknown, display raw bytes
       // The subjectPublicKey field is encoded as a bit string.
       // ProcessRawBytes expects the length to be in bytes, so 
       // let's convert the lenght into a temporary SECItem.
       SECItem data;
       data.data = spki->subjectPublicKey.data;