From a80edab4b51c5092a619bd6ac84f66d696003efa Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Fri, 14 Dec 2018 19:42:47 +0000
Subject: [PATCH] Move some manual freeing into freersakey().

Several pieces of old code were disposing of pieces of an RSAKey by
manually freeing them one at a time. We have a centralised
freersakey(), so we should use that instead wherever possible.

Where it wasn't possible to switch over to that, it was because we
were only freeing the private fields of the key - so I've fixed that
by cutting freersakey() down the middle and exposing the private-only
half as freersapriv().
---
 pageant.c   | 10 ++++------
 ssh.h       |  1 +
 ssh1login.c | 20 +++-----------------
 sshrsa.c    | 41 ++++++++++++++++++++++++++++++-----------
 4 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/pageant.c b/pageant.c
index 40ec8b48..3da719e9 100644
--- a/pageant.c
+++ b/pageant.c
@@ -308,8 +308,7 @@ void pageant_handle_msg(BinarySink *bs,
             if (response)
                 freebn(response);
             freebn(challenge);
-            freebn(reqkey.exponent);
-            freebn(reqkey.modulus);
+            freersakey(&reqkey);
 	}
 	break;
       case SSH2_AGENTC_SIGN_REQUEST:
@@ -526,13 +525,13 @@ void pageant_handle_msg(BinarySink *bs,
 
             plog(logctx, logfn, "request: SSH1_AGENTC_REMOVE_RSA_IDENTITY");
 
+            memset(&reqkey, 0, sizeof(reqkey));
             get_rsa_ssh1_pub(msg, &reqkey, RSA_SSH1_EXPONENT_FIRST);
 
             if (get_err(msg)) {
                 pageant_failure_msg(bs, "unable to decode request",
                                     logctx, logfn);
-                freebn(reqkey.exponent);
-                freebn(reqkey.modulus);
+                freersakey(&reqkey);
                 return;
             }
 
@@ -545,8 +544,7 @@ void pageant_handle_msg(BinarySink *bs,
             }
 
 	    key = find234(rsakeys, &reqkey, NULL);
-	    freebn(reqkey.exponent);
-	    freebn(reqkey.modulus);
+            freersakey(&reqkey);
 	    if (key) {
                 plog(logctx, logfn, "found with comment: %s", key->comment);
 
diff --git a/ssh.h b/ssh.h
index 691ed967..88987fbc 100644
--- a/ssh.h
+++ b/ssh.h
@@ -514,6 +514,7 @@ bool rsa_verify(struct RSAKey *key);
 void rsa_ssh1_public_blob(BinarySink *bs, struct RSAKey *key,
                           RsaSsh1Order order);
 int rsa_ssh1_public_blob_len(void *data, int maxlen);
+void freersapriv(struct RSAKey *key);
 void freersakey(struct RSAKey *key);
 
 unsigned long crc32_compute(const void *s, size_t len);
diff --git a/ssh1login.c b/ssh1login.c
index bb77d569..6e9aafc6 100644
--- a/ssh1login.c
+++ b/ssh1login.c
@@ -368,22 +368,8 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
         ssh1_bpp_new_cipher(s->ppl.bpp, cipher, s->session_key);
     }
 
-    if (s->servkey.modulus) {
-        sfree(s->servkey.modulus);
-        s->servkey.modulus = NULL;
-    }
-    if (s->servkey.exponent) {
-        sfree(s->servkey.exponent);
-        s->servkey.exponent = NULL;
-    }
-    if (s->hostkey.modulus) {
-        sfree(s->hostkey.modulus);
-        s->hostkey.modulus = NULL;
-    }
-    if (s->hostkey.exponent) {
-        sfree(s->hostkey.exponent);
-        s->hostkey.exponent = NULL;
-    }
+    freersakey(&s->servkey);
+    freersakey(&s->hostkey);
     crMaybeWaitUntilV((pktin = ssh1_login_pop(s)) != NULL);
 
     if (pktin->type != SSH1_SMSG_SUCCESS) {
@@ -745,7 +731,7 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
                         return;
                     }
                     response = rsa_ssh1_decrypt(challenge, &s->key);
-                    freebn(s->key.private_exponent);/* burn the evidence */
+                    freersapriv(&s->key);   /* burn the evidence */
 
                     for (i = 0; i < 32; i++) {
                         buffer[i] = bignum_byte(response, 31 - i);
diff --git a/sshrsa.c b/sshrsa.c
index 09627408..bf2f7bef 100644
--- a/sshrsa.c
+++ b/sshrsa.c
@@ -486,22 +486,41 @@ int rsa_ssh1_public_blob_len(void *data, int maxlen)
     return src->pos;
 }
 
+void freersapriv(struct RSAKey *key)
+{
+    if (key->private_exponent) {
+	freebn(key->private_exponent);
+        key->private_exponent = NULL;
+    }
+    if (key->p) {
+	freebn(key->p);
+        key->p = NULL;
+    }
+    if (key->q) {
+	freebn(key->q);
+        key->q = NULL;
+    }
+    if (key->iqmp) {
+	freebn(key->iqmp);
+        key->iqmp = NULL;
+    }
+}
+
 void freersakey(struct RSAKey *key)
 {
-    if (key->modulus)
+    freersapriv(key);
+    if (key->modulus) {
 	freebn(key->modulus);
-    if (key->exponent)
+        key->modulus = NULL;
+    }
+    if (key->exponent) {
 	freebn(key->exponent);
-    if (key->private_exponent)
-	freebn(key->private_exponent);
-    if (key->p)
-	freebn(key->p);
-    if (key->q)
-	freebn(key->q);
-    if (key->iqmp)
-	freebn(key->iqmp);
-    if (key->comment)
+        key->exponent = NULL;
+    }
+    if (key->comment) {
 	sfree(key->comment);
+        key->comment = NULL;
+    }
 }
 
 /* ----------------------------------------------------------------------