Bug 488241: Fix cert7/cert8.db issues with large issuer names in NSS_3_11_BRANCH as well NSS_3_11_BRANCH
authornelson%bolyard.com
Wed, 15 Apr 2009 08:36:58 +0000
branchNSS_3_11_BRANCH
changeset 9210 25b2922cc564265648138f3219f83879e2c92765
parent 9166 b960f7144e7df65007705b5e2cd6e17124755c0f
push idunknown
push userunknown
push dateunknown
bugs488241
Bug 488241: Fix cert7/cert8.db issues with large issuer names in NSS_3_11_BRANCH as well Patch contributed by Kaspar Brand <mozbugzilla@velox.ch>
security/nss/lib/softoken/lowcert.c
security/nss/lib/softoken/pcertdb.c
security/nss/lib/softoken/pcertt.h
--- a/security/nss/lib/softoken/lowcert.c
+++ b/security/nss/lib/softoken/lowcert.c
@@ -364,16 +364,20 @@ nsslowcert_KeyFromIssuerAndSN(PRArenaPoo
 			      SECItem *issuer, SECItem *sn, SECItem *key)
 {
     unsigned int len = sn->len + issuer->len;
 
     if (!arena) {
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	goto loser;
     }
+    if (len > NSS_MAX_LEGACY_DB_KEY_SIZE) {
+	PORT_SetError(SEC_ERROR_INPUT_LEN);
+	goto loser;
+    }
     key->data = (unsigned char*)PORT_ArenaAlloc(arena, len);
     if ( !key->data ) {
 	goto loser;
     }
 
     key->len = len;
     /* copy the serialNumber */
     PORT_Memcpy(key->data, sn->data, sn->len);
--- a/security/nss/lib/softoken/pcertdb.c
+++ b/security/nss/lib/softoken/pcertdb.c
@@ -644,26 +644,26 @@ EncodeDBCertEntry(certDBEntryCert *entry
     if ( dbitem->data == NULL) {
 	PORT_SetError(SEC_ERROR_NO_MEMORY);
 	goto loser;
     }
     
     /* fill in database record */
     buf = &dbitem->data[SEC_DB_ENTRY_HEADER_LEN];
     
-    buf[0] = ( entry->trust.sslFlags >> 8 ) & 0xff;
-    buf[1] = entry->trust.sslFlags & 0xff;
-    buf[2] = ( entry->trust.emailFlags >> 8 ) & 0xff;
-    buf[3] = entry->trust.emailFlags & 0xff;
-    buf[4] = ( entry->trust.objectSigningFlags >> 8 ) & 0xff;
-    buf[5] = entry->trust.objectSigningFlags & 0xff;
-    buf[6] = ( entry->derCert.len >> 8 ) & 0xff;
-    buf[7] = entry->derCert.len & 0xff;
-    buf[8] = ( nnlen >> 8 ) & 0xff;
-    buf[9] = nnlen & 0xff;
+    buf[0] = (PRUint8)( entry->trust.sslFlags >> 8 );
+    buf[1] = (PRUint8)( entry->trust.sslFlags      );
+    buf[2] = (PRUint8)( entry->trust.emailFlags >> 8 );
+    buf[3] = (PRUint8)( entry->trust.emailFlags      );
+    buf[4] = (PRUint8)( entry->trust.objectSigningFlags >> 8 );
+    buf[5] = (PRUint8)( entry->trust.objectSigningFlags      );
+    buf[6] = (PRUint8)( entry->derCert.len >> 8 );
+    buf[7] = (PRUint8)( entry->derCert.len      );
+    buf[8] = (PRUint8)( nnlen >> 8 );
+    buf[9] = (PRUint8)( nnlen      );
     
     PORT_Memcpy(&buf[DB_CERT_ENTRY_HEADER_LEN], entry->derCert.data,
 	      entry->derCert.len);
 
     PORT_Memcpy(&buf[DB_CERT_ENTRY_HEADER_LEN + entry->derCert.len],
 	      nn, nnlen);
 
     return(SECSuccess);
@@ -674,16 +674,18 @@ loser:
 
 /*
  * encode a database key for a cert record
  */
 static SECStatus
 EncodeDBCertKey(SECItem *certKey, PRArenaPool *arena, SECItem *dbkey)
 {
     unsigned int len = certKey->len + SEC_DB_KEY_HEADER_LEN;
+    if (len > NSS_MAX_LEGACY_DB_KEY_SIZE)
+	goto loser;
     if (arena) {
 	dbkey->data = (unsigned char *)PORT_ArenaAlloc(arena, len);
     } else {
 	if (dbkey->len < len) {
 	    dbkey->data = (unsigned char *)PORT_Alloc(len);
 	}
     }
     dbkey->len = len;
@@ -713,22 +715,24 @@ EncodeDBGenericKey(SECItem *certKey, PRA
 	    goto loser;
 	}
         dbkey->data[0] = (unsigned char) entryType;
         return(SECSuccess);
     }
     
 
     dbkey->len = certKey->len + SEC_DB_KEY_HEADER_LEN;
+    if (dbkey->len > NSS_MAX_LEGACY_DB_KEY_SIZE)
+	goto loser;
     dbkey->data = (unsigned char *)PORT_ArenaAlloc(arena, dbkey->len);
     if ( dbkey->data == NULL ) {
 	goto loser;
     }
     PORT_Memcpy(&dbkey->data[SEC_DB_KEY_HEADER_LEN],
-	      certKey->data, certKey->len);
+	       certKey->data, certKey->len);
     dbkey->data[0] = (unsigned char) entryType;
 
     return(SECSuccess);
 loser:
     return(SECFailure);
 }
 
 static SECStatus
@@ -768,24 +772,27 @@ DecodeDBCertEntry(certDBEntryCert *entry
 	PORT_SetError(SEC_ERROR_BAD_DATABASE);
 	goto loser;
     }
     
     /* is database entry correct length? */
     entry->derCert.len = ( ( dbentry->data[lenoff] << 8 ) |
 			  dbentry->data[lenoff+1] );
     nnlen = ( ( dbentry->data[lenoff+2] << 8 ) | dbentry->data[lenoff+3] );
-    if ( ( entry->derCert.len + nnlen + headerlen )
-	!= dbentry->len) {
-	PORT_SetError(SEC_ERROR_BAD_DATABASE);
-	goto loser;
+    lenoff = dbentry->len - ( entry->derCert.len + nnlen + headerlen );
+    if ( lenoff ) {
+	if ( lenoff < 0 || (lenoff & 0xffff) != 0 ) {
+	    PORT_SetError(SEC_ERROR_BAD_DATABASE);
+	    goto loser;
+	}
+	/* The cert size exceeded 64KB.  Reconstruct the correct length. */
+	entry->derCert.len += lenoff;
     }
     
     /* copy the dercert */
-
     entry->derCert.data = pkcs11_copyStaticData(&dbentry->data[headerlen],
 	entry->derCert.len,entry->derCertSpace,sizeof(entry->derCertSpace));
     if ( entry->derCert.data == NULL ) {
 	PORT_SetError(SEC_ERROR_NO_MEMORY);
 	goto loser;
     }
 
     /* copy the nickname */
@@ -1160,20 +1167,20 @@ EncodeDBCrlEntry(certDBEntryRevocation *
     if ( dbitem->data == NULL) {
 	PORT_SetError(SEC_ERROR_NO_MEMORY);
 	goto loser;
     }
     
     /* fill in database record */
     buf = &dbitem->data[SEC_DB_ENTRY_HEADER_LEN];
     
-    buf[0] = ( entry->derCrl.len >> 8 ) & 0xff;
-    buf[1] = entry->derCrl.len & 0xff;
-    buf[2] = ( nnlen >> 8 ) & 0xff;
-    buf[3] = nnlen & 0xff;
+    buf[0] = (PRUint8)( entry->derCrl.len >> 8 );
+    buf[1] = (PRUint8)( entry->derCrl.len      );
+    buf[2] = (PRUint8)( nnlen >> 8 );
+    buf[3] = (PRUint8)( nnlen      );
     
     PORT_Memcpy(&buf[DB_CRL_ENTRY_HEADER_LEN], entry->derCrl.data,
 	      entry->derCrl.len);
 
     if (nnlen != 0) {
 	PORT_Memcpy(&buf[DB_CRL_ENTRY_HEADER_LEN + entry->derCrl.len],
 	      entry->url, nnlen);
     }
@@ -1182,60 +1189,60 @@ EncodeDBCrlEntry(certDBEntryRevocation *
 
 loser:
     return(SECFailure);
 }
 
 static SECStatus
 DecodeDBCrlEntry(certDBEntryRevocation *entry, SECItem *dbentry)
 {
-    unsigned int nnlen;
-    
+    unsigned int urlLen;
+    int lenDiff;
+
     /* is record long enough for header? */
     if ( dbentry->len < DB_CRL_ENTRY_HEADER_LEN ) {
 	PORT_SetError(SEC_ERROR_BAD_DATABASE);
 	goto loser;
     }
     
     /* is database entry correct length? */
     entry->derCrl.len = ( ( dbentry->data[0] << 8 ) | dbentry->data[1] );
-    nnlen = ( ( dbentry->data[2] << 8 ) | dbentry->data[3] );
-    if ( ( entry->derCrl.len + nnlen + DB_CRL_ENTRY_HEADER_LEN )
-	!= dbentry->len) {
-      /* CRL entry is greater than 64 K. Hack to make this continue to work */
-      if (dbentry->len >= (0xffff - DB_CRL_ENTRY_HEADER_LEN) - nnlen) {
-          entry->derCrl.len = 
-                      (dbentry->len - DB_CRL_ENTRY_HEADER_LEN) - nnlen;
-      } else {
-          PORT_SetError(SEC_ERROR_BAD_DATABASE);
-          goto loser;
-      }    
-    }
-    
-    /* copy the dercert */
+    urlLen =            ( ( dbentry->data[2] << 8 ) | dbentry->data[3] );
+    lenDiff = dbentry->len - 
+			(entry->derCrl.len + urlLen + DB_CRL_ENTRY_HEADER_LEN);
+    if (lenDiff) {
+    	if (lenDiff < 0 || (lenDiff & 0xffff) != 0) {
+	    PORT_SetError(SEC_ERROR_BAD_DATABASE);
+	    goto loser;
+	}    
+	/* CRL entry is greater than 64 K. Hack to make this continue to work */
+	entry->derCrl.len += lenDiff;
+    }
+    
+    /* copy the der CRL */
     entry->derCrl.data = (unsigned char *)PORT_ArenaAlloc(entry->common.arena,
 							 entry->derCrl.len);
     if ( entry->derCrl.data == NULL ) {
 	PORT_SetError(SEC_ERROR_NO_MEMORY);
 	goto loser;
     }
     PORT_Memcpy(entry->derCrl.data, &dbentry->data[DB_CRL_ENTRY_HEADER_LEN],
 	      entry->derCrl.len);
 
     /* copy the url */
     entry->url = NULL;
-    if (nnlen != 0) {
-	entry->url = (char *)PORT_ArenaAlloc(entry->common.arena, nnlen);
+    if (urlLen != 0) {
+	entry->url = (char *)PORT_ArenaAlloc(entry->common.arena, urlLen);
 	if ( entry->url == NULL ) {
 	    PORT_SetError(SEC_ERROR_NO_MEMORY);
 	    goto loser;
 	}
 	PORT_Memcpy(entry->url,
 	      &dbentry->data[DB_CRL_ENTRY_HEADER_LEN + entry->derCrl.len],
-	      nnlen);
+	      urlLen);
     }
     
     return(SECSuccess);
 loser:
     return(SECFailure);
 }
 
 /*
@@ -1456,28 +1463,25 @@ EncodeDBNicknameEntry(certDBEntryNicknam
 {
     unsigned char *buf;
     
     /* allocate space for encoded database record, including space
      * for low level header
      */
     dbitem->len = entry->subjectName.len + DB_NICKNAME_ENTRY_HEADER_LEN +
 	SEC_DB_ENTRY_HEADER_LEN;
-    
     dbitem->data = (unsigned char *)PORT_ArenaAlloc(arena, dbitem->len);
     if ( dbitem->data == NULL) {
 	goto loser;
     }
     
     /* fill in database record */
     buf = &dbitem->data[SEC_DB_ENTRY_HEADER_LEN];
-    
-    buf[0] = ( entry->subjectName.len >> 8 ) & 0xff;
-    buf[1] = entry->subjectName.len & 0xff;
-    
+    buf[0] = (PRUint8)( entry->subjectName.len >> 8 );
+    buf[1] = (PRUint8)( entry->subjectName.len      );
     PORT_Memcpy(&buf[DB_NICKNAME_ENTRY_HEADER_LEN], entry->subjectName.data,
 	      entry->subjectName.len);
 
     return(SECSuccess);
 
 loser:
     return(SECFailure);
 }
@@ -1490,16 +1494,18 @@ EncodeDBNicknameKey(char *nickname, PRAr
 		    SECItem *dbkey)
 {
     unsigned int nnlen;
     
     nnlen = PORT_Strlen(nickname) + 1; /* includes null */
 
     /* now get the database key and format it */
     dbkey->len = nnlen + SEC_DB_KEY_HEADER_LEN;
+    if (dbkey->len > NSS_MAX_LEGACY_DB_KEY_SIZE)
+	goto loser;
     dbkey->data = (unsigned char *)PORT_ArenaAlloc(arena, dbkey->len);
     if ( dbkey->data == NULL ) {
 	goto loser;
     }
     PORT_Memcpy(&dbkey->data[SEC_DB_KEY_HEADER_LEN], nickname, nnlen);
     dbkey->data[0] = certDBEntryTypeNickname;
 
     return(SECSuccess);
@@ -1507,30 +1513,37 @@ EncodeDBNicknameKey(char *nickname, PRAr
 loser:
     return(SECFailure);
 }
 
 static SECStatus
 DecodeDBNicknameEntry(certDBEntryNickname *entry, SECItem *dbentry,
                       char *nickname)
 {
+    int lenDiff;
+
     /* is record long enough for header? */
     if ( dbentry->len < DB_NICKNAME_ENTRY_HEADER_LEN ) {
 	PORT_SetError(SEC_ERROR_BAD_DATABASE);
 	goto loser;
     }
     
     /* is database entry correct length? */
     entry->subjectName.len = ( ( dbentry->data[0] << 8 ) | dbentry->data[1] );
-    if (( entry->subjectName.len + DB_NICKNAME_ENTRY_HEADER_LEN ) !=
-	dbentry->len ){
-	PORT_SetError(SEC_ERROR_BAD_DATABASE);
-	goto loser;
-    }
-    
+    lenDiff = dbentry->len - 
+	      (entry->subjectName.len + DB_NICKNAME_ENTRY_HEADER_LEN);
+    if (lenDiff) {
+	if (lenDiff < 0 || (lenDiff & 0xffff) != 0 ) { 
+	    PORT_SetError(SEC_ERROR_BAD_DATABASE);
+	    goto loser;
+	}
+	/* The entry size exceeded 64KB.  Reconstruct the correct length. */
+	entry->subjectName.len += lenDiff;
+    }
+
     /* copy the certkey */
     entry->subjectName.data =
 	(unsigned char *)PORT_ArenaAlloc(entry->common.arena,
 					 entry->subjectName.len);
     if ( entry->subjectName.data == NULL ) {
 	PORT_SetError(SEC_ERROR_NO_MEMORY);
 	goto loser;
     }
@@ -1774,22 +1787,22 @@ EncodeDBSMimeEntry(certDBEntrySMime *ent
     if ( dbitem->data == NULL) {
 	PORT_SetError(SEC_ERROR_NO_MEMORY);
 	goto loser;
     }
     
     /* fill in database record */
     buf = &dbitem->data[SEC_DB_ENTRY_HEADER_LEN];
     
-    buf[0] = ( entry->subjectName.len >> 8 ) & 0xff;
-    buf[1] = entry->subjectName.len & 0xff;
-    buf[2] = ( entry->smimeOptions.len >> 8 ) & 0xff;
-    buf[3] = entry->smimeOptions.len & 0xff;
-    buf[4] = ( entry->optionsDate.len >> 8 ) & 0xff;
-    buf[5] = entry->optionsDate.len & 0xff;
+    buf[0] = (PRUint8)( entry->subjectName.len >> 8 );
+    buf[1] = (PRUint8)( entry->subjectName.len      );
+    buf[2] = (PRUint8)( entry->smimeOptions.len >> 8 );
+    buf[3] = (PRUint8)( entry->smimeOptions.len      );
+    buf[4] = (PRUint8)( entry->optionsDate.len >> 8 );
+    buf[5] = (PRUint8)( entry->optionsDate.len      );
 
     /* if no smime options, then there should not be an options date either */
     PORT_Assert( ! ( ( entry->smimeOptions.len == 0 ) &&
 		    ( entry->optionsDate.len != 0 ) ) );
     
     PORT_Memcpy(&buf[DB_SMIME_ENTRY_HEADER_LEN], entry->subjectName.data,
 	      entry->subjectName.len);
     if ( entry->smimeOptions.len ) {
@@ -1816,16 +1829,18 @@ EncodeDBSMimeKey(char *emailAddr, PRAren
 		 SECItem *dbkey)
 {
     unsigned int addrlen;
     
     addrlen = PORT_Strlen(emailAddr) + 1; /* includes null */
 
     /* now get the database key and format it */
     dbkey->len = addrlen + SEC_DB_KEY_HEADER_LEN;
+    if (dbkey->len > NSS_MAX_LEGACY_DB_KEY_SIZE)
+	goto loser;
     dbkey->data = (unsigned char *)PORT_ArenaAlloc(arena, dbkey->len);
     if ( dbkey->data == NULL ) {
 	goto loser;
     }
     PORT_Memcpy(&dbkey->data[SEC_DB_KEY_HEADER_LEN], emailAddr, addrlen);
     dbkey->data[0] = certDBEntryTypeSMimeProfile;
 
     return(SECSuccess);
@@ -1835,32 +1850,41 @@ loser:
 }
 
 /*
  * Decode a database SMIME record
  */
 static SECStatus
 DecodeDBSMimeEntry(certDBEntrySMime *entry, SECItem *dbentry, char *emailAddr)
 {
+    int lenDiff;
+
     /* is record long enough for header? */
     if ( dbentry->len < DB_SMIME_ENTRY_HEADER_LEN ) {
 	PORT_SetError(SEC_ERROR_BAD_DATABASE);
 	goto loser;
     }
     
     /* is database entry correct length? */
-    entry->subjectName.len = ( ( dbentry->data[0] << 8 ) | dbentry->data[1] );
-    entry->smimeOptions.len = ( ( dbentry->data[2] << 8 ) | dbentry->data[3] );
-    entry->optionsDate.len = ( ( dbentry->data[4] << 8 ) | dbentry->data[5] );
-    if (( entry->subjectName.len + entry->smimeOptions.len +
-	 entry->optionsDate.len + DB_SMIME_ENTRY_HEADER_LEN ) != dbentry->len){
-	PORT_SetError(SEC_ERROR_BAD_DATABASE);
-	goto loser;
-    }
-    
+    entry->subjectName.len  = (( dbentry->data[0] << 8 ) | dbentry->data[1] );
+    entry->smimeOptions.len = (( dbentry->data[2] << 8 ) | dbentry->data[3] );
+    entry->optionsDate.len  = (( dbentry->data[4] << 8 ) | dbentry->data[5] );
+    lenDiff = dbentry->len - (entry->subjectName.len + 
+                              entry->smimeOptions.len + 
+			      entry->optionsDate.len + 
+			      DB_SMIME_ENTRY_HEADER_LEN);
+    if (lenDiff) {
+	if (lenDiff < 0 || (lenDiff & 0xffff) != 0 ) { 
+	    PORT_SetError(SEC_ERROR_BAD_DATABASE);
+	    goto loser;
+	}
+	/* The entry size exceeded 64KB.  Reconstruct the correct length. */
+	entry->subjectName.len += lenDiff;
+    }
+
     /* copy the subject name */
     entry->subjectName.data =
 	(unsigned char *)PORT_ArenaAlloc(entry->common.arena,
 					 entry->subjectName.len);
     if ( entry->subjectName.data == NULL ) {
 	PORT_SetError(SEC_ERROR_NO_MEMORY);
 	goto loser;
     }
@@ -2155,97 +2179,96 @@ EncodeDBSubjectEntry(certDBEntrySubject 
     unsigned char *buf;
     int len;
     unsigned int ncerts;
     unsigned int i;
     unsigned char *tmpbuf;
     unsigned int nnlen = 0;
     unsigned int eaddrslen = 0;
     int keyidoff;
-    SECItem *certKeys;
-    SECItem *keyIDs;
+    SECItem *certKeys = entry->certKeys;
+    SECItem *keyIDs   = entry->keyIDs;;
     
     if ( entry->nickname ) {
 	nnlen = PORT_Strlen(entry->nickname) + 1;
     }
     if ( entry->emailAddrs ) {
 	eaddrslen = 2;
 	for (i=0; i < entry->nemailAddrs; i++) {
 	    eaddrslen += PORT_Strlen(entry->emailAddrs[i]) + 1 + 2;
 	}
     }
 
     ncerts = entry->ncerts;
     
     /* compute the length of the entry */
     keyidoff = DB_SUBJECT_ENTRY_HEADER_LEN + nnlen ;
-    len = keyidoff + 4 * ncerts + eaddrslen;
+    len = keyidoff + (4 * ncerts) + eaddrslen;
     for ( i = 0; i < ncerts; i++ ) {
-	len += entry->certKeys[i].len;
-	len += entry->keyIDs[i].len;
+	if (keyIDs[i].len   > 0xffff ||
+	   (certKeys[i].len > 0xffff)) {
+    	    PORT_SetError(SEC_ERROR_INPUT_LEN);
+	    goto loser;
+	}
+	len += certKeys[i].len;
+	len += keyIDs[i].len;
     }
     
     /* allocate space for encoded database record, including space
      * for low level header
      */
     dbitem->len = len + SEC_DB_ENTRY_HEADER_LEN;
     
     dbitem->data = (unsigned char *)PORT_ArenaAlloc(arena, dbitem->len);
     if ( dbitem->data == NULL) {
 	PORT_SetError(SEC_ERROR_NO_MEMORY);
 	goto loser;
     }
     
     /* fill in database record */
     buf = &dbitem->data[SEC_DB_ENTRY_HEADER_LEN];
     
-    buf[0] = ( ncerts >> 8 ) & 0xff;
-    buf[1] = ncerts & 0xff;
-    buf[2] = ( nnlen >> 8 ) & 0xff;
-    buf[3] = nnlen & 0xff;
+    buf[0] = (PRUint8)( ncerts >> 8 );
+    buf[1] = (PRUint8)( ncerts      );
+    buf[2] = (PRUint8)( nnlen >> 8 );
+    buf[3] = (PRUint8)( nnlen      );
     /* v7 email field is NULL in v8 */
     buf[4] = 0;
     buf[5] = 0;
 
     PORT_Memcpy(&buf[DB_SUBJECT_ENTRY_HEADER_LEN], entry->nickname, nnlen);
-    
+    tmpbuf = &buf[keyidoff];   
     for ( i = 0; i < ncerts; i++ ) {
-
-	certKeys = entry->certKeys;
-	keyIDs = entry->keyIDs;
-
-	buf[keyidoff+i*2] = ( certKeys[i].len >> 8 ) & 0xff;
-	buf[keyidoff+1+i*2] = certKeys[i].len & 0xff;
-	buf[keyidoff+ncerts*2+i*2] = ( keyIDs[i].len >> 8 ) & 0xff;
-	buf[keyidoff+1+ncerts*2+i*2] = keyIDs[i].len & 0xff;
-    }
-    
-    /* temp pointer used to stuff certkeys and keyids into the buffer */
-    tmpbuf = &buf[keyidoff+ncerts*4];
-
+	tmpbuf[0] = (PRUint8)( certKeys[i].len >> 8 );
+	tmpbuf[1] = (PRUint8)( certKeys[i].len      );
+	tmpbuf += 2;
+    }
     for ( i = 0; i < ncerts; i++ ) {
-	certKeys = entry->certKeys;
-	PORT_Memcpy(tmpbuf, certKeys[i].data, certKeys[i].len);
-	tmpbuf = tmpbuf + certKeys[i].len;
+	tmpbuf[0] = (PRUint8)( keyIDs[i].len >> 8 );
+	tmpbuf[1] = (PRUint8)( keyIDs[i].len      );
+	tmpbuf += 2;
     }
     
     for ( i = 0; i < ncerts; i++ ) {
-	keyIDs = entry->keyIDs;
+	PORT_Memcpy(tmpbuf, certKeys[i].data, certKeys[i].len);
+	tmpbuf += certKeys[i].len;
+    }
+    for ( i = 0; i < ncerts; i++ ) {
 	PORT_Memcpy(tmpbuf, keyIDs[i].data, keyIDs[i].len);
-	tmpbuf = tmpbuf + keyIDs[i].len;
+	tmpbuf += keyIDs[i].len;
     }
 
     if (entry->emailAddrs) {
-	tmpbuf[0] =  (entry->nemailAddrs >> 8) & 0xff;
-	tmpbuf[1] =  entry->nemailAddrs  & 0xff;
+	tmpbuf[0] = (PRUint8)( entry->nemailAddrs >> 8 );
+	tmpbuf[1] = (PRUint8)( entry->nemailAddrs      );
 	tmpbuf += 2;
 	for (i=0; i < entry->nemailAddrs; i++) {
 	    int nameLen = PORT_Strlen(entry->emailAddrs[i]) + 1;
-	    tmpbuf[0] =  (nameLen >> 8) & 0xff;
-	    tmpbuf[1] =  nameLen & 0xff;
+	    tmpbuf[0] = (PRUint8)( nameLen >> 8 );
+	    tmpbuf[1] = (PRUint8)( nameLen      );
 	    tmpbuf += 2;
 	    PORT_Memcpy(tmpbuf,entry->emailAddrs[i],nameLen);
 	    tmpbuf +=nameLen;
 	}
     }
 
     PORT_Assert(tmpbuf == &buf[len]);
     
@@ -2258,16 +2281,18 @@ loser:
 /*
  * Encode a database key for a subject record
  */
 static SECStatus
 EncodeDBSubjectKey(SECItem *derSubject, PRArenaPool *arena,
 		   SECItem *dbkey)
 {
     dbkey->len = derSubject->len + SEC_DB_KEY_HEADER_LEN;
+    if (dbkey->len > NSS_MAX_LEGACY_DB_KEY_SIZE)
+	goto loser;
     dbkey->data = (unsigned char *)PORT_ArenaAlloc(arena, dbkey->len);
     if ( dbkey->data == NULL ) {
 	goto loser;
     }
     PORT_Memcpy(&dbkey->data[SEC_DB_KEY_HEADER_LEN], derSubject->data,
 	      derSubject->len);
     dbkey->data[0] = certDBEntryTypeSubject;
 
@@ -2276,54 +2301,51 @@ EncodeDBSubjectKey(SECItem *derSubject, 
 loser:
     return(SECFailure);
 }
 
 static SECStatus
 DecodeDBSubjectEntry(certDBEntrySubject *entry, SECItem *dbentry,
 		     SECItem *derSubject)
 {
-    unsigned int ncerts;
-    PRArenaPool *arena;
-    unsigned int len, itemlen;
+    PRArenaPool *arena     = entry->common.arena;
     unsigned char *tmpbuf;
     unsigned char *end;
+    void        *mark      = PORT_ArenaMark(arena);
+    unsigned int eaddrlen;
     unsigned int i;
-    SECStatus rv;
     unsigned int keyidoff;
-    unsigned int nnlen, eaddrlen;
-    unsigned int stdlen;
-    
-    arena = entry->common.arena;
+    unsigned int len;
+    unsigned int ncerts    = 0;
+    unsigned int nnlen;
+    SECStatus rv;
 
     rv = SECITEM_CopyItem(arena, &entry->derSubject, derSubject);
     if ( rv != SECSuccess ) {
 	goto loser;
     }
 
     /* is record long enough for header? */
     if ( dbentry->len < DB_SUBJECT_ENTRY_HEADER_LEN ) {
 	PORT_SetError(SEC_ERROR_BAD_DATABASE);
 	goto loser;
     }
     
-    entry->ncerts = ncerts = ( ( dbentry->data[0] << 8 ) | dbentry->data[1] );
-    nnlen = ( ( dbentry->data[2] << 8 ) | dbentry->data[3] );
-    eaddrlen = ( ( dbentry->data[4] << 8 ) | dbentry->data[5] );
-    stdlen = ncerts * 4 + DB_SUBJECT_ENTRY_HEADER_LEN + nnlen + eaddrlen;
-    if ( dbentry->len < stdlen) {
+    entry->ncerts = ncerts = (( dbentry->data[0] << 8 ) | dbentry->data[1] );
+    nnlen =                  (( dbentry->data[2] << 8 ) | dbentry->data[3] );
+    eaddrlen =               (( dbentry->data[4] << 8 ) | dbentry->data[5] );
+    keyidoff = DB_SUBJECT_ENTRY_HEADER_LEN + nnlen + eaddrlen;
+    len = keyidoff + (4 * ncerts);
+    if ( dbentry->len < len) {
 	PORT_SetError(SEC_ERROR_BAD_DATABASE);
 	goto loser;
     }
     
-    entry->certKeys = (SECItem *)PORT_ArenaAlloc(arena,
-						 sizeof(SECItem) * ncerts);
-    entry->keyIDs = (SECItem *)PORT_ArenaAlloc(arena,
-					       sizeof(SECItem) * ncerts);
-
+    entry->certKeys = PORT_ArenaNewArray(arena, SECItem, ncerts);
+    entry->keyIDs   = PORT_ArenaNewArray(arena, SECItem, ncerts);
     if ( ( entry->certKeys == NULL ) || ( entry->keyIDs == NULL ) ) {
 	PORT_SetError(SEC_ERROR_NO_MEMORY);
 	goto loser;
     }
 
     if ( nnlen > 1 ) { /* null terminator is stored */
 	entry->nickname = (char *)PORT_ArenaAlloc(arena, nnlen);
 	if ( entry->nickname == NULL ) {
@@ -2335,17 +2357,17 @@ DecodeDBSubjectEntry(certDBEntrySubject 
 		    nnlen);
     } else {
 	entry->nickname = NULL;
     }
 
     /* if we have an old style email entry, there is only one */    
     entry->nemailAddrs = 0;
     if ( eaddrlen > 1 ) { /* null terminator is stored */
-	entry->emailAddrs = (char **)PORT_ArenaAlloc(arena, sizeof(char *));
+	entry->emailAddrs = PORT_ArenaNewArray(arena, char *, 2);
 	if ( entry->emailAddrs == NULL ) {
 	    PORT_SetError(SEC_ERROR_NO_MEMORY);
 	    goto loser;
 	}
 	entry->emailAddrs[0] = (char *)PORT_ArenaAlloc(arena, eaddrlen);
 	if ( entry->emailAddrs[0] == NULL ) {
 	    PORT_SetError(SEC_ERROR_NO_MEMORY);
 	    goto loser;
@@ -2356,97 +2378,95 @@ DecodeDBSubjectEntry(certDBEntrySubject 
 	 entry->nemailAddrs = 1;
     } else {
 	entry->emailAddrs = NULL;
     }
     
     /* collect the lengths of the certKeys and keyIDs, and total the
      * overall length.
      */
-    keyidoff = DB_SUBJECT_ENTRY_HEADER_LEN + nnlen + eaddrlen;
-    len = keyidoff + 4 * ncerts;
-
-    tmpbuf = &dbentry->data[0];
-    
+    tmpbuf = &dbentry->data[keyidoff];
+    for ( i = 0; i < ncerts; i++ ) {
+        unsigned int itemlen = ( tmpbuf[0] << 8 ) | tmpbuf[1];
+        entry->certKeys[i].len = itemlen;
+        len += itemlen;
+        tmpbuf += 2;
+    }
     for ( i = 0; i < ncerts; i++ ) {
-
-	itemlen = ( tmpbuf[keyidoff + 2*i] << 8 ) | tmpbuf[keyidoff + 1 + 2*i] ;
-	len += itemlen;
-	entry->certKeys[i].len = itemlen;
-
-	itemlen = ( tmpbuf[keyidoff + 2*ncerts + 2*i] << 8 ) |
-	    tmpbuf[keyidoff + 1 + 2*ncerts + 2*i] ;
-	len += itemlen;
-	entry->keyIDs[i].len = itemlen;
-    }
-    
-    /* is database entry correct length? */
+        unsigned int itemlen = ( tmpbuf[0] << 8 ) | tmpbuf[1] ;
+        entry->keyIDs[i].len = itemlen;
+        len += itemlen;
+        tmpbuf += 2;
+    }
+
+    /* is encoded entry large enough ? */
     if ( len > dbentry->len ){
 	PORT_SetError(SEC_ERROR_BAD_DATABASE);
 	goto loser;
     }
-    
-    tmpbuf = &tmpbuf[keyidoff + 4*ncerts];
+
     for ( i = 0; i < ncerts; i++ ) {
-	entry->certKeys[i].data =
-	    (unsigned char *)PORT_ArenaAlloc(arena, entry->certKeys[i].len);
+	unsigned int kLen = entry->certKeys[i].len;
+	entry->certKeys[i].data = (unsigned char *)PORT_ArenaAlloc(arena, kLen);
 	if ( entry->certKeys[i].data == NULL ) {
 	    PORT_SetError(SEC_ERROR_NO_MEMORY);
 	    goto loser;
 	}
-	PORT_Memcpy(entry->certKeys[i].data, tmpbuf, entry->certKeys[i].len);
-	tmpbuf = &tmpbuf[entry->certKeys[i].len];
-    }
-
+	PORT_Memcpy(entry->certKeys[i].data, tmpbuf, kLen);
+	tmpbuf += kLen;
+    }
     for ( i = 0; i < ncerts; i++ ) {
-	entry->keyIDs[i].data =
-	    (unsigned char *)PORT_ArenaAlloc(arena, entry->keyIDs[i].len);
+	unsigned int iLen = entry->keyIDs[i].len;
+	entry->keyIDs[i].data = (unsigned char *)PORT_ArenaAlloc(arena, iLen);
 	if ( entry->keyIDs[i].data == NULL ) {
 	    PORT_SetError(SEC_ERROR_NO_MEMORY);
 	    goto loser;
 	}
-	PORT_Memcpy(entry->keyIDs[i].data, tmpbuf, entry->keyIDs[i].len);
-	tmpbuf = &tmpbuf[entry->keyIDs[i].len];
-    }
-
-    end = &dbentry->data[dbentry->len];
-    if ((eaddrlen == 0) && (tmpbuf+1 < end)) {
+	PORT_Memcpy(entry->keyIDs[i].data, tmpbuf, iLen);
+	tmpbuf += iLen;
+    }
+
+    end = dbentry->data + dbentry->len;
+    if ((eaddrlen == 0) && (end - tmpbuf > 1)) {
 	/* read in the additional email addresses */
-	entry->nemailAddrs = tmpbuf[0] << 8 | tmpbuf[1];
+	entry->nemailAddrs = (((unsigned int)tmpbuf[0]) << 8) | tmpbuf[1];
 	tmpbuf += 2;
-	entry->emailAddrs = (char **)
-		PORT_ArenaAlloc(arena, entry->nemailAddrs * sizeof(char *));
+	if (end - tmpbuf < 2 * (int)entry->nemailAddrs)
+	    goto loser;
+	entry->emailAddrs = PORT_ArenaNewArray(arena, char *, entry->nemailAddrs);
 	if (entry->emailAddrs == NULL) {
 	    PORT_SetError(SEC_ERROR_NO_MEMORY);
 	    goto loser;
 	}
 	for (i=0; i < entry->nemailAddrs; i++) {
 	    int nameLen;
-	    if (tmpbuf + 2 > end) {
+	    if (end - tmpbuf < 2) {
 		goto loser;
 	    }
-
-	    nameLen = tmpbuf[0] << 8 | tmpbuf[1];
+	    nameLen = (((int)tmpbuf[0]) << 8) | tmpbuf[1];
+	    tmpbuf += 2;
+	    if (end - tmpbuf < nameLen) {
+		goto loser;
+	    }
 	    entry->emailAddrs[i] = PORT_ArenaAlloc(arena,nameLen);
 	    if (entry->emailAddrs == NULL) {
 	        PORT_SetError(SEC_ERROR_NO_MEMORY);
 	        goto loser;
 	    }
-	    if (tmpbuf + (nameLen+2) > end) {
-		goto loser;
-	    }
-	    PORT_Memcpy(entry->emailAddrs[i],&tmpbuf[2],nameLen);
-	    tmpbuf += 2 + nameLen;
+	    PORT_Memcpy(entry->emailAddrs[i], tmpbuf, nameLen);
+	    tmpbuf += nameLen;
 	}
-    }
-    
-    
+	if (tmpbuf != end) 
+	    goto loser;
+    }
+    PORT_ArenaUnmark(arena, mark);
     return(SECSuccess);
 
 loser:
+    PORT_ArenaRelease(arena, mark); /* discard above allocations */
     return(SECFailure);
 }
 
 /*
  * create a new subject entry with a single cert
  */
 static certDBEntrySubject *
 NewDBSubjectEntry(SECItem *derSubject, SECItem *certKey,
@@ -4272,17 +4292,17 @@ SECStatus
 nsslowcert_TraverseDBEntries(NSSLOWCERTCertDBHandle *handle,
 		      certDBEntryType type,
 		      SECStatus (* callback)(SECItem *data, SECItem *key,
 					    certDBEntryType type, void *pdata),
 		      void *udata )
 {
     DBT data;
     DBT key;
-    SECStatus rv;
+    SECStatus rv = SECSuccess;
     int ret;
     SECItem dataitem;
     SECItem keyitem;
     unsigned char *buf;
     unsigned char *keybuf;
     
     ret = certdb_Seq(handle->permCertDB, &key, &data, R_FIRST);
 
@@ -4299,23 +4319,26 @@ nsslowcert_TraverseDBEntries(NSSLOWCERTC
             dataitem.type = siBuffer;
 	    keyitem.len = key.size - SEC_DB_KEY_HEADER_LEN;
 	    keybuf = (unsigned char *)key.data;
 	    keyitem.data = &keybuf[SEC_DB_KEY_HEADER_LEN];
             keyitem.type = siBuffer;
 	    /* type should equal keybuf[0].  */
 
 	    rv = (* callback)(&dataitem, &keyitem, type, udata);
-	    if ( rv != SECSuccess ) {
-		return(rv);
+	    if ( rv == SECSuccess ) {
+		++ret;
 	    }
 	}
     } while ( certdb_Seq(handle->permCertDB, &key, &data, R_NEXT) == 0 );
 
-    return(SECSuccess);
+    /* If any callbacks succeeded, or no calls to callbacks were made, 
+     * then report success.  Otherwise, report failure.
+     */
+    return (ret ? SECSuccess : rv);
 }
 /*
  * Decode a certificate and enter it into the temporary certificate database.
  * Deal with nicknames correctly
  *
  * This is the private entry point.
  */
 static NSSLOWCERTCertificate *
--- a/security/nss/lib/softoken/pcertt.h
+++ b/security/nss/lib/softoken/pcertt.h
@@ -161,16 +161,18 @@ struct NSSLOWCERTCertificateStr {
 
 #define SEC_CERTIFICATE_VERSION_1		0	/* default created */
 #define SEC_CERTIFICATE_VERSION_2		1	/* v2 */
 #define SEC_CERTIFICATE_VERSION_3		2	/* v3 extensions */
 
 #define SEC_CRL_VERSION_1		0	/* default */
 #define SEC_CRL_VERSION_2		1	/* v2 extensions */
 
+#define NSS_MAX_LEGACY_DB_KEY_SIZE (60 * 1024)
+
 struct NSSLOWCERTIssuerAndSNStr {
     SECItem derIssuer;
     SECItem serialNumber;
 };
 
 typedef SECStatus (* NSSLOWCERTCertCallback)(NSSLOWCERTCertificate *cert, void *arg);
 
 /* This is the typedef for the callback passed to nsslowcert_OpenCertDB() */