Fix freeing of retkey in openssh_new_read.

Now it's always freed in the cleanup epilogue (unless we're returning
it), rather than ad-hoc earlier in the code. That should make it more
reliably freed on error paths.

diff --git a/import.c b/import.c
index 340785bdbd6000aead4857bd31dea4cad4aa5c02..a00406aeb83e71f7600953568e9cf3909b437d1f 100644 (file)
--- a/import.c
+++ b/import.c
@@ -1543,7 +1543,7 @@ struct ssh2_userkey *openssh_new_read(const Filename *filename,
                                       const char **errmsg_p)
 {
     struct openssh_new_key *key = load_openssh_new_key(filename, errmsg_p);
-    struct ssh2_userkey *retkey;
+    struct ssh2_userkey *retkey = NULL;
     int i;
     struct ssh2_userkey *retval = NULL;
     const char *errmsg;
@@ -1552,7 +1552,7 @@ struct ssh2_userkey *openssh_new_read(const Filename *filename,
     unsigned checkint0, checkint1;
     const void *priv, *string;
     int privlen, stringlen, key_index;
-    const struct ssh_signkey *alg;
+    const struct ssh_signkey *alg = NULL;
 
     blob = NULL;
 
@@ -1678,10 +1678,10 @@ struct ssh2_userkey *openssh_new_read(const Filename *filename,
                            (const unsigned char *)thiskey);
         if (key_index == key->key_wanted) {
             retkey = snew(struct ssh2_userkey);
+            retkey->comment = NULL;
             retkey->alg = alg;
             retkey->data = alg->openssh_createkey(alg, &thiskey, &thiskeylen);
             if (!retkey->data) {
-                sfree(retkey);
                 errmsg = "unable to create key data structure";
                 goto error;
             }
@@ -1718,12 +1718,21 @@ struct ssh2_userkey *openssh_new_read(const Filename *filename,
 
     errmsg = NULL;                     /* no error */
     retval = retkey;
+    retkey = NULL;                     /* prevent the free */
 
     error:
     if (blob) {
         smemclr(blob, blobsize);
         sfree(blob);
     }
+    if (retkey) {
+        sfree(retkey->comment);
+        if (retkey->data) {
+            assert(alg);
+            alg->freekey(retkey->data);
+        }
+        sfree(retkey);
+    }
     smemclr(key->keyblob, key->keyblob_size);
     sfree(key->keyblob);
     smemclr(key, sizeof(*key));