From 27f0140e5c613ae7c92bd30401d03f8201e09553 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 19 Aug 2023 09:59:01 +0100 Subject: [PATCH] Fix use-after-free on error returns from share_receive. Spotted by Coverity. If PuTTY is functioning as a sharing upstream, and a new downstream mishandles the version string exchange in any way that provokes an error message from share_receive() (such as failing to start the greeting with the expected protocol-name string), we were calling share_disconnect() and then going to crFinish. But share_disconnect is capable of actually freeing the entire ssh_sharing_connstate which contains the coroutine state - in which case, crFinish's zeroing out of crLine is a use-after-free. The usual pattern elsewhere in this code is to exit a coroutine with an ordinary 'return' when you've destroyed its state structure. Switch to doing that here. --- ssh/sharing.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ssh/sharing.c b/ssh/sharing.c index 97337229..7d338a3b 100644 --- a/ssh/sharing.c +++ b/ssh/sharing.c @@ -1770,7 +1770,7 @@ static void share_receive(Plug *plug, int urgent, const char *data, size_t len) char *buf = dupprintf("Version string far too long\n"); share_disconnect(cs, buf); sfree(buf); - goto dead; + return; } cs->recvbuf[cs->recvlen++] = c; } @@ -1785,7 +1785,7 @@ static void share_receive(Plug *plug, int urgent, const char *data, size_t len) char *buf = dupprintf("Version string did not have expected prefix\n"); share_disconnect(cs, buf); sfree(buf); - goto dead; + return; } if (cs->recvlen > 0 && cs->recvbuf[cs->recvlen-1] == '\015') cs->recvlen--; /* trim off \r before \n */ @@ -1810,7 +1810,7 @@ static void share_receive(Plug *plug, int urgent, const char *data, size_t len) (unsigned)cs->curr_packetlen); share_disconnect(cs, buf); sfree(buf); - goto dead; + return; } while (cs->recvlen < cs->curr_packetlen) { crGetChar(c); @@ -1821,7 +1821,6 @@ static void share_receive(Plug *plug, int urgent, const char *data, size_t len) cs->recvbuf + 5, cs->recvlen - 5); } - dead:; crFinishV; }