From 8a4364a99bb4c367c13dd54328c30d61ec6b3624 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@cryptomilk.org>
Date: Wed, 19 Jun 2013 11:17:48 +0200
Subject: [PATCH] ecdh: Avoid memory leaks in ssh_server_ecdh_init().

---
 src/ecdh.c | 105 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 69 insertions(+), 36 deletions(-)

diff --git a/src/ecdh.c b/src/ecdh.c
index b6b1ba06..89c3213a 100644
--- a/src/ecdh.c
+++ b/src/ecdh.c
@@ -199,9 +199,9 @@ error:
 
 int ssh_server_ecdh_init(ssh_session session, ssh_buffer packet){
     /* ECDH keys */
-    ssh_string q_c_string = NULL;
-    ssh_string q_s_string = NULL;
-    EC_KEY *ecdh_key=NULL;
+    ssh_string q_c_string;
+    ssh_string q_s_string;
+    EC_KEY *ecdh_key;
     const EC_GROUP *group;
     const EC_POINT *ecdh_pubkey;
     bignum_CTX ctx;
@@ -211,14 +211,11 @@ int ssh_server_ecdh_init(ssh_session session, ssh_buffer packet){
     int len;
     int rc;
 
-    enter_function();
-
     /* Extract the client pubkey from the init packet */
-
     q_c_string = buffer_get_ssh_string(packet);
     if (q_c_string == NULL) {
-      ssh_set_error(session,SSH_FATAL, "No Q_C ECC point in packet");
-      goto error;
+        ssh_set_error(session,SSH_FATAL, "No Q_C ECC point in packet");
+        return SSH_ERROR;
     }
     session->next_crypto->ecdh_client_pubkey = q_c_string;
 
@@ -226,36 +223,64 @@ int ssh_server_ecdh_init(ssh_session session, ssh_buffer packet){
 
     ctx = BN_CTX_new();
     ecdh_key = EC_KEY_new_by_curve_name(NISTP256);
+    if (ecdh_key == NULL) {
+        ssh_set_error_oom(session);
+        BN_CTX_free(ctx);
+        return SSH_ERROR;
+    }
+
     group = EC_KEY_get0_group(ecdh_key);
     EC_KEY_generate_key(ecdh_key);
-    ecdh_pubkey=EC_KEY_get0_public_key(ecdh_key);
-    len = EC_POINT_point2oct(group,ecdh_pubkey,POINT_CONVERSION_UNCOMPRESSED,
-        NULL,0,ctx);
-    q_s_string=ssh_string_new(len);
 
-    EC_POINT_point2oct(group,ecdh_pubkey,POINT_CONVERSION_UNCOMPRESSED,
-        ssh_string_data(q_s_string),len,ctx);
+    ecdh_pubkey = EC_KEY_get0_public_key(ecdh_key);
+    len = EC_POINT_point2oct(group,
+                             ecdh_pubkey,
+                             POINT_CONVERSION_UNCOMPRESSED,
+                             NULL,
+                             0,
+                             ctx);
 
+    q_s_string = ssh_string_new(len);
+    if (q_s_string == NULL) {
+        EC_KEY_free(ecdh_key);
+        BN_CTX_free(ctx);
+        return SSH_ERROR;
+    }
+
+    EC_POINT_point2oct(group,
+                       ecdh_pubkey,
+                       POINT_CONVERSION_UNCOMPRESSED,
+                       ssh_string_data(q_s_string),
+                       len,
+                       ctx);
     BN_CTX_free(ctx);
+
     session->next_crypto->ecdh_privkey = ecdh_key;
     session->next_crypto->ecdh_server_pubkey = q_s_string;
 
     rc = buffer_add_u8(session->out_buffer, SSH2_MSG_KEXDH_REPLY);
     if (rc < 0) {
-      ssh_set_error_oom(session);
-      goto error;
+        ssh_set_error_oom(session);
+        return SSH_ERROR;
     }
 
     /* build k and session_id */
-    if (ecdh_build_k(session) < 0) {
-      ssh_set_error(session, SSH_FATAL, "Cannot build k number");
-      goto error;
+    rc = ecdh_build_k(session);
+    if (rc < 0) {
+        ssh_set_error(session, SSH_FATAL, "Cannot build k number");
+        return SSH_ERROR;
     }
-    if (ssh_get_key_params(session, &privkey) == SSH_ERROR)
-        goto error;
-    if (make_sessionid(session) != SSH_OK) {
-      ssh_set_error(session, SSH_FATAL, "Could not create a session id");
-      goto error;
+
+    /* privkey is not allocated */
+    rc = ssh_get_key_params(session, &privkey);
+    if (rc == SSH_ERROR) {
+        return SSH_ERROR;
+    }
+
+    rc = make_sessionid(session);
+    if (rc != SSH_OK) {
+        ssh_set_error(session, SSH_FATAL, "Could not create a session id");
+        return SSH_ERROR;
     }
 
     /* add host's public key */
@@ -263,23 +288,29 @@ int ssh_server_ecdh_init(ssh_session session, ssh_buffer packet){
                                session->next_crypto->server_pubkey);
     if (rc < 0) {
         ssh_set_error_oom(session);
-        goto error;
+        return SSH_ERROR;
     }
 
     /* add ecdh public key */
     rc = buffer_add_ssh_string(session->out_buffer, q_s_string);
     if (rc < 0) {
         ssh_set_error_oom(session);
-        goto error;
+        return SSH_ERROR;
     }
     /* add signature blob */
     sig_blob = ssh_srv_pki_do_sign_sessionid(session, privkey);
     if (sig_blob == NULL) {
         ssh_set_error(session, SSH_FATAL, "Could not sign the session id");
-        goto error;
+        return SSH_ERROR;
     }
-    buffer_add_ssh_string(session->out_buffer, sig_blob);
+
+    rc = buffer_add_ssh_string(session->out_buffer, sig_blob);
     ssh_string_free(sig_blob);
+    if (rc < 0) {
+        ssh_set_error_oom(session);
+        return SSH_ERROR;
+    }
+
     /* Free private keys as they should not be readable after this point */
     if (session->srv.rsa_key) {
         ssh_key_free(session->srv.rsa_key);
@@ -292,19 +323,21 @@ int ssh_server_ecdh_init(ssh_session session, ssh_buffer packet){
 
     ssh_log(session,SSH_LOG_PROTOCOL, "SSH_MSG_KEXDH_REPLY sent");
     rc = packet_send(session);
-    if (rc == SSH_ERROR)
-        goto error;
+    if (rc == SSH_ERROR) {
+        return SSH_ERROR;
+    }
 
     /* Send the MSG_NEWKEYS */
-    if (buffer_add_u8(session->out_buffer, SSH2_MSG_NEWKEYS) < 0) {
-      goto error;
+    rc = buffer_add_u8(session->out_buffer, SSH2_MSG_NEWKEYS);
+    if (rc < 0) {
+        return SSH_ERROR;;
     }
-    session->dh_handshake_state=DH_STATE_NEWKEYS_SENT;
-    rc=packet_send(session);
+
+    session->dh_handshake_state = DH_STATE_NEWKEYS_SENT;
+    rc = packet_send(session);
     ssh_log(session, SSH_LOG_PROTOCOL, "SSH_MSG_NEWKEYS sent");
+
     return rc;
-  error:
-    return SSH_ERROR;
 }
 
 #endif /* WITH_SERVER */