nhyatt/putty-source
1
0
Fork 0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00
Code Issues Projects Releases Wiki Activity

winpgnt: fix crash if deferred-decryption passphrase is wrong.

Browse Source 
Thanks to Jacob for spotting this one: when we hand a passphrase back
to pageant.c via pageant_passphrase_request_success(), if the key
doesn't decrypt successfully, pageant.c responds by immediately
issuing another passphrase prompt - and it does it _synchronously_, by
calling back from within pageant_passphrase_request_success(). In this
case, the effect is that we end up in ask_passphrase_common(), which
starts by asserting that nonmodal_passphrase_hwnd is NULL - but it
wasn't NULL _quite_ yet, because end_passphrase_dialog() was expecting
to clean it up immediately after pageant_passphrase_request_success()
returned, i.e. just too late.

The heavyweight fix would be to arrange a toplevel callback to defer
opening the new window until after the old one had been cleaned up.
But in this case I don't think there's any need: it's enough to simply
do the operations in end_passphrase_dialog() in the opposite order, so
that first we destroy the old window and set nonmodal_passphrase_hwnd
back to NULL, and _then_ we call into pageant.c which might call us
back and open a fresh window.
This commit is contained in:
Simon Tatham 2021-04-07 20:15:34 +01:00
parent 1069ba6a01
commit 21c2e451da
1 changed files with 15 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
windows/winpgnt.c
View File

@ -188,6 +188,21 @@ static void end_passphrase_dialog(HWND hwnd, INT_PTR result)
if (p->modal) {
EndDialog(hwnd, result);
} else {
/*
* Destroy this passphrase dialog box before passing the
* results back to pageant.c, to avoid re-entrancy issues.
*
* If we successfully got a passphrase from the user, but it
* was _wrong_, then pageant_passphrase_request_success will
* respond by calling back - synchronously - to our
* ask_passphrase() implementation, which will expect the
* previous value of nonmodal_passphrase_hwnd to have already
* been cleaned up.
*/
SetWindowLongPtr(hwnd, GWLP_USERDATA, (LONG_PTR) NULL);
DestroyWindow(hwnd);
nonmodal_passphrase_hwnd = NULL;
if (result)
pageant_passphrase_request_success(
p->dlgid, ptrlen_from_asciz(p->passphrase));
@ -196,9 +211,6 @@ static void end_passphrase_dialog(HWND hwnd, INT_PTR result)
burnstr(p->passphrase);
sfree(p);
SetWindowLongPtr(hwnd, GWLP_USERDATA, (LONG_PTR) NULL);
DestroyWindow(hwnd);
nonmodal_passphrase_hwnd = NULL;
}
}