Skip to content

Commit

Permalink
winpgnt: fix GUI removal of encrypted keys.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sgtatham committed Apr 2, 2021
1 parent b0f9e3a commit fbab166
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 33 deletions.
26 changes: 8 additions & 18 deletions pageant.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions pageant.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
15 changes: 2 additions & 13 deletions windows/winpgnt.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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--;
}
}
Expand Down

0 comments on commit fbab166

Please sign in to comment.