[Bug 334274] double free in CRMF_EncryptedKeyGetEncryptedValue. r=nelson

2 files changed, 49 insertions, 19 deletions

diff --git a/security/nss/lib/crmf/crmfcont.c b/security/nss/lib/crmf/crmfcont.c
index b609e84ea..f2cab57b3 100644
--- a/security/nss/lib/crmf/crmfcont.c
+++ b/security/nss/lib/crmf/crmfcont.c
@@ -148,21 +148,27 @@ crmf_destroy_encrypted_value(CRMFEncryptedValue *inEncrValue, PRBool freeit)
     if (inEncrValue != NULL) {
         if (inEncrValue->intendedAlg) {
 	    SECOID_DestroyAlgorithmID(inEncrValue->intendedAlg, PR_TRUE);
+	    inEncrValue->intendedAlg = NULL;
 	}
 	if (inEncrValue->symmAlg) {
 	    SECOID_DestroyAlgorithmID(inEncrValue->symmAlg, PR_TRUE);
+	    inEncrValue->symmAlg = NULL;
 	}
         if (inEncrValue->encSymmKey.data) {
 	    PORT_Free(inEncrValue->encSymmKey.data);
+	    inEncrValue->encSymmKey.data = NULL;
 	}
 	if (inEncrValue->keyAlg) {
 	    SECOID_DestroyAlgorithmID(inEncrValue->keyAlg, PR_TRUE);
+	    inEncrValue->keyAlg = NULL;
 	}
 	if (inEncrValue->valueHint.data) {
 	    PORT_Free(inEncrValue->valueHint.data);
+	    inEncrValue->valueHint.data = NULL;
 	}
         if (inEncrValue->encValue.data) {
 	    PORT_Free(inEncrValue->encValue.data);
+	    inEncrValue->encValue.data = NULL;
 	}
 	if (freeit) {
 	    PORT_Free(inEncrValue);
@@ -183,15 +189,24 @@ crmf_copy_encryptedvalue_secalg(PRArenaPool     *poolp,
 				SECAlgorithmID **destAlgId)
 {
     SECAlgorithmID *newAlgId;
+    SECStatus rv;
 
-    *destAlgId = newAlgId = (poolp != NULL) ?
-                            PORT_ArenaZNew(poolp, SECAlgorithmID) :
-                            PORT_ZNew(SECAlgorithmID);
+    newAlgId = (poolp != NULL) ? PORT_ArenaZNew(poolp, SECAlgorithmID) :
+                                 PORT_ZNew(SECAlgorithmID);
     if (newAlgId == NULL) {
         return SECFailure;
     }
     
-    return SECOID_CopyAlgorithmID(poolp, newAlgId, srcAlgId);
+    rv = SECOID_CopyAlgorithmID(poolp, newAlgId, srcAlgId);
+    if (rv != SECSuccess) {
+        if (!poolp) {
+            SECOID_DestroyAlgorithmID(newAlgId, PR_TRUE);
+        }
+        return rv;
+    }
+    *destAlgId = newAlgId;
+    
+    return rv;
 }
 
 SECStatus
@@ -252,7 +267,7 @@ crmf_copy_encryptedvalue(PRArenaPool        *poolp,
     return SECSuccess;
  loser:
     if (poolp == NULL && destValue != NULL) {
-        crmf_destroy_encrypted_value(destValue, PR_TRUE);
+        crmf_destroy_encrypted_value(destValue, PR_FALSE);
     }
     return SECFailure;
 }
diff --git a/security/nss/lib/crmf/respcmn.c b/security/nss/lib/crmf/respcmn.c
index 153ecee51..54fbb3faf 100644
--- a/security/nss/lib/crmf/respcmn.c
+++ b/security/nss/lib/crmf/respcmn.c
@@ -46,12 +46,15 @@ cmmf_DestroyPKIStatusInfo (CMMFPKIStatusInfo *info, PRBool freeit)
 {
     if (info->status.data != NULL) {
         PORT_Free(info->status.data);
+        info->status.data = NULL;
     }
     if (info->statusString.data != NULL) {
         PORT_Free(info->statusString.data);
+        info->statusString.data = NULL;
     }
     if (info->failInfo.data != NULL) {
         PORT_Free(info->failInfo.data);
+        info->failInfo.data = NULL;
     }
     if (freeit) {
         PORT_Free(info);
@@ -232,6 +235,7 @@ cmmf_DestroyCertOrEncCert(CMMFCertOrEncCert *certOrEncCert, PRBool freeit)
     case cmmfEncryptedCert:
         crmf_destroy_encrypted_value(certOrEncCert->cert.encryptedCert,
 				     PR_TRUE);
+        certOrEncCert->cert.encryptedCert = NULL;
 	break;
     default:
         break;
@@ -292,17 +296,22 @@ cmmf_CopyCertResponse(PRArenaPool      *poolp,
         return rv;
     }
     if (src->certifiedKeyPair != NULL) {
-        dest->certifiedKeyPair = (poolp == NULL) ?
-                                  PORT_ZNew(CMMFCertifiedKeyPair) :
-	                          PORT_ArenaZNew(poolp, CMMFCertifiedKeyPair);
-	if (dest->certifiedKeyPair == NULL) {
+	CMMFCertifiedKeyPair *destKeyPair;
+
+	destKeyPair = (poolp == NULL) ? PORT_ZNew(CMMFCertifiedKeyPair) :
+	                        PORT_ArenaZNew(poolp, CMMFCertifiedKeyPair);
+	if (!destKeyPair) {
 	    return SECFailure;
 	}
-        rv = cmmf_CopyCertifiedKeyPair(poolp, dest->certifiedKeyPair,
+	rv = cmmf_CopyCertifiedKeyPair(poolp, destKeyPair,
 				       src->certifiedKeyPair);
 	if (rv != SECSuccess) {
+	    if (!poolp) {
+	        CMMF_DestroyCertifiedKeyPair(destKeyPair);
+	    }
 	    return rv;
 	}
+	dest->certifiedKeyPair = destKeyPair;
     }
     return SECSuccess;
 }
@@ -321,16 +330,19 @@ cmmf_CopyCertOrEncCert(PRArenaPool *poolp, CMMFCertOrEncCert *dest,
         dest->cert.certificate = CERT_DupCertificate(src->cert.certificate);
 	break;
     case cmmfEncryptedCert:
-        dest->cert.encryptedCert = encVal = (poolp == NULL) ?
-	                             PORT_ZNew(CRMFEncryptedValue) :
-				     PORT_ArenaZNew(poolp, CRMFEncryptedValue);
+ 	encVal = (poolp == NULL) ? PORT_ZNew(CRMFEncryptedValue) :
+	                           PORT_ArenaZNew(poolp, CRMFEncryptedValue);
 	if (encVal == NULL) {
 	    return SECFailure;
 	}
         rv = crmf_copy_encryptedvalue(poolp, src->cert.encryptedCert, encVal);
 	if (rv != SECSuccess) {
+	    if (!poolp) {
+	        crmf_destroy_encrypted_value(encVal, PR_TRUE);
+	    }
 	    return rv;
 	}
+	dest->cert.encryptedCert = encVal;        
 	break;
     default:
         rv = SECFailure;
@@ -351,19 +363,22 @@ cmmf_CopyCertifiedKeyPair(PRArenaPool *poolp, CMMFCertifiedKeyPair *dest,
     }
 
     if (src->privateKey != NULL) {
-        CRMFEncryptedValue *encVal;
+	CRMFEncryptedValue *encVal;
 
-	encVal = dest->privateKey = (poolp == NULL) ?
-	                             PORT_ZNew(CRMFEncryptedValue) :
-                                     PORT_ArenaZNew(poolp, CRMFEncryptedValue);
+	encVal = (poolp == NULL) ? PORT_ZNew(CRMFEncryptedValue) :
+	                           PORT_ArenaZNew(poolp, CRMFEncryptedValue);
 	if (encVal == NULL) {
 	    return SECFailure;
 	}
-        rv = crmf_copy_encryptedvalue(poolp, src->privateKey, 
-				      dest->privateKey);
+	rv = crmf_copy_encryptedvalue(poolp, src->privateKey, 
+				      encVal);
 	if (rv != SECSuccess) {
+	    if (!poolp) {
+	        crmf_destroy_encrypted_value(encVal, PR_TRUE);
+	    }
 	    return rv;
 	}
+	dest->privateKey = encVal;
     }
     rv = cmmf_copy_secitem(poolp, &dest->derPublicationInfo, 
 			   &src->derPublicationInfo);