Bug 1209546 - Fix possible PLArena leaks in ssl3_HandleServerKeyExchange() and ssl3_HandleECDHServerKeyExchange() r=wtc,ekr
authorTim Taubert <ttaubert@mozilla.com>
Wed, 30 Sep 2015 10:55:01 -0700 (2015-09-30)
changeset 11621 a245a4ccd354bb223971b9a5065ad885a65f4022
parent 11620 3a2530b1882ef0d394e0b404d9f8638918aee086
child 11622 c31a1c7cbf42db9bc963f988ef9576a98164cb0c
push id780
push usermartin.thomson@gmail.com
push dateWed, 30 Sep 2015 17:55:55 +0000 (2015-09-30)
reviewerswtc, ekr
bugs1209546
Bug 1209546 - Fix possible PLArena leaks in ssl3_HandleServerKeyExchange() and ssl3_HandleECDHServerKeyExchange() r=wtc,ekr
lib/ssl/ssl3con.c
lib/ssl/ssl3ecc.c
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -6912,28 +6912,26 @@ ssl3_HandleServerKeyExchange(sslSocket *
 	 */
     	arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
 	if (arena == NULL) {
 	    goto no_memory;
 	}
 
     	peerKey = PORT_ArenaZNew(arena, SECKEYPublicKey);
     	if (peerKey == NULL) {
-            PORT_FreeArena(arena, PR_FALSE);
 	    goto no_memory;
 	}
 
 	peerKey->arena              = arena;
 	peerKey->keyType            = rsaKey;
 	peerKey->pkcs11Slot         = NULL;
 	peerKey->pkcs11ID           = CK_INVALID_HANDLE;
 	if (SECITEM_CopyItem(arena, &peerKey->u.rsa.modulus,        &modulus) ||
 	    SECITEM_CopyItem(arena, &peerKey->u.rsa.publicExponent, &exponent))
 	{
-            PORT_FreeArena(arena, PR_FALSE);
 	    goto no_memory;
         }
     	ss->sec.peerKey = peerKey;
     	ss->ssl3.hs.ws = wait_cert_request;
     	return SECSuccess;
     }
 
     case kt_dh: {
@@ -7023,31 +7021,30 @@ ssl3_HandleServerKeyExchange(sslSocket *
 	 * ignore calling SECKEY_DestroyPublicKey. Using the key may allocate
 	 * pkcs11 slots and ID's.
 	 */
     	arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
 	if (arena == NULL) {
 	    goto no_memory;
 	}
 
-    	ss->sec.peerKey = peerKey = PORT_ArenaZNew(arena, SECKEYPublicKey);
+    	peerKey = PORT_ArenaZNew(arena, SECKEYPublicKey);
     	if (peerKey == NULL) {
 	    goto no_memory;
 	}
 
 	peerKey->arena              = arena;
 	peerKey->keyType            = dhKey;
 	peerKey->pkcs11Slot         = NULL;
 	peerKey->pkcs11ID           = CK_INVALID_HANDLE;
 
 	if (SECITEM_CopyItem(arena, &peerKey->u.dh.prime,        &dh_p) ||
 	    SECITEM_CopyItem(arena, &peerKey->u.dh.base,         &dh_g) ||
 	    SECITEM_CopyItem(arena, &peerKey->u.dh.publicValue,  &dh_Ys))
 	{
-            PORT_FreeArena(arena, PR_FALSE);
 	    goto no_memory;
         }
     	ss->sec.peerKey = peerKey;
     	ss->ssl3.hs.ws = wait_cert_request;
     	return SECSuccess;
     }
 
 #ifndef NSS_DISABLE_ECC
@@ -7060,20 +7057,26 @@ ssl3_HandleServerKeyExchange(sslSocket *
     	desc    = handshake_failure;
 	errCode = SEC_ERROR_UNSUPPORTED_KEYALG;
 	break;		/* goto alert_loser; */
     }
 
 alert_loser:
     (void)SSL3_SendAlert(ss, alert_fatal, desc);
 loser:
+    if (arena) {
+        PORT_FreeArena(arena, PR_FALSE);
+    }
     PORT_SetError( errCode );
     return SECFailure;
 
 no_memory:	/* no-memory error has already been set. */
+    if (arena) {
+        PORT_FreeArena(arena, PR_FALSE);
+    }
     ssl_MapLowLevelError(SSL_ERROR_SERVER_KEY_EXCHANGE_FAILURE);
     return SECFailure;
 }
 
 /*
  * Returns the TLS signature algorithm for the client authentication key and
  * whether it is an RSA or DSA key that may be able to sign only SHA-1 hashes.
  */
--- a/lib/ssl/ssl3ecc.c
+++ b/lib/ssl/ssl3ecc.c
@@ -699,17 +699,17 @@ ssl3_HandleECDHServerKeyExchange(sslSock
         goto alert_loser;
     }
 
     arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
     if (arena == NULL) {
         goto no_memory;
     }
 
-    ss->sec.peerKey = peerKey = PORT_ArenaZNew(arena, SECKEYPublicKey);
+    peerKey = PORT_ArenaZNew(arena, SECKEYPublicKey);
     if (peerKey == NULL) {
         goto no_memory;
     }
 
     peerKey->arena                 = arena;
     peerKey->keyType               = ecKey;
 
     /* set up EC parameters in peerKey */
@@ -720,34 +720,39 @@ ssl3_HandleECDHServerKeyExchange(sslSock
          */
         errCode = SEC_ERROR_UNSUPPORTED_ELLIPTIC_CURVE;
         goto alert_loser;
     }
 
     /* copy publicValue in peerKey */
     if (SECITEM_CopyItem(arena, &peerKey->u.ec.publicValue,  &ec_point))
     {
-        PORT_FreeArena(arena, PR_FALSE);
         goto no_memory;
     }
     peerKey->pkcs11Slot         = NULL;
     peerKey->pkcs11ID           = CK_INVALID_HANDLE;
 
     ss->sec.peerKey = peerKey;
     ss->ssl3.hs.ws = wait_cert_request;
 
     return SECSuccess;
 
 alert_loser:
     (void)SSL3_SendAlert(ss, alert_fatal, desc);
 loser:
+    if (arena) {
+        PORT_FreeArena(arena, PR_FALSE);
+    }
     PORT_SetError( errCode );
     return SECFailure;
 
 no_memory:      /* no-memory error has already been set. */
+    if (arena) {
+        PORT_FreeArena(arena, PR_FALSE);
+    }
     ssl_MapLowLevelError(SSL_ERROR_SERVER_KEY_EXCHANGE_FAILURE);
     return SECFailure;
 }
 
 SECStatus
 ssl3_SendECDHServerKeyExchange(
     sslSocket *ss,
     const SSLSignatureAndHashAlg *sigAndHash)