[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] bind: fix possible double-frees in ssh_bind_free


Hi,

Attached is a patch which fixes possible double-frees via 'ssh_bind_free'.


-Jon
From 916d26b5701cc454414f930b5817421025da90f6 Mon Sep 17 00:00:00 2001
From: Jon Simons <jon@xxxxxxxxxxxxx>
Date: Wed, 15 Jan 2014 13:50:15 -0800
Subject: [PATCH] bind: fix possible double-frees in ssh_bind_free

Make sure to explicitly set key pointers to NULL following the use
of 'ssh_key_free' throughout bind.c.

Before this change, a double free can happen via 'ssh_bind_free'
as in this example callpath:

  // create an ssh_bind
  ssh_bind b = ssh_bind_new();

  // provide a path to a wrong key-type
  ssh_bind_options_set(b, SSH_BIND_OPTIONS_DSAKEY, path_to_rsa_key);

  // initialize set key-type
  ssh_bind_listen(b);

    -> error path "The DSA host key has the wrong type: %d",

       ssh_key_free(sshbind->dsa)

         -> ssh_key_clean(key) // OK

         -> SAFE_FREE(key)     // OK, but, sshbind->dsa is *not* set to NULL

  // ssh_bind_listen failed, so clean up ssh_bind
  ssh_bind_free(b);

    -> ssh_key_free(sshbind->dsa)  // double-free here

To fix, set pointers to NULL that have been free'd with 'ssh_key_free'.
---
 src/bind.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/bind.c b/src/bind.c
index c8306da..698b953 100644
--- a/src/bind.c
+++ b/src/bind.c
@@ -179,6 +179,7 @@ int ssh_bind_listen(ssh_bind sshbind) {
           ssh_set_error(sshbind, SSH_FATAL,
                   "The ECDSA host key has the wrong type");
           ssh_key_free(sshbind->ecdsa);
+          sshbind->ecdsa = NULL;
           return SSH_ERROR;
       }
   }
@@ -201,6 +202,7 @@ int ssh_bind_listen(ssh_bind sshbind) {
                   "The DSA host key has the wrong type: %d",
                   ssh_key_type(sshbind->dsa));
           ssh_key_free(sshbind->dsa);
+          sshbind->dsa = NULL;
           return SSH_ERROR;
       }
   }
@@ -222,6 +224,7 @@ int ssh_bind_listen(ssh_bind sshbind) {
           ssh_set_error(sshbind, SSH_FATAL,
                   "The RSA host key has the wrong type");
           ssh_key_free(sshbind->rsa);
+          sshbind->rsa = NULL;
           return SSH_ERROR;
       }
   }
@@ -235,7 +238,9 @@ int ssh_bind_listen(ssh_bind sshbind) {
       fd = bind_socket(sshbind, host, sshbind->bindport);
       if (fd == SSH_INVALID_SOCKET) {
           ssh_key_free(sshbind->dsa);
+          sshbind->dsa = NULL;
           ssh_key_free(sshbind->rsa);
+          sshbind->rsa = NULL;
           return -1;
       }
       sshbind->bindfd = fd;
@@ -246,7 +251,9 @@ int ssh_bind_listen(ssh_bind sshbind) {
                   fd, strerror(errno));
           close(fd);
           ssh_key_free(sshbind->dsa);
+          sshbind->dsa = NULL;
           ssh_key_free(sshbind->rsa);
+          sshbind->rsa = NULL;
           return -1;
       }
   } else {
@@ -348,8 +355,11 @@ void ssh_bind_free(ssh_bind sshbind){
   SAFE_FREE(sshbind->ecdsakey);
 
   ssh_key_free(sshbind->dsa);
+  sshbind->dsa = NULL;
   ssh_key_free(sshbind->rsa);
+  sshbind->rsa = NULL;
   ssh_key_free(sshbind->ecdsa);
+  sshbind->ecdsa = NULL;
 
   for (i = 0; i < 10; i++) {
     if (sshbind->wanted_methods[i]) {
-- 
1.8.4.21.g992c386


Archive administrator: postmaster@lists.cynapses.org