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

Fix use-after-free on error returns from share_receive.

Browse Source 
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.
This commit is contained in:
Simon Tatham 2023-08-19 09:59:01 +01:00
parent 74820e9408
commit 27f0140e5c
1 changed files with 3 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
ssh/sharing.c
View File

@ -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"); char *buf = dupprintf("Version string far too long\n");
share_disconnect(cs, buf); share_disconnect(cs, buf);
sfree(buf); sfree(buf);
goto dead; return;
} }
cs->recvbuf[cs->recvlen++] = c; 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"); char *buf = dupprintf("Version string did not have expected prefix\n");
share_disconnect(cs, buf); share_disconnect(cs, buf);
sfree(buf); sfree(buf);
goto dead; return;
} }
if (cs->recvlen > 0 && cs->recvbuf[cs->recvlen-1] == '\015') if (cs->recvlen > 0 && cs->recvbuf[cs->recvlen-1] == '\015')
cs->recvlen--; /* trim off \r before \n */ 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); (unsigned)cs->curr_packetlen);
share_disconnect(cs, buf); share_disconnect(cs, buf);
sfree(buf); sfree(buf);
goto dead; return;
} }
while (cs->recvlen < cs->curr_packetlen) { while (cs->recvlen < cs->curr_packetlen) {
crGetChar(c); 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); cs->recvbuf + 5, cs->recvlen - 5);
} }
dead:;
crFinishV; crFinishV;
} }