Bug 562636: Memory leak when decoding PKCS12, r=rrelyea
authornelson%bolyard.com
Fri, 18 Jun 2010 06:43:20 +0000
changeset 9682 284eb4ea9782528a5ad2fb0e3dc9514e145d1c3c
parent 9681 a5852b9b6a9f31520156fe23873a610b5e887184
child 9683 b8de622b4a861f513e64d3575230e83292f1e524
push idunknown
push userunknown
push dateunknown
reviewersrrelyea
bugs562636
Bug 562636: Memory leak when decoding PKCS12, r=rrelyea
security/coreconf/WIN32.mk
security/nss/cmd/pk12util/pk12util.c
security/nss/lib/pkcs12/p12d.c
--- a/security/coreconf/WIN32.mk
+++ b/security/coreconf/WIN32.mk
@@ -138,17 +138,17 @@ else # !NS_USE_GCC
 		LDFLAGS += -DEBUG -OPT:REF
 	endif
     else
 	#
 	# Define USE_DEBUG_RTL if you want to use the debug runtime library
 	# (RTL) in the debug build
 	#
 	ifdef USE_DEBUG_RTL
-		OS_CFLAGS += -MDd
+		OS_CFLAGS += -MDd -DUSE_DEBUG_RTL -D_CRTDBG_MAP_ALLOC
 	else
 		OS_CFLAGS += -MD
 	endif
 	OPTIMIZER += -Zi -Fd$(OBJDIR)/ -Od
 	NULLSTRING :=
 	SPACE      := $(NULLSTRING) # end of the line
 	USERNAME   := $(subst $(SPACE),_,$(USERNAME))
 	USERNAME   := $(subst -,_,$(USERNAME))
--- a/security/nss/cmd/pk12util/pk12util.c
+++ b/security/nss/cmd/pk12util/pk12util.c
@@ -29,16 +29,21 @@
  * use your version of this file under the terms of the MPL, indicate your
  * decision by deleting the provisions above and replace them with the notice
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
+#ifdef _CRTDBG_MAP_ALLOC
+#include <stdlib.h>
+#include <crtdbg.h>
+#endif
+
 #include "nspr.h"
 #include "secutil.h"
 #include "pk11func.h"
 #include "pkcs12.h"
 #include "p12plcy.h"
 #include "pk12util.h"
 #include "nss.h"
 #include "secport.h"
@@ -477,16 +482,17 @@ done:
     PR_Close(p12cxt->file);
     p12cxt->file = NULL;
     /* PK11_FreeSlot(slot); */
     p12u_DestroyContext(&p12cxt, PR_FALSE);
 
     if (pwitem) {
 	SECITEM_ZfreeItem(pwitem, PR_TRUE);
     }
