From 65270b56f050975ea255a6556cfde2dd38546308 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Thu, 16 Sep 2021 09:41:03 +0100 Subject: [PATCH] free_prompts: deal with a reference from an Ldisc. In a GUI app, when interactive userpass input begins, the Ldisc acquires a reference to a prompts_t. If something bad happens to the SSH connection (e.g. unexpected server-side closure), then all the SSH layers will be destroyed, including freeing that prompts_t. So the Ldisc will have a stale reference to it, which it might potentially use. To fix that, I've arranged a back-pointer so that prompts_t itself can find the Ldisc's reference to it, and NULL it out on free. So now, whichever of a prompts_t and an Ldisc is freed first, the link between them should be cleanly broken. (I'm not 100% sure this is absolutely necessary, in the sense of whether a sequence of events can _actually_ happen that causes a stale pointer dereference. But I don't want to take the chance!) --- ldisc.c | 4 ++++ putty.h | 12 ++++++++++-- utils/prompts.c | 7 +++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/ldisc.c b/ldisc.c index b549c14a..f0e59658 100644 --- a/ldisc.c +++ b/ldisc.c @@ -182,6 +182,8 @@ void ldisc_free(Ldisc *ldisc) backend_provide_ldisc(ldisc->backend, NULL); if (ldisc->buf) sfree(ldisc->buf); + if (ldisc->prompts && ldisc->prompts->ldisc_ptr_to_us == &ldisc->prompts) + ldisc->prompts->ldisc_ptr_to_us = NULL; delete_callbacks_for_context(ldisc); sfree(ldisc); } @@ -201,6 +203,8 @@ void ldisc_enable_prompt_callback(Ldisc *ldisc, prompts_t *prompts) * that it can continue the interactive prompting process. */ ldisc->prompts = prompts; + if (prompts) + ldisc->prompts->ldisc_ptr_to_us = &ldisc->prompts; } static void ldisc_input_queue_callback(void *ctx) diff --git a/putty.h b/putty.h index 417579e4..907f3ff0 100644 --- a/putty.h +++ b/putty.h @@ -767,7 +767,8 @@ typedef struct { bool echo; strbuf *result; } prompt_t; -typedef struct { +typedef struct prompts_t prompts_t; +struct prompts_t { /* * Indicates whether the information entered is to be used locally * (for instance a key passphrase prompt), or is destined for the wire. @@ -806,7 +807,14 @@ typedef struct { */ toplevel_callback_fn_t callback; void *callback_ctx; -} prompts_t; + + /* + * When this prompts_t is known to an Ldisc, we might need to + * break the connection if things get freed in an emergency. So + * this is a pointer to the Ldisc's pointer to us. + */ + prompts_t **ldisc_ptr_to_us; +}; prompts_t *new_prompts(void); void add_prompt(prompts_t *p, char *promptstr, bool echo); void prompt_set_result(prompt_t *pr, const char *newstr); diff --git a/utils/prompts.c b/utils/prompts.c index 583316f6..e26ef905 100644 --- a/utils/prompts.c +++ b/utils/prompts.c @@ -17,6 +17,7 @@ prompts_t *new_prompts(void) p->name_reqd = p->instr_reqd = false; p->callback = NULL; p->callback_ctx = NULL; + p->ldisc_ptr_to_us = NULL; return p; } @@ -49,6 +50,12 @@ char *prompt_get_result(prompt_t *pr) void free_prompts(prompts_t *p) { size_t i; + + /* If an Ldisc currently knows about us, tell it to forget us, so + * it won't dereference a stale pointer later. */ + if (p->ldisc_ptr_to_us) + *p->ldisc_ptr_to_us = NULL; + for (i=0; i < p->n_prompts; i++) { prompt_t *pr = p->prompts[i]; strbuf_free(pr->result);