From fbab1667284a99beb9640d0c6ba2ef7cd4fab003 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Fri, 2 Apr 2021 11:30:18 +0100
Subject: [PATCH] winpgnt: fix GUI removal of encrypted keys.

The GUI loop that responded to the 'Remove Key' button in the key list
worked by actually trying to retrieve a pointer to the ssh_key for a
stored key, and then passing that back to the delete function. But
when a key is encrypted, that pointer is NULL, so we segfaulted.

Fixed by changing pageant_delete_ssh2_key() to take a numeric index in
the list instead of a key pointer.
---
 pageant.c         | 26 ++++++++------------------
 pageant.h         |  4 ++--
 windows/winpgnt.c | 15 ++-------------
 3 files changed, 12 insertions(+), 33 deletions(-)

diff --git a/pageant.c b/pageant.c
index f6fe74af..a9684393 100644
--- a/pageant.c
+++ b/pageant.c
@@ -1388,31 +1388,21 @@ ssh2_userkey *pageant_nth_ssh2_key(int i)
         return NULL;
 }
 
-bool pageant_delete_ssh1_key(RSAKey *rkey)
+bool pageant_delete_nth_ssh1_key(int i)
 {
-    strbuf *blob = makeblob1(rkey);
-    PageantKeySort sort = keysort(1, ptrlen_from_strbuf(blob));
-    PageantKey *deleted = del234(keytree, &sort);
-    strbuf_free(blob);
-
-    if (!deleted)
+    PageantKey *pk = delpos234(keytree, find_first_key_for_version(1) + i);
+    if (!pk)
         return false;
-    assert(deleted->sort.ssh_version == 1);
-    assert(deleted->rkey == rkey);
+    pk_free(pk);
     return true;
 }
 
-bool pageant_delete_ssh2_key(ssh2_userkey *skey)
+bool pageant_delete_nth_ssh2_key(int i)
 {
-    strbuf *blob = makeblob2(skey);
-    PageantKeySort sort = keysort(2, ptrlen_from_strbuf(blob));
-    PageantKey *deleted = del234(keytree, &sort);
-    strbuf_free(blob);
-
-    if (!deleted)
+    PageantKey *pk = delpos234(keytree, find_first_key_for_version(2) + i);
+    if (!pk)
         return false;
-    assert(deleted->sort.ssh_version == 2);
-    assert(deleted->skey == skey);
+    pk_free(pk);
     return true;
 }
 
diff --git a/pageant.h b/pageant.h
index 86c42c1f..e1631445 100644
--- a/pageant.h
+++ b/pageant.h
@@ -125,8 +125,8 @@ int pageant_count_ssh1_keys(void);
 int pageant_count_ssh2_keys(void);
 bool pageant_add_ssh1_key(RSAKey *rkey);
 bool pageant_add_ssh2_key(ssh2_userkey *skey);
-bool pageant_delete_ssh1_key(RSAKey *rkey);
-bool pageant_delete_ssh2_key(ssh2_userkey *skey);
+bool pageant_delete_nth_ssh1_key(int i);
+bool pageant_delete_nth_ssh2_key(int i);
 
 /*
  * This callback must be provided by the Pageant front end code.
diff --git a/windows/winpgnt.c b/windows/winpgnt.c
index ded0073a..cb74a917 100644
--- a/windows/winpgnt.c
+++ b/windows/winpgnt.c
@@ -505,9 +505,6 @@ static void prompt_add_keyfile(bool encrypted)
 static INT_PTR CALLBACK KeyListProc(HWND hwnd, UINT msg,
                                 WPARAM wParam, LPARAM lParam)
 {
-    RSAKey *rkey;
-    ssh2_userkey *skey;
-
     static const struct {
         const char *name;
         FingerprintType value;
@@ -615,24 +612,16 @@ static INT_PTR CALLBACK KeyListProc(HWND hwnd, UINT msg,
                  * things hence altering the offset of subsequent items
                  */
                 for (i = sCount - 1; (itemNum >= 0) && (i >= 0); i--) {
-                    skey = pageant_nth_ssh2_key(i);
-
                     if (selectedArray[itemNum] == rCount + i) {
-                        pageant_delete_ssh2_key(skey);
-                        ssh_key_free(skey->key);
-                        sfree(skey);
+                        pageant_delete_nth_ssh2_key(i);
                         itemNum--;
                     }
                 }
 
                 /* do the same for the rsa keys */
                 for (i = rCount - 1; (itemNum >= 0) && (i >= 0); i--) {
-                    rkey = pageant_nth_ssh1_key(i);
-
                     if(selectedArray[itemNum] == i) {
-                        pageant_delete_ssh1_key(rkey);
-                        freersakey(rkey);
-                        sfree(rkey);
+                        pageant_delete_nth_ssh1_key(i);
                         itemNum--;
                     }
                 }