+    SECITEM_ZfreeItem(&p12file, PR_FALSE);
     return p12dcx;
 }
 
 /*
  * given a filename for pkcs12 file, imports certs and keys
  *
  * Change: altitude
  *  I've changed this function so that it takes the keydb and pkcs12 file
@@ -973,18 +979,22 @@ main(int argc, char **argv)
     char *export_file = NULL;
     char *dbprefix = "";
     SECStatus rv;
     SECOidTag cipher = 
             SEC_OID_PKCS12_V2_PBE_WITH_SHA1_AND_3KEY_TRIPLE_DES_CBC;
     SECOidTag certCipher;
     int keyLen = 0;
     int certKeyLen = 0;
+    secuCommand pk12util;
 
-    secuCommand pk12util;
+#ifdef _CRTDBG_MAP_ALLOC
+    _CrtSetDbgFlag ( _CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF );
+#endif
+
     pk12util.numCommands = 0;
     pk12util.commands = 0;
     pk12util.numOptions = sizeof(pk12util_options) / sizeof(secuCommandFlag);
     pk12util.options = pk12util_options;
 
     progName = strrchr(argv[0], '/');
     progName = progName ? progName+1 : argv[0];
 
@@ -1119,10 +1129,12 @@ done:
 	PORT_ZFree(slotPw.data, PL_strlen(slotPw.data));
     if (p12FilePw.data != NULL)
 	PORT_ZFree(p12FilePw.data, PL_strlen(p12FilePw.data));
     if (slot) 
     	PK11_FreeSlot(slot);
     if (NSS_Shutdown() != SECSuccess) {
 	pk12uErrno = 1;
     }
+    PR_Cleanup();
+    PL_ArenaFinish();
     return pk12uErrno;
 }
--- a/security/nss/lib/pkcs12/p12d.c
+++ b/security/nss/lib/pkcs12/p12d.c
@@ -53,41 +53,47 @@
 #include "p12local.h"
 #include "secder.h"
 #include "secport.h"
 
 #include "certdb.h"
 
 #include "prcpucfg.h"
 
+/* This belongs in secport.h */
+#define PORT_ArenaGrowArray(poolp, oldptr, type, oldnum, newnum) \
+    (type *)PORT_ArenaGrow((poolp), (oldptr), \
+			   (oldnum) * sizeof(type), (newnum) * sizeof(type))
+
+
 typedef struct sec_PKCS12SafeContentsContextStr sec_PKCS12SafeContentsContext;
 
 /* Opaque structure for decoding SafeContents.  These are used
  * for each authenticated safe as well as any nested safe contents.
  */
 struct sec_PKCS12SafeContentsContextStr {
     /* the parent decoder context */
     SEC_PKCS12DecoderContext *p12dcx;
 
     /* memory arena to allocate space from */
     PRArenaPool *arena;
 
     /* decoder context and destination for decoding safe contents */
-    SEC_ASN1DecoderContext *safeContentsDcx;
+    SEC_ASN1DecoderContext *safeContentsA1Dcx;
     sec_PKCS12SafeContents safeContents;
 
     /* information for decoding safe bags within the safe contents.
      * these variables are updated for each safe bag decoded.
      */
-    SEC_ASN1DecoderContext *currentSafeBagDcx;
+    SEC_ASN1DecoderContext *currentSafeBagA1Dcx;
     sec_PKCS12SafeBag *currentSafeBag;
     PRBool skipCurrentSafeBag;
 
     /* if the safe contents is nested, the parent is pointed to here. */
-    sec_PKCS12SafeContentsContext *nestedCtx;
+    sec_PKCS12SafeContentsContext *nestedSafeContentsCtx;
 };
 
 /* opaque decoder context structure.  information for decoding a pkcs 12
  * PDU are stored here as well as decoding pointers for intermediary 
  * structures which are part of the PKCS 12 PDU.  Upon a successful
  * decode, the safe bags containing certificates and keys encountered.
  */  
 struct SEC_PKCS12DecoderContextStr {
@@ -96,52 +102,51 @@ struct SEC_PKCS12DecoderContextStr {
     void *wincx;
     PRBool error;
     int errorValue;
 
     /* password */
     SECItem *pwitem;
 
     /* used for decoding the PFX structure */
-    SEC_ASN1DecoderContext *pfxDcx;
-    sec_PKCS12PFXItem pfx;
+    SEC_ASN1DecoderContext 	*pfxA1Dcx;
+    sec_PKCS12PFXItem 		pfx;
 
     /* safe bags found during decoding */  
-    sec_PKCS12SafeBag **safeBags;
-    unsigned int safeBagCount;
+    sec_PKCS12SafeBag 		**safeBags;
+    unsigned int 		safeBagCount;
 
     /* state variables for decoding authenticated safes. */
-    SEC_PKCS7DecoderContext *currentASafeP7Dcx;
-    SEC_ASN1DecoderContext *aSafeDcx;
-    SEC_PKCS7DecoderContext *aSafeP7Dcx;
+    SEC_PKCS7DecoderContext 	*currentASafeP7Dcx;
+    SEC_ASN1DecoderContext 	*aSafeA1Dcx;
+    SEC_PKCS7DecoderContext 	*aSafeP7Dcx;
+    SEC_PKCS7ContentInfo 	*aSafeCinfo;
     sec_PKCS12AuthenticatedSafe authSafe;
-    SEC_PKCS7ContentInfo *aSafeCinfo;
-    sec_PKCS12SafeContents safeContents;
+    sec_PKCS12SafeContents 	safeContents;
 
     /* safe contents info */
-    unsigned int safeContentsCnt;
+    unsigned int 		safeContentsCnt;
     sec_PKCS12SafeContentsContext **safeContentsList;
 
     /* HMAC info */
-    sec_PKCS12MacData	macData;
-    SEC_ASN1DecoderContext *hmacDcx;
+    sec_PKCS12MacData		macData;
 
     /* routines for reading back the data to be hmac'd */
-    digestOpenFn dOpen;
-    digestCloseFn dClose;
-    digestIOFn dRead, dWrite;
-    void *dArg;
+    digestOpenFn 		dOpen;
+    digestCloseFn 		dClose;
+    digestIOFn 			dRead, dWrite;
+    void 			*dArg;
 
     /* helper functions */
-    SECKEYGetPasswordKey pwfn;
-    void *pwfnarg;
-    PRBool swapUnicodeBytes;
+    SECKEYGetPasswordKey 	pwfn;
+    void 			*pwfnarg;
+    PRBool 			swapUnicodeBytes;
 
     /* import information */
-    PRBool bagsVerified;
+    PRBool 			bagsVerified;
 
     /* buffer management for the default callbacks implementation */
     void        *buffer;      /* storage area */
     PRInt32     filesize;     /* actual data size */
     PRInt32     allocated;    /* total buffer size allocated */
     PRInt32     currentpos;   /* position counter */
     SECPKCS12TargetTokenCAs tokenCAs;
     sec_PKCS12SafeBag **keyList;/* used by ...IterateNext() */
@@ -167,18 +172,17 @@ sec_pkcs12_proper_version(sec_PKCS12PFXI
 
     return PR_TRUE;
 }
 
 /* retrieve the key for decrypting the safe contents */ 
 static PK11SymKey *
 sec_pkcs12_decoder_get_decrypt_key(void *arg, SECAlgorithmID *algid)
 {
-    SEC_PKCS12DecoderContext *p12dcx =
-	(SEC_PKCS12DecoderContext *) arg;
+    SEC_PKCS12DecoderContext *p12dcx = (SEC_PKCS12DecoderContext *) arg;
     PK11SlotInfo *slot;
     PK11SymKey *bulkKey;
 
     if(!p12dcx) {
 	return NULL;
     }
 
     /* if no slot specified, use the internal key slot */
@@ -251,37 +255,33 @@ sec_pkcs12_decoder_init_new_safe_bag(sec
 
     p12dcx = safeContentsCtx->p12dcx;
     mark = PORT_ArenaMark(p12dcx->arena);
 
     /* allocate a new safe bag, if bags already exist, grow the 
      * list of bags, otherwise allocate a new list.  the list is
      * NULL terminated.
      */
-    if(p12dcx->safeBagCount) {
-	p12dcx->safeBags = 
-	    (sec_PKCS12SafeBag**)PORT_ArenaGrow(p12dcx->arena,p12dcx->safeBags,
-			(p12dcx->safeBagCount + 1) * sizeof(sec_PKCS12SafeBag *),
-			(p12dcx->safeBagCount + 2) * sizeof(sec_PKCS12SafeBag *));
-    } else {
-	p12dcx->safeBags = (sec_PKCS12SafeBag**)PORT_ArenaZAlloc(p12dcx->arena,
-					    2 * sizeof(sec_PKCS12SafeBag *));
-    }
+    p12dcx->safeBags = (!p12dcx->safeBagCount)
+	? PORT_ArenaZNewArray(p12dcx->arena, sec_PKCS12SafeBag *, 2)
+        : PORT_ArenaGrowArray(p12dcx->arena, p12dcx->safeBags,
+				sec_PKCS12SafeBag *, p12dcx->safeBagCount + 1,
+				p12dcx->safeBagCount + 2);
+
     if(!p12dcx->safeBags) {
 	p12dcx->errorValue = PORT_GetError();
 	goto loser;
     }
 
     /* append the bag to the end of the list and update the reference
      * in the safeContentsCtx.
      */
     p12dcx->safeBags[p12dcx->safeBagCount] = 
     safeContentsCtx->currentSafeBag = 
-        (sec_PKCS12SafeBag*)PORT_ArenaZAlloc(p12dcx->arena,
-					     sizeof(sec_PKCS12SafeBag));
+			    PORT_ArenaZNew(p12dcx->arena, sec_PKCS12SafeBag);
     if(!safeContentsCtx->currentSafeBag) {
 	p12dcx->errorValue = PORT_GetError();
 	goto loser;
     }
     p12dcx->safeBags[++p12dcx->safeBagCount] = NULL;
 
     safeContentsCtx->currentSafeBag->slot = safeContentsCtx->p12dcx->slot;
     safeContentsCtx->currentSafeBag->pwitem = safeContentsCtx->p12dcx->pwitem;
@@ -328,33 +328,33 @@ sec_pkcs12_decoder_safe_bag_update(void 
      */
     if(!safeContentsCtx || !safeContentsCtx->p12dcx 
 		|| safeContentsCtx->p12dcx->error 
 		|| safeContentsCtx->skipCurrentSafeBag) {
 	return;
     }
     p12dcx = safeContentsCtx->p12dcx;
 
-    rv = SEC_ASN1DecoderUpdate(safeContentsCtx->currentSafeBagDcx, data, len);
+    rv = SEC_ASN1DecoderUpdate(safeContentsCtx->currentSafeBagA1Dcx, data, len);
     if(rv != SECSuccess) {
 	p12dcx->errorValue = PORT_GetError();
 	goto loser;
     }
 
     return;
 
 loser:
     /* set the error, and finish the decoder context.  because there 
      * is not a way of returning an error message, it may be worth
      * while to do a check higher up and finish any decoding contexts
      * that are still open.
      */
     p12dcx->error = PR_TRUE;
-    SEC_ASN1DecoderFinish(safeContentsCtx->currentSafeBagDcx);
-    safeContentsCtx->currentSafeBagDcx = NULL;
+    SEC_ASN1DecoderFinish(safeContentsCtx->currentSafeBagA1Dcx);
+    safeContentsCtx->currentSafeBagA1Dcx = NULL;
     return;
 }
 
 /* forward declarations of functions that are used when decoding
  * safeContents bags which are nested and when decoding the 
  * authenticatedSafes.
  */
 static SECStatus
@@ -466,64 +466,65 @@ sec_pkcs12_decoder_safe_contents_notify(
 	return;
     }
     p12dcx = safeContentsCtx->p12dcx;
 
     /* if we are done with the current safeBag, then we need to
      * finish the context and set the state variables appropriately.
      */
     if(!before) {
-	SEC_ASN1DecoderClearFilterProc(safeContentsCtx->safeContentsDcx);
-	SEC_ASN1DecoderFinish(safeContentsCtx->currentSafeBagDcx);
-	safeContentsCtx->currentSafeBagDcx = NULL;
+	SEC_ASN1DecoderClearFilterProc(safeContentsCtx->safeContentsA1Dcx);
+	SEC_ASN1DecoderFinish(safeContentsCtx->currentSafeBagA1Dcx);
+	safeContentsCtx->currentSafeBagA1Dcx = NULL;
 	safeContentsCtx->skipCurrentSafeBag = PR_FALSE;
     } else {
 	/* we are starting a new safe bag.  we need to allocate space
 	 * for the bag and initialize the decoding context.
 	 */
 	rv = sec_pkcs12_decoder_init_new_safe_bag(safeContentsCtx);
 	if(rv != SECSuccess) {
 	    goto loser;
 	}
 
 	/* set up the decoder context */
-	safeContentsCtx->currentSafeBagDcx = SEC_ASN1DecoderStart(p12dcx->arena,
-						safeContentsCtx->currentSafeBag,
-						sec_PKCS12SafeBagTemplate);
-	if(!safeContentsCtx->currentSafeBagDcx) {
+	safeContentsCtx->currentSafeBagA1Dcx = 
+		SEC_ASN1DecoderStart(p12dcx->arena,
+				     safeContentsCtx->currentSafeBag,
+				     sec_PKCS12SafeBagTemplate);
+	if(!safeContentsCtx->currentSafeBagA1Dcx) {
 	    p12dcx->errorValue = PORT_GetError();
 	    goto loser;
 	}
 
 	/* set the notify and filter procs so that the safe bag
 	 * data gets sent to the proper location when decoding.
 	 */
-	SEC_ASN1DecoderSetNotifyProc(safeContentsCtx->currentSafeBagDcx, 
+	SEC_ASN1DecoderSetNotifyProc(safeContentsCtx->currentSafeBagA1Dcx, 
 				 sec_pkcs12_decoder_safe_bag_notify, 
 				 safeContentsCtx);
-	SEC_ASN1DecoderSetFilterProc(safeContentsCtx->safeContentsDcx, 
+	SEC_ASN1DecoderSetFilterProc(safeContentsCtx->safeContentsA1Dcx, 
 				 sec_pkcs12_decoder_safe_bag_update, 
 				 safeContentsCtx, PR_TRUE);
     }
 
     return;
 
 loser:
     /* in the event of an error, we want to close the decoding
      * context and clear the filter and notify procedures.
      */
     p12dcx->error = PR_TRUE;
 
-    if(safeContentsCtx->currentSafeBagDcx) {
-	SEC_ASN1DecoderFinish(safeContentsCtx->currentSafeBagDcx);
-	safeContentsCtx->currentSafeBagDcx = NULL;
+    if(safeContentsCtx->currentSafeBagA1Dcx) {
+	SEC_ASN1DecoderFinish(safeContentsCtx->currentSafeBagA1Dcx);
+	safeContentsCtx->currentSafeBagA1Dcx = NULL;
     }
 
-    SEC_ASN1DecoderClearNotifyProc(safeContentsCtx->safeContentsDcx);
-    SEC_ASN1DecoderClearFilterProc(safeContentsCtx->safeContentsDcx);
+    SEC_ASN1DecoderClearNotifyProc(safeContentsCtx->safeContentsA1Dcx);
+    SEC_ASN1DecoderClearFilterProc(safeContentsCtx->safeContentsA1Dcx);
 
     return;
 }
 
 /* initialize the safeContents for decoding.  this routine
  * is used for authenticatedSafes as well as nested safeContents.
  */
 static sec_PKCS12SafeContentsContext *
@@ -535,38 +536,30 @@ sec_pkcs12_decoder_safe_contents_init_de
 
     if(!p12dcx || p12dcx->error) {
 	return NULL;
     }
 
     /* allocate a new safeContents list or grow the existing list and
      * append the new safeContents onto the end.
      */
-    if(!p12dcx->safeContentsCnt) {
-	p12dcx->safeContentsList = 
-	    (sec_PKCS12SafeContentsContext**)PORT_ArenaZAlloc(p12dcx->arena, 
-	       			 2 * sizeof(sec_PKCS12SafeContentsContext *));
-    } else {
-	p12dcx->safeContentsList = 
-	   (sec_PKCS12SafeContentsContext **) PORT_ArenaGrow(p12dcx->arena,
-			p12dcx->safeContentsList,
-			(1 + p12dcx->safeContentsCnt) * 
-				sizeof(sec_PKCS12SafeContentsContext *),
-			(2 + p12dcx->safeContentsCnt) * 
-				sizeof(sec_PKCS12SafeContentsContext *));
-    }
+    p12dcx->safeContentsList = (!p12dcx->safeContentsCnt) 
+	? PORT_ArenaZNewArray(p12dcx->arena, sec_PKCS12SafeContentsContext *, 2)
+	: PORT_ArenaGrowArray(p12dcx->arena, p12dcx->safeContentsList,
+			        sec_PKCS12SafeContentsContext *,
+			        1 + p12dcx->safeContentsCnt,
+			        2 + p12dcx->safeContentsCnt);
+
     if(!p12dcx->safeContentsList) {
 	p12dcx->errorValue = PORT_GetError();
 	goto loser;
     }
 
     p12dcx->safeContentsList[p12dcx->safeContentsCnt] = safeContentsCtx = 
-        (sec_PKCS12SafeContentsContext*)PORT_ArenaZAlloc(
-					p12dcx->arena,
-					sizeof(sec_PKCS12SafeContentsContext));
+        PORT_ArenaZNew(p12dcx->arena, sec_PKCS12SafeContentsContext);
     if(!p12dcx->safeContentsList[p12dcx->safeContentsCnt]) {
 	p12dcx->errorValue = PORT_GetError();
 	goto loser;
     }
     p12dcx->safeContentsList[++p12dcx->safeContentsCnt] = NULL;
 
     /* set up the state variables */
     safeContentsCtx->p12dcx = p12dcx;
@@ -577,41 +570,41 @@ sec_pkcs12_decoder_safe_contents_init_de
      */
     if(nestedSafe == PR_TRUE) {
 	theTemplate = sec_PKCS12NestedSafeContentsDecodeTemplate;
     } else {
 	theTemplate = sec_PKCS12SafeContentsDecodeTemplate;
     }
 
     /* start the decoder context */
-    safeContentsCtx->safeContentsDcx = SEC_ASN1DecoderStart(p12dcx->arena, 
+    safeContentsCtx->safeContentsA1Dcx = SEC_ASN1DecoderStart(p12dcx->arena, 
 					&safeContentsCtx->safeContents,
 					theTemplate);
 	
-    if(!safeContentsCtx->safeContentsDcx) {
+    if(!safeContentsCtx->safeContentsA1Dcx) {
 	p12dcx->errorValue = PORT_GetError();
 	goto loser;
     }
 
     /* set the safeContents notify procedure to look for
      * and start the decode of safeBags.
      */
-    SEC_ASN1DecoderSetNotifyProc(safeContentsCtx->safeContentsDcx, 
+    SEC_ASN1DecoderSetNotifyProc(safeContentsCtx->safeContentsA1Dcx, 
 				sec_pkcs12_decoder_safe_contents_notify,
 				safeContentsCtx);
 
     return safeContentsCtx;
 
 loser:
     /* in the case of an error, we want to finish the decoder
      * context and set the error flag.
      */
-    if(safeContentsCtx && safeContentsCtx->safeContentsDcx) {
-	SEC_ASN1DecoderFinish(safeContentsCtx->safeContentsDcx);
-	safeContentsCtx->safeContentsDcx = NULL;
+    if(safeContentsCtx && safeContentsCtx->safeContentsA1Dcx) {
+	SEC_ASN1DecoderFinish(safeContentsCtx->safeContentsA1Dcx);
+	safeContentsCtx->safeContentsA1Dcx = NULL;
     }
 
     p12dcx->error = PR_TRUE;
 
     return NULL;
 }
 
 /* wrapper for updating safeContents.  this is set as the filter of
@@ -625,71 +618,74 @@ sec_pkcs12_decoder_nested_safe_contents_
     sec_PKCS12SafeContentsContext *safeContentsCtx = 
         (sec_PKCS12SafeContentsContext *)arg;
     SEC_PKCS12DecoderContext *p12dcx;
     SECStatus rv;
 
     /* check for an error */
     if(!safeContentsCtx || !safeContentsCtx->p12dcx 
 			|| safeContentsCtx->p12dcx->error
-			|| !safeContentsCtx->safeContentsDcx) {
+			|| !safeContentsCtx->safeContentsA1Dcx) {
 	return;
     }
 
     /* no need to update if no data sent in */
     if(!len || !buf) {
 	return;
     }
 
     /* update the decoding context */
     p12dcx = safeContentsCtx->p12dcx;
-    rv = SEC_ASN1DecoderUpdate(safeContentsCtx->safeContentsDcx, buf, len);
+    rv = SEC_ASN1DecoderUpdate(safeContentsCtx->safeContentsA1Dcx, buf, len);
     if(rv != SECSuccess) {
 	p12dcx->errorValue = PORT_GetError();
 	goto loser;
     }
 
     return;
 
 loser:
     /* handle any errors.  If a decoding context is open, close it. */
     p12dcx->error = PR_TRUE;
-    if(safeContentsCtx->safeContentsDcx) {
-	SEC_ASN1DecoderFinish(safeContentsCtx->safeContentsDcx);
-	safeContentsCtx->safeContentsDcx = NULL;
+    if(safeContentsCtx->safeContentsA1Dcx) {
+	SEC_ASN1DecoderFinish(safeContentsCtx->safeContentsA1Dcx);
+	safeContentsCtx->safeContentsA1Dcx = NULL;
     }
 }
 
 /* whenever a new safeContentsSafeBag is encountered, we need
  * to init a safeContentsContext.  
  */
 static SECStatus  
 sec_pkcs12_decoder_begin_nested_safe_contents(sec_PKCS12SafeContentsContext 
 							*safeContentsCtx)
 {
     /* check for an error */
     if(!safeContentsCtx || !safeContentsCtx->p12dcx || 
 		safeContentsCtx->p12dcx->error) {
 	return SECFailure;
     }
 
-    safeContentsCtx->nestedCtx = sec_pkcs12_decoder_safe_contents_init_decode(
-						safeContentsCtx->p12dcx,
-						PR_TRUE);
-    if(!safeContentsCtx->nestedCtx) {
+    safeContentsCtx->nestedSafeContentsCtx = 
+    	sec_pkcs12_decoder_safe_contents_init_decode(safeContentsCtx->p12dcx,
+						     PR_TRUE);
+    if(!safeContentsCtx->nestedSafeContentsCtx) {
 	return SECFailure;
     }
 
     /* set up new filter proc */
-    SEC_ASN1DecoderSetNotifyProc(safeContentsCtx->nestedCtx->safeContentsDcx,
+    SEC_ASN1DecoderSetNotifyProc(
+                     safeContentsCtx->nestedSafeContentsCtx->safeContentsA1Dcx,
 				 sec_pkcs12_decoder_safe_contents_notify,
-				 safeContentsCtx->nestedCtx);
-    SEC_ASN1DecoderSetFilterProc(safeContentsCtx->currentSafeBagDcx,
+				 safeContentsCtx->nestedSafeContentsCtx);
+
+    SEC_ASN1DecoderSetFilterProc(safeContentsCtx->currentSafeBagA1Dcx,
 				 sec_pkcs12_decoder_nested_safe_contents_update,
-				 safeContentsCtx->nestedCtx, PR_TRUE);
+				 safeContentsCtx->nestedSafeContentsCtx, 
+				 PR_TRUE);
 
     return SECSuccess;
 }
 
 /* when the safeContents is done decoding, we need to reset the
  * proper filter and notify procs and close the decoding context 
  */
 static SECStatus
@@ -698,21 +694,23 @@ sec_pkcs12_decoder_finish_nested_safe_co
 {
     /* check for error */
     if(!safeContentsCtx || !safeContentsCtx->p12dcx || 
 		safeContentsCtx->p12dcx->error) {
 	return SECFailure;
     }
 
     /* clean up */	
-    SEC_ASN1DecoderClearFilterProc(safeContentsCtx->currentSafeBagDcx);
-    SEC_ASN1DecoderClearNotifyProc(safeContentsCtx->nestedCtx->safeContentsDcx);
-    SEC_ASN1DecoderFinish(safeContentsCtx->nestedCtx->safeContentsDcx);
-    safeContentsCtx->nestedCtx->safeContentsDcx = NULL;
-    safeContentsCtx->nestedCtx = NULL;
+    SEC_ASN1DecoderClearFilterProc(safeContentsCtx->currentSafeBagA1Dcx);
+    SEC_ASN1DecoderClearNotifyProc(
+                    safeContentsCtx->nestedSafeContentsCtx->safeContentsA1Dcx);
+    SEC_ASN1DecoderFinish(
+                    safeContentsCtx->nestedSafeContentsCtx->safeContentsA1Dcx);
+    safeContentsCtx->nestedSafeContentsCtx->safeContentsA1Dcx = NULL;
+    safeContentsCtx->nestedSafeContentsCtx = NULL;
 
     return SECSuccess;
 }
 
 /* wrapper for updating safeContents.  This is used when decoding
  * the nested safeContents and any authenticatedSafes.
  */
 static void
@@ -722,40 +720,40 @@ sec_pkcs12_decoder_safe_contents_callbac
     SECStatus rv;
     sec_PKCS12SafeContentsContext *safeContentsCtx = 
         (sec_PKCS12SafeContentsContext *)arg;
     SEC_PKCS12DecoderContext *p12dcx;
 
     /* check for error */  
     if(!safeContentsCtx || !safeContentsCtx->p12dcx 
 		|| safeContentsCtx->p12dcx->error
-		|| !safeContentsCtx->safeContentsDcx) {
+		|| !safeContentsCtx->safeContentsA1Dcx) {
 	return;
     }
     p12dcx = safeContentsCtx->p12dcx;
 
     /* update the decoder */
-    rv = SEC_ASN1DecoderUpdate(safeContentsCtx->safeContentsDcx, buf, len);
+    rv = SEC_ASN1DecoderUpdate(safeContentsCtx->safeContentsA1Dcx, buf, len);
     if(rv != SECSuccess) {
 	/* if we fail while trying to decode a 'safe', it's probably because
 	 * we didn't have the correct password. */
 	PORT_SetError(SEC_ERROR_BAD_PASSWORD);
 	p12dcx->errorValue = SEC_ERROR_PKCS12_CORRUPT_PFX_STRUCTURE;
 	SEC_PKCS7DecoderAbort(p12dcx->currentASafeP7Dcx,SEC_ERROR_BAD_PASSWORD);
 	goto loser;
     }
 
     return;
 
 loser:
     /* set the error and finish the context */
     p12dcx->error = PR_TRUE;
-    if(safeContentsCtx->safeContentsDcx) {
-	SEC_ASN1DecoderFinish(safeContentsCtx->safeContentsDcx);
-	safeContentsCtx->safeContentsDcx = NULL;
+    if(safeContentsCtx->safeContentsA1Dcx) {
+	SEC_ASN1DecoderFinish(safeContentsCtx->safeContentsA1Dcx);
+	safeContentsCtx->safeContentsA1Dcx = NULL;
     }
 
     return;
 }
 
 /* this is a wrapper for the ASN1 decoder to call SEC_PKCS7DecoderUpdate
  */
 static void
@@ -801,30 +799,38 @@ sec_pkcs12_decoder_asafes_notify(void *a
 				safeContentsCtx, 
 				p12dcx->pwfn, p12dcx->pwfnarg,
 				sec_pkcs12_decoder_get_decrypt_key, p12dcx,
 				sec_pkcs12_decoder_decryption_allowed);
 	if(!p12dcx->currentASafeP7Dcx) {
 	    p12dcx->errorValue = PORT_GetError();
 	    goto loser;
 	}
-	SEC_ASN1DecoderSetFilterProc(p12dcx->aSafeDcx, 
+	SEC_ASN1DecoderSetFilterProc(p12dcx->aSafeA1Dcx, 
 				     sec_pkcs12_decoder_wrap_p7_update,
 				     p12dcx->currentASafeP7Dcx, PR_TRUE);
     }
 
     if(!before) {
 	/* if one is being decoded, finish the decode */
 	if(p12dcx->currentASafeP7Dcx != NULL) {
-	    if(!SEC_PKCS7DecoderFinish(p12dcx->currentASafeP7Dcx)) {
-		p12dcx->currentASafeP7Dcx = NULL;
+	    SEC_PKCS7ContentInfo * cinfo;
+	    unsigned int cnt = p12dcx->safeContentsCnt - 1;
+	    safeContentsCtx = p12dcx->safeContentsList[cnt];
+	    if (safeContentsCtx->safeContentsA1Dcx) {
+		SEC_ASN1DecoderFinish(safeContentsCtx->safeContentsA1Dcx);
+		safeContentsCtx->safeContentsA1Dcx = NULL;
+	    }
+	    cinfo = SEC_PKCS7DecoderFinish(p12dcx->currentASafeP7Dcx);
+	    p12dcx->currentASafeP7Dcx = NULL;
+	    if(!cinfo) {
 		p12dcx->errorValue = PORT_GetError();
 		goto loser;
 	    }
-	    p12dcx->currentASafeP7Dcx = NULL;
+	    SEC_PKCS7DestroyContentInfo(cinfo); /* don't leak it */
 	}
     }
 
 
     return;
 
 loser:
     /* set the error flag */
@@ -843,17 +849,17 @@ sec_pkcs12_decoder_asafes_callback(void 
     SEC_PKCS12DecoderContext *p12dcx = (SEC_PKCS12DecoderContext *)arg;
     SECStatus rv;
 
     if(!p12dcx || p12dcx->error) {
 	return;
     }
 
     /* update the context */
-    rv = SEC_ASN1DecoderUpdate(p12dcx->aSafeDcx, buf, len);
+    rv = SEC_ASN1DecoderUpdate(p12dcx->aSafeA1Dcx, buf, len);
     if(rv != SECSuccess) {
 	p12dcx->errorValue = PORT_GetError();
 	p12dcx->error = PR_TRUE;
 	goto loser;
     }
 
     /* if we are writing to a file, write out the new information */
     if(p12dcx->dWrite) {
@@ -865,42 +871,42 @@ sec_pkcs12_decoder_asafes_callback(void 
 	}
     }
 
     return;
 
 loser:
     /* set the error flag */
     p12dcx->error = PR_TRUE;
-    SEC_ASN1DecoderFinish(p12dcx->aSafeDcx);
-    p12dcx->aSafeDcx = NULL;
+    SEC_ASN1DecoderFinish(p12dcx->aSafeA1Dcx);
+    p12dcx->aSafeA1Dcx = NULL;
 
     return;
 }
    
 /* start the decode of an authenticatedSafe contentInfo.
  */ 
 static SECStatus
 sec_pkcs12_decode_start_asafes_cinfo(SEC_PKCS12DecoderContext *p12dcx)
 {
     if(!p12dcx || p12dcx->error) {
 	return SECFailure;
     }
 
     /* start the decode context */
-    p12dcx->aSafeDcx = SEC_ASN1DecoderStart(p12dcx->arena, 
+    p12dcx->aSafeA1Dcx = SEC_ASN1DecoderStart(p12dcx->arena, 
     					&p12dcx->authSafe,
     					sec_PKCS12AuthenticatedSafeTemplate);
-    if(!p12dcx->aSafeDcx) {
+    if(!p12dcx->aSafeA1Dcx) {
 	p12dcx->errorValue = PORT_GetError();
    	goto loser;
     }
 
     /* set the notify function */
-    SEC_ASN1DecoderSetNotifyProc(p12dcx->aSafeDcx,
+    SEC_ASN1DecoderSetNotifyProc(p12dcx->aSafeA1Dcx,
     				 sec_pkcs12_decoder_asafes_notify, p12dcx);
 
     /* begin the authSafe decoder context */
     p12dcx->aSafeP7Dcx = SEC_PKCS7DecoderStart(
     				sec_pkcs12_decoder_asafes_callback, p12dcx,
     				p12dcx->pwfn, p12dcx->pwfnarg, NULL, NULL, NULL);
     if(!p12dcx->aSafeP7Dcx) {
 	p12dcx->errorValue = PORT_GetError();
@@ -914,19 +920,19 @@ sec_pkcs12_decode_start_asafes_cinfo(SEC
 	goto loser;
     }
 
     return SECSuccess;
 
 loser:
     p12dcx->error = PR_TRUE;
 
-    if(p12dcx->aSafeDcx) {
-	SEC_ASN1DecoderFinish(p12dcx->aSafeDcx);
-	p12dcx->aSafeDcx = NULL;
+    if(p12dcx->aSafeA1Dcx) {
+	SEC_ASN1DecoderFinish(p12dcx->aSafeA1Dcx);
+	p12dcx->aSafeA1Dcx = NULL;
     } 
 
     if(p12dcx->aSafeP7Dcx) {
 	SEC_PKCS7DecoderFinish(p12dcx->aSafeP7Dcx);
 	p12dcx->aSafeP7Dcx = NULL;
     }
 
     return SECFailure;
@@ -979,18 +985,18 @@ sec_pkcs12_decoder_pfx_notify_proc(void 
 {
     SECStatus rv;
     SEC_PKCS12DecoderContext *p12dcx = (SEC_PKCS12DecoderContext*)arg;
 
     /* if an error occurs, clear the notifyProc and the filterProc 
      * and continue. 
      */
     if(p12dcx->error) {
-	SEC_ASN1DecoderClearNotifyProc(p12dcx->pfxDcx);
-	SEC_ASN1DecoderClearFilterProc(p12dcx->pfxDcx);
+	SEC_ASN1DecoderClearNotifyProc(p12dcx->pfxA1Dcx);
+	SEC_ASN1DecoderClearFilterProc(p12dcx->pfxA1Dcx);
 	return;
     }
 
     if(before && (dest == &p12dcx->pfx.encodedAuthSafe)) {
 
 	/* we want to make sure this is a version we support */
 	if(!sec_pkcs12_proper_version(&p12dcx->pfx)) {
 	    p12dcx->errorValue = SEC_ERROR_PKCS12_UNSUPPORTED_VERSION;
@@ -999,34 +1005,34 @@ sec_pkcs12_decoder_pfx_notify_proc(void 
 
 	/* start the decode of the aSafes cinfo... */
 	rv = sec_pkcs12_decode_start_asafes_cinfo(p12dcx);
 	if(rv != SECSuccess) {
 	    goto loser;
 	}
 
 	/* set the filter proc to update the authenticated safes. */
-	SEC_ASN1DecoderSetFilterProc(p12dcx->pfxDcx,
+	SEC_ASN1DecoderSetFilterProc(p12dcx->pfxA1Dcx,
 				     sec_pkcs12_decode_asafes_cinfo_update,
 				     p12dcx, PR_TRUE);
     }
 
     if(!before && (dest == &p12dcx->pfx.encodedAuthSafe)) {
 
 	/* we are done decoding the authenticatedSafes, so we need to 
 	 * finish the decoderContext and clear the filter proc
 	 * and close the hmac callback, if present
 	 */
 	p12dcx->aSafeCinfo = SEC_PKCS7DecoderFinish(p12dcx->aSafeP7Dcx);
 	p12dcx->aSafeP7Dcx = NULL;
 	if(!p12dcx->aSafeCinfo) {
 	    p12dcx->errorValue = PORT_GetError();
 	    goto loser;
 	}
-	SEC_ASN1DecoderClearFilterProc(p12dcx->pfxDcx);
+	SEC_ASN1DecoderClearFilterProc(p12dcx->pfxA1Dcx);
 	if(p12dcx->dClose && ((*p12dcx->dClose)(p12dcx->dArg, PR_FALSE) 
 				!= SECSuccess)) {
 	    p12dcx->errorValue = PORT_GetError();
 	    goto loser;
 	}
 
     }
 
@@ -1180,17 +1186,17 @@ SEC_PKCS12DecoderStart(SECItem *pwitem, 
     PRArenaPool *arena;
 
     arena = PORT_NewArena(2048); /* different size? */
     if(!arena) {
 	return NULL;	/* error is already set */
     }
 
     /* allocate the decoder context and set the state variables */
-    p12dcx = (SEC_PKCS12DecoderContext*)PORT_ArenaZAlloc(arena, sizeof(SEC_PKCS12DecoderContext));
+    p12dcx = PORT_ArenaZNew(arena, SEC_PKCS12DecoderContext);
     if(!p12dcx) {
 	goto loser;	/* error is already set */
     }
 
     if (!dOpen && !dClose && !dRead && !dWrite && !dArg) {
         /* use default implementations */
         dOpen = p12u_DigestOpen;
         dClose = p12u_DigestClose;
@@ -1211,24 +1217,24 @@ SEC_PKCS12DecoderStart(SECItem *pwitem, 
     p12dcx->swapUnicodeBytes = PR_FALSE;
 #endif
     p12dcx->errorValue = 0;
     p12dcx->error = PR_FALSE;
 
     /* start the decoding of the PFX and set the notify proc
      * for the PFX item.
      */
-    p12dcx->pfxDcx = SEC_ASN1DecoderStart(p12dcx->arena, &p12dcx->pfx,
+    p12dcx->pfxA1Dcx = SEC_ASN1DecoderStart(p12dcx->arena, &p12dcx->pfx,
     					  sec_PKCS12PFXItemTemplate);
-    if(!p12dcx->pfxDcx) {
+    if(!p12dcx->pfxA1Dcx) {
 	PK11_FreeSlot(p12dcx->slot);
 	goto loser;
     }
 
-    SEC_ASN1DecoderSetNotifyProc(p12dcx->pfxDcx, 
+    SEC_ASN1DecoderSetNotifyProc(p12dcx->pfxA1Dcx, 
 				 sec_pkcs12_decoder_pfx_notify_proc,
     				 p12dcx); 
     
     /* set up digest functions */
     p12dcx->dOpen = dOpen;
     p12dcx->dWrite = dWrite;
     p12dcx->dClose = dClose;
     p12dcx->dRead = dRead;
@@ -1275,17 +1281,17 @@ SEC_PKCS12DecoderUpdate(SEC_PKCS12Decode
     SECStatus rv;
 
     if(!p12dcx || p12dcx->error) {
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	return SECFailure;
     }
 
     /* update the PFX decoder context */
-    rv = SEC_ASN1DecoderUpdate(p12dcx->pfxDcx, (const char *)data, len);
+    rv = SEC_ASN1DecoderUpdate(p12dcx->pfxA1Dcx, (const char *)data, len);
     if(rv != SECSuccess) {
 	p12dcx->errorValue = SEC_ERROR_PKCS12_CORRUPT_PFX_STRUCTURE;
 	goto loser;
     }
 
     return SECSuccess;
 
 loser:
@@ -1462,85 +1468,115 @@ SEC_PKCS12DecoderVerify(SEC_PKCS12Decode
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	return SECFailure;
     }
     if(p12dcx->error) {
 	/* error code is already set! PORT_SetError(p12dcx->errorValue); */
 	return SECFailure;
     }
 
-    rv = SEC_ASN1DecoderFinish(p12dcx->pfxDcx);
-    p12dcx->pfxDcx = NULL;
+    rv = SEC_ASN1DecoderFinish(p12dcx->pfxA1Dcx);
+    p12dcx->pfxA1Dcx = NULL;
     if(rv != SECSuccess) {
 	return rv;
     }
 
     /* check the signature or the mac depending on the type of
      * integrity used.
      */
     if(p12dcx->pfx.encodedMacData.len) {
 	rv = SEC_ASN1DecodeItem(p12dcx->arena, &p12dcx->macData,
 				sec_PKCS12MacDataTemplate,
 				&p12dcx->pfx.encodedMacData);
 	if(rv == SECSuccess) {
 	    return sec_pkcs12_decoder_verify_mac(p12dcx);
 	}
-    } else {
-	if(SEC_PKCS7VerifySignature(p12dcx->aSafeCinfo, certUsageEmailSigner,
-				    PR_FALSE)) {
-	    return SECSuccess;
-	} else {
-	    PORT_SetError(SEC_ERROR_PKCS12_INVALID_MAC);
-	}
-    }
-
+	return rv;
+    } 
+    if (SEC_PKCS7VerifySignature(p12dcx->aSafeCinfo, certUsageEmailSigner, 
+                                 PR_FALSE)) {
+	return SECSuccess;
+    } 
+    PORT_SetError(SEC_ERROR_PKCS12_INVALID_MAC);
     return SECFailure;
 }
 
 /* SEC_PKCS12DecoderFinish
  *	Free any open ASN1 or PKCS7 decoder contexts and then
  *	free the arena pool which everything should be allocated
  *	from.  This function should be called upon completion of
  *	decoding and installing of a pfx pdu.  This should be
  *	called even if an error occurs.
  *
  *	p12dcx - the decoder context
  */
 void
 SEC_PKCS12DecoderFinish(SEC_PKCS12DecoderContext *p12dcx)
 {
+    unsigned int i;
+
     if(!p12dcx) {
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	return;
     }
 
-    if(p12dcx->pfxDcx) {
-	SEC_ASN1DecoderFinish(p12dcx->pfxDcx);
-	p12dcx->pfxDcx = NULL;
+    if(p12dcx->pfxA1Dcx) {
+	SEC_ASN1DecoderFinish(p12dcx->pfxA1Dcx);
+	p12dcx->pfxA1Dcx = NULL;
+    }
+
+    if(p12dcx->aSafeA1Dcx) {
+	SEC_ASN1DecoderFinish(p12dcx->aSafeA1Dcx);
+	p12dcx->aSafeA1Dcx = NULL;
     }
 
-    if(p12dcx->aSafeDcx) {
-	SEC_ASN1DecoderFinish(p12dcx->aSafeDcx);
-	p12dcx->aSafeDcx = NULL;
+    /* cleanup any old ASN1 decoder contexts */
+    for (i = 0; i < p12dcx->safeContentsCnt; ++i) {
+	sec_PKCS12SafeContentsContext *safeContentsCtx, *nested;
+	safeContentsCtx = p12dcx->safeContentsList[i];
+	if (safeContentsCtx) {
+	    nested = safeContentsCtx->nestedSafeContentsCtx;
+	    while (nested) {
+		if (nested->safeContentsA1Dcx) {
+		    SEC_ASN1DecoderFinish(nested->safeContentsA1Dcx);
+		    nested->safeContentsA1Dcx = NULL;
+		}
+		nested = nested->nestedSafeContentsCtx;
+	    }
+	    if (safeContentsCtx->safeContentsA1Dcx) {
+		SEC_ASN1DecoderFinish(safeContentsCtx->safeContentsA1Dcx);
+		safeContentsCtx->safeContentsA1Dcx = NULL;
+	    }
+	}
     }
 
-    if(p12dcx->currentASafeP7Dcx) {
-	SEC_PKCS7DecoderFinish(p12dcx->currentASafeP7Dcx);
-	p12dcx->currentASafeP7Dcx = NULL;
+    if (p12dcx->currentASafeP7Dcx &&
+	p12dcx->currentASafeP7Dcx != p12dcx->aSafeP7Dcx) {
+	SEC_PKCS7ContentInfo * cinfo;
+	cinfo = SEC_PKCS7DecoderFinish(p12dcx->currentASafeP7Dcx);
+	if (cinfo) {
+	    SEC_PKCS7DestroyContentInfo(cinfo); /* don't leak it */
+	}
     }
+    p12dcx->currentASafeP7Dcx = NULL;
 
     if(p12dcx->aSafeP7Dcx) {
-	SEC_PKCS7DecoderFinish(p12dcx->aSafeP7Dcx);
+	SEC_PKCS7ContentInfo * cinfo;
+	cinfo = SEC_PKCS7DecoderFinish(p12dcx->aSafeP7Dcx);
+	if (cinfo) {
+	    SEC_PKCS7DestroyContentInfo(cinfo);
+	}
+	p12dcx->aSafeP7Dcx = NULL;
     }
 
-    if(p12dcx->hmacDcx) {
-	SEC_ASN1DecoderFinish(p12dcx->hmacDcx);
-	p12dcx->hmacDcx = NULL;
+    if(p12dcx->aSafeCinfo) {
+	SEC_PKCS7DestroyContentInfo(p12dcx->aSafeCinfo);
+	p12dcx->aSafeCinfo = NULL;
     }
-    
+
     if (p12dcx->decitem.type != 0 && p12dcx->decitem.der != NULL) {
         SECITEM_FreeItem(p12dcx->decitem.der, PR_TRUE);
     }
     if (p12dcx->decitem.friendlyName != NULL) {
         SECITEM_FreeItem(p12dcx->decitem.friendlyName, PR_TRUE);
     }
 
     if(p12dcx->slot) {
@@ -1567,73 +1603,62 @@ sec_pkcs12_decoder_set_attribute_value(s
     }
 
     oid = SECOID_FindOIDByTag(attributeType);
     if(!oid) {
 	return SECFailure;
     }
 
     if(!bag->attribs) {
-	bag->attribs = (sec_PKCS12Attribute**)PORT_ArenaZAlloc(bag->arena, 
-					sizeof(sec_PKCS12Attribute *) * 2);
+	bag->attribs = 
+		PORT_ArenaZNewArray(bag->arena, sec_PKCS12Attribute *, 2);
     } else {
-	while(bag->attribs[i]) i++;
-	bag->attribs = (sec_PKCS12Attribute **)PORT_ArenaGrow(bag->arena, 
-				      bag->attribs, 
-				      (i + 1) * sizeof(sec_PKCS12Attribute *),
-				      (i + 2) * sizeof(sec_PKCS12Attribute *));
+	while(bag->attribs[i]) 
+	    i++;
+	bag->attribs = PORT_ArenaGrowArray(bag->arena, bag->attribs, 
+				           sec_PKCS12Attribute *, i + 1, i + 2);
     }
 
     if(!bag->attribs) {
 	return SECFailure;
     }
 
-    bag->attribs[i] = (sec_PKCS12Attribute*)PORT_ArenaZAlloc(bag->arena, 
-						  sizeof(sec_PKCS12Attribute));
+    bag->attribs[i] = PORT_ArenaZNew(bag->arena, sec_PKCS12Attribute);
     if(!bag->attribs) {
 	return SECFailure;
     }
 
-    bag->attribs[i]->attrValue = (SECItem**)PORT_ArenaZAlloc(bag->arena, 
-						  sizeof(SECItem *) * 2);
+    bag->attribs[i]->attrValue = PORT_ArenaZNewArray(bag->arena, SECItem *, 2);
     if(!bag->attribs[i]->attrValue) {
 	return SECFailure;
     }
 
     bag->attribs[i+1] = NULL;
     bag->attribs[i]->attrValue[0] = attrValue;
     bag->attribs[i]->attrValue[1] = NULL;
 
-    if(SECITEM_CopyItem(bag->arena, &bag->attribs[i]->attrType, &oid->oid)
-			!= SECSuccess) {
-	return SECFailure;
-    }
-
-    return SECSuccess;
+    return SECITEM_CopyItem(bag->arena, &bag->attribs[i]->attrType, &oid->oid);
 }
 
 static SECItem *
 sec_pkcs12_get_attribute_value(sec_PKCS12SafeBag *bag,
 			       SECOidTag attributeType)
 {
-    int i = 0;
+    int i;
 
     if(!bag->attribs) {
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	return NULL;
     }
 
-    while(bag->attribs[i] != NULL) {
-	if(SECOID_FindOIDTag(&bag->attribs[i]->attrType) 
-			== attributeType) {
+    for (i = 0; bag->attribs[i] != NULL; i++) {
+	if (SECOID_FindOIDTag(&bag->attribs[i]->attrType) == attributeType) {
 	    return bag->attribs[i]->attrValue[0];
 	}
-	i++;
     }
-
     return NULL;
 }
 
 /* For now, this function will merely remove any ":"
  * in the nickname which the PK11 functions may have
  * placed there.  This will keep dual certs from appearing
  * twice under "Your" certificates when imported onto smart
  * cards.  Once with the name "Slot:Cert" and another with
@@ -1710,91 +1735,83 @@ loser:
     bag->problem = PR_TRUE;
     bag->error = PORT_GetError();
     return NULL;
 }
 
 static SECStatus
 sec_pkcs12_set_nickname(sec_PKCS12SafeBag *bag, SECItem *name)
 {
-    int i = 0;
     sec_PKCS12Attribute *attr = NULL;
     SECOidData *oid = SECOID_FindOIDByTag(SEC_OID_PKCS9_FRIENDLY_NAME);
 
     if(!bag || !bag->arena || !name) {
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	return SECFailure;
     }
 	
     if(!bag->attribs) {
 	if(!oid) {
 	    goto loser;
 	}
 
-	bag->attribs = (sec_PKCS12Attribute**)PORT_ArenaZAlloc(bag->arena, 
-					     sizeof(sec_PKCS12Attribute *)*2);
+	bag->attribs = 
+	    PORT_ArenaZNewArray(bag->arena, sec_PKCS12Attribute *, 2);
 	if(!bag->attribs) {
 	    goto loser;
 	}
-	bag->attribs[0] = (sec_PKCS12Attribute*)PORT_ArenaZAlloc(bag->arena, 
-						  sizeof(sec_PKCS12Attribute));
+	bag->attribs[0] = PORT_ArenaZNew(bag->arena, sec_PKCS12Attribute);
 	if(!bag->attribs[0]) {
 	    goto loser;
 	}
 	bag->attribs[1] = NULL;
 
 	attr = bag->attribs[0];
 	if(SECITEM_CopyItem(bag->arena, &attr->attrType, &oid->oid) 
 			!= SECSuccess) {
 	    goto loser;
 	}
     } else {
-	while(bag->attribs[i]) {
+	int i;
+	for (i = 0; bag->attribs[i]; i++) {
 	    if(SECOID_FindOIDTag(&bag->attribs[i]->attrType)
 			== SEC_OID_PKCS9_FRIENDLY_NAME) {
 		attr = bag->attribs[i];
 		break;
 	    }
-	    i++;
 	}
 	if(!attr) {
 	    if(!oid) {
 		goto loser;
 	    }
-	    bag->attribs = (sec_PKCS12Attribute **)PORT_ArenaGrow(bag->arena, 
-								  bag->attribs,
-					(i+1) * sizeof(sec_PKCS12Attribute *),
-					(i+2) * sizeof(sec_PKCS12Attribute *));
+	    bag->attribs = PORT_ArenaGrowArray(bag->arena, bag->attribs,
+					       sec_PKCS12Attribute *, i+1, i+2);
 	    if(!bag->attribs) {
 		goto loser;
 	    }
-	    bag->attribs[i] = 
-	        (sec_PKCS12Attribute *)PORT_ArenaZAlloc(bag->arena, 
-						  sizeof(sec_PKCS12Attribute));
+	    bag->attribs[i] = PORT_ArenaZNew(bag->arena, sec_PKCS12Attribute);
 	    if(!bag->attribs[i]) {
 		goto loser;
 	    }
 	    bag->attribs[i+1] = NULL;
 	    attr = bag->attribs[i];
 	    if(SECITEM_CopyItem(bag->arena, &attr->attrType, &oid->oid) 
 				!= SECSuccess) {
 		goto loser;
 	    }
 	}
     }
 
     PORT_Assert(attr);
     if(!attr->attrValue) {
-	attr->attrValue = (SECItem **)PORT_ArenaZAlloc(bag->arena, 
-						       sizeof(SECItem *) * 2);
+	attr->attrValue = PORT_ArenaZNewArray(bag->arena, SECItem *, 2);
 	if(!attr->attrValue) {
 	    goto loser;
 	}
-	attr->attrValue[0] = (SECItem*)PORT_ArenaZAlloc(bag->arena, 
-							sizeof(SECItem));
+	attr->attrValue[0] = PORT_ArenaZNew(bag->arena, SECItem);
 	if(!attr->attrValue[0]) {
 	    goto loser;
 	}
 	attr->attrValue[1] = NULL;
     }
 
     name->len = PORT_Strlen((char *)name->data);
     if(!sec_pkcs12_convert_item_to_unicode(bag->arena, attr->attrValue[0],
@@ -1989,32 +2006,28 @@ gatherNicknames(CERTCertificate *cert, v
 	    if(SECITEM_CompareItem(nickArg->nickList[i], &tempNick) 
 				== SECEqual) {
 		return SECSuccess;
 	    }
 	}
     }
 
     /* add the nickname to the list */
-    if(nickArg->nNicks == 0) {
-	nickArg->nickList = (SECItem **)PORT_ArenaZAlloc(nickArg->arena, 
-					     2 * sizeof(SECItem *));
-    } else {
-	nickArg->nickList = (SECItem **)PORT_ArenaGrow(nickArg->arena,
-				nickArg->nickList, 
-				(nickArg->nNicks + 1) * sizeof(SECItem *),
-				(nickArg->nNicks + 2) * sizeof(SECItem *));
-    }
+    nickArg->nickList = (nickArg->nNicks == 0) 
+	? PORT_ArenaZNewArray(nickArg->arena, SECItem *, 2)
+	: PORT_ArenaGrowArray(nickArg->arena, nickArg->nickList, SECItem *, 
+	                      nickArg->nNicks + 1, nickArg->nNicks + 2);
+
     if(!nickArg->nickList) {
 	nickArg->error = SEC_ERROR_NO_MEMORY;
 	return SECFailure;
     }
 
     nickArg->nickList[nickArg->nNicks] = 
-        (SECItem *)PORT_ArenaZAlloc(nickArg->arena, sizeof(SECItem));
+				    PORT_ArenaZNew(nickArg->arena, SECItem);
     if(!nickArg->nickList[nickArg->nNicks]) {
 	nickArg->error = PORT_GetError();
 	return SECFailure;
     }
     
 
     if(SECITEM_CopyItem(nickArg->arena, nickArg->nickList[nickArg->nNicks],
 			&tempNick) != SECSuccess) {
@@ -2055,18 +2068,17 @@ sec_pkcs12_get_existing_nick_for_dn(sec_
 	goto loser;
     }
 
     arena = PORT_NewArena(1024);
     if(!arena) {
 	returnDn = NULL;
 	goto loser;
     }
-    nickArg = (struct certNickInfo *)PORT_ArenaZAlloc(arena, 
-						 sizeof(struct certNickInfo));
+    nickArg = PORT_ArenaZNew(arena, struct certNickInfo);
     if(!nickArg) {
 	returnDn = NULL;
 	goto loser;
     }
     nickArg->error = 0;
     nickArg->nNicks = 0;
     nickArg->nickList = NULL;
     nickArg->arena = arena;
@@ -2512,25 +2524,22 @@ sec_pkcs12_add_item_to_bag_list(sec_PKCS
     int i = 0;
 
     if(!bagList || !bag) {
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	return SECFailure;
     }
 
     if(!(*bagList)) {
-	newBagList = (sec_PKCS12SafeBag **)PORT_ArenaZAlloc(bag->arena, 
-				      sizeof(sec_PKCS12SafeBag *) * 2);
+	newBagList = PORT_ArenaZNewArray(bag->arena, sec_PKCS12SafeBag *, 2);
     } else {
 	while((*bagList)[i]) 
 	    i++;
-	newBagList = (sec_PKCS12SafeBag **)PORT_ArenaGrow(bag->arena, 
-	                        *bagList,
-				sizeof(sec_PKCS12SafeBag *) * (i + 1),
-				sizeof(sec_PKCS12SafeBag *) * (i + 2));
+	newBagList = PORT_ArenaGrowArray(bag->arena, *bagList,
+				         sec_PKCS12SafeBag *, i + 1, i + 2);
     }
 
     if(!newBagList) {
 	PORT_SetError(SEC_ERROR_NO_MEMORY);
 	return SECFailure;
     }
 
     newBagList[i]   = bag;
@@ -3141,25 +3150,21 @@ static SECStatus
 sec_pkcs12_decoder_append_bag_to_context(SEC_PKCS12DecoderContext *p12dcx,
 					 sec_PKCS12SafeBag *bag)
 {
     if(!p12dcx || p12dcx->error) {
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	return SECFailure;
     }
 
-    if(!p12dcx->safeBagCount) {
-	p12dcx->safeBags = (sec_PKCS12SafeBag **)PORT_ArenaZAlloc(p12dcx->arena, 
-					    sizeof(sec_PKCS12SafeBag *) * 2);
-    } else {
-	p12dcx->safeBags = 
-	  (sec_PKCS12SafeBag **)PORT_ArenaGrow(p12dcx->arena, p12dcx->safeBags,
-		     (p12dcx->safeBagCount + 1) * sizeof(sec_PKCS12SafeBag *),
-		     (p12dcx->safeBagCount + 2) * sizeof(sec_PKCS12SafeBag *));
-    }
+    p12dcx->safeBags = !p12dcx->safeBagCount 
+	? PORT_ArenaZNewArray(p12dcx->arena, sec_PKCS12SafeBag *, 2)
+	: PORT_ArenaGrowArray(p12dcx->arena, p12dcx->safeBags, 
+	                      sec_PKCS12SafeBag *, p12dcx->safeBagCount + 1, 
+			      p12dcx->safeBagCount + 2);
 
     if(!p12dcx->safeBags) {
 	PORT_SetError(SEC_ERROR_NO_MEMORY);
 	return SECFailure;
     }
 
     p12dcx->safeBags[p12dcx->safeBagCount] = bag;
     p12dcx->safeBags[p12dcx->safeBagCount+1] = NULL;
@@ -3177,19 +3182,18 @@ sec_pkcs12_decoder_convert_old_key(SEC_P
     SECOidTag keyTag;
     SECItem *keyID, *nickName, *newNickName;
 
     if(!p12dcx || p12dcx->error || !key) {
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	return NULL;
     }
 
-    newNickName =(SECItem *)PORT_ArenaZAlloc(p12dcx->arena, sizeof(SECItem));
-    keyBag = (sec_PKCS12SafeBag *)PORT_ArenaZAlloc(p12dcx->arena, 
-						   sizeof(sec_PKCS12SafeBag));
+    newNickName = PORT_ArenaZNew(p12dcx->arena, SECItem);
+    keyBag      = PORT_ArenaZNew(p12dcx->arena, sec_PKCS12SafeBag);
     if(!keyBag || !newNickName) {
 	return NULL;
     }
 
     keyBag->swapUnicodeBytes = p12dcx->swapUnicodeBytes;
     keyBag->slot = p12dcx->slot;
     keyBag->arena = p12dcx->arena;
     keyBag->pwitem = p12dcx->pwitem;
@@ -3279,17 +3283,17 @@ sec_pkcs12_decoder_create_cert(SEC_PKCS1
     SECItem *keyId;
     SECStatus rv;
 
     if(!p12dcx || p12dcx->error || !derCert) {
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	return NULL;
     }
 
-    keyId = (SECItem *)PORT_ArenaZAlloc(p12dcx->arena, sizeof(SECItem));
+    keyId = PORT_ArenaZNew(p12dcx->arena, SECItem);
     if(!keyId) {
 	return NULL;
     }
 
     digest = sec_pkcs12_compute_thumbprint(derCert);
     if(!digest) {
 	return NULL;
     }
@@ -3297,33 +3301,31 @@ sec_pkcs12_decoder_create_cert(SEC_PKCS1
     rv = SECITEM_CopyItem(p12dcx->arena, keyId, &digest->digest);
     SGN_DestroyDigestInfo(digest);
     if(rv != SECSuccess) {
 	PORT_SetError(SEC_ERROR_NO_MEMORY);
 	return NULL;
     }
 
     oid = SECOID_FindOIDByTag(SEC_OID_PKCS12_V1_CERT_BAG_ID);
-    certBag = (sec_PKCS12SafeBag *)PORT_ArenaZAlloc(p12dcx->arena, 
-						    sizeof(sec_PKCS12SafeBag));
+    certBag = PORT_ArenaZNew(p12dcx->arena, sec_PKCS12SafeBag);
     if(!certBag || !oid || (SECITEM_CopyItem(p12dcx->arena, 
 			&certBag->safeBagType, &oid->oid) != SECSuccess)) {
 	return NULL;
     }
 
     certBag->slot = p12dcx->slot;
     certBag->pwitem = p12dcx->pwitem;
     certBag->swapUnicodeBytes = p12dcx->swapUnicodeBytes;
     certBag->arena = p12dcx->arena;
     certBag->tokenCAs = p12dcx->tokenCAs;
 
     oid = SECOID_FindOIDByTag(SEC_OID_PKCS9_X509_CERT);
     certBag->safeBagContent.certBag = 
-        (sec_PKCS12CertBag *)PORT_ArenaZAlloc(p12dcx->arena, 
-					      sizeof(sec_PKCS12CertBag));
+			PORT_ArenaZNew(p12dcx->arena, sec_PKCS12CertBag);
     if(!certBag->safeBagContent.certBag || !oid ||
 			(SECITEM_CopyItem(p12dcx->arena, 
 				 &certBag->safeBagContent.certBag->bagID,
 				 &oid->oid) != SECSuccess)) {
 	return NULL;
     }
       
     if(SECITEM_CopyItem(p12dcx->arena, 
@@ -3356,18 +3358,17 @@ sec_pkcs12_decoder_convert_old_cert(SEC_
     derCertList = SEC_PKCS7GetCertificateList(&oldCert->value.x509->certOrCRL);
     if(!derCertList) {
 	return NULL;
     }
 
     i = 0;
     while(derCertList[i]) i++;
 
-    certList = (sec_PKCS12SafeBag **)PORT_ArenaZAlloc(p12dcx->arena, 
-				(i + 1) * sizeof(sec_PKCS12SafeBag *));
+    certList = PORT_ArenaZNewArray(p12dcx->arena, sec_PKCS12SafeBag *, (i + 1));
     if(!certList) {
 	return NULL;
     }
 
     for(j = 0; j < i; j++) {
 	certList[j] = sec_pkcs12_decoder_create_cert(p12dcx, derCertList[j]);
 	if(!certList[j]) {
 	    return NULL;
@@ -3530,18 +3531,17 @@ sec_PKCS12ConvertOldSafeToNew(PRArenaPoo
 	return NULL;
     }
 
     if(!safe && !baggage) {
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	return NULL;
     }
 
-    p12dcx = (SEC_PKCS12DecoderContext *)PORT_ArenaZAlloc(arena, 
-					    sizeof(SEC_PKCS12DecoderContext));
+    p12dcx = PORT_ArenaZNew(arena, SEC_PKCS12DecoderContext);
     if(!p12dcx) {
 	return NULL;
     }
 
     p12dcx->arena = arena;
     p12dcx->slot = PK11_ReferenceSlot(slot);
     p12dcx->wincx = wincx;
     p12dcx->error = PR_FALSE;