1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-10 09:58:01 +00:00
putty-source/unix/uxagentc.c
Simon Tatham 12aa06ccc9 Fix double-free in remote->local forwardings.
This bug applies to both the new stream-based agent forwarding, and
ordinary remote->local TCP port forwardings, because it was introduced
by the preliminary infrastructure in commit 09954a87c.

new_connection() and sk_new() accept a SockAddr *, and take ownership
of it. So it's a mistake to make an address, connect to it, and then
sk_addr_free() it: the free will decrement its reference count to
zero, and then the Socket made by the connection will be holding a
stale pointer. But that's exactly what I was doing in the version of
portfwdmgr_connect() that I rewrote in that refactoring. And then I
made the same error again in commit ae1148267 in the Unix stream-based
agent forwarding.

Now both fixed. Rather than remove the sk_addr_free() to make the code
look more like it used to, I've instead solved the problem by adding
an sk_addr_dup() at the point of making the connection. The idea is
that that should be more robust, in that it will still do the right
thing if portfwdmgr_connect_socket should later change so as not to
call its connect helper function at all.

The new Windows stream-based agent forwarding is unaffected by this
bug, because it calls new_named_pipe_client() with a pathname in
string format, without first wrapping it into a SockAddr.
2020-01-14 19:52:54 +00:00

245 lines
6.1 KiB
C

/*
* SSH agent client code.
*/
#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
#include <unistd.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <fcntl.h>
#include "putty.h"
#include "misc.h"
#include "tree234.h"
#include "puttymem.h"
bool agent_exists(void)
{
const char *p = getenv("SSH_AUTH_SOCK");
if (p && *p)
return true;
return false;
}
static tree234 *agent_pending_queries;
struct agent_pending_query {
int fd;
char *retbuf;
char sizebuf[4];
int retsize, retlen;
void (*callback)(void *, void *, int);
void *callback_ctx;
};
static int agent_conncmp(void *av, void *bv)
{
agent_pending_query *a = (agent_pending_query *) av;
agent_pending_query *b = (agent_pending_query *) bv;
if (a->fd < b->fd)
return -1;
if (a->fd > b->fd)
return +1;
return 0;
}
static int agent_connfind(void *av, void *bv)
{
int afd = *(int *) av;
agent_pending_query *b = (agent_pending_query *) bv;
if (afd < b->fd)
return -1;
if (afd > b->fd)
return +1;
return 0;
}
/*
* Attempt to read from an agent socket fd. Returns false if the
* expected response is as yet incomplete; returns true if it's either
* complete (conn->retbuf non-NULL and filled with something useful)
* or has failed totally (conn->retbuf is NULL).
*/
static bool agent_try_read(agent_pending_query *conn)
{
int ret;
ret = read(conn->fd, conn->retbuf+conn->retlen,
conn->retsize-conn->retlen);
if (ret <= 0) {
if (conn->retbuf != conn->sizebuf) sfree(conn->retbuf);
conn->retbuf = NULL;
conn->retlen = 0;
return true;
}
conn->retlen += ret;
if (conn->retsize == 4 && conn->retlen == 4) {
conn->retsize = toint(GET_32BIT_MSB_FIRST(conn->retbuf) + 4);
if (conn->retsize <= 0) {
conn->retbuf = NULL;
conn->retlen = 0;
return true; /* way too large */
}
assert(conn->retbuf == conn->sizebuf);
conn->retbuf = snewn(conn->retsize, char);
memcpy(conn->retbuf, conn->sizebuf, 4);
}
if (conn->retlen < conn->retsize)
return false; /* more data to come */
return true;
}
void agent_cancel_query(agent_pending_query *conn)
{
uxsel_del(conn->fd);
close(conn->fd);
del234(agent_pending_queries, conn);
if (conn->retbuf && conn->retbuf != conn->sizebuf)
sfree(conn->retbuf);
sfree(conn);
}
static void agent_select_result(int fd, int event)
{
agent_pending_query *conn;
assert(event == SELECT_R); /* not selecting for anything but R */
conn = find234(agent_pending_queries, &fd, agent_connfind);
if (!conn) {
uxsel_del(fd);
return;
}
if (!agent_try_read(conn))
return; /* more data to come */
/*
* We have now completed the agent query. Do the callback.
*/
conn->callback(conn->callback_ctx, conn->retbuf, conn->retlen);
/* Null out conn->retbuf, since ownership of that buffer has
* passed to the callback. */
conn->retbuf = NULL;
agent_cancel_query(conn);
}
static const char *agent_socket_path(void)
{
return getenv("SSH_AUTH_SOCK");
}
struct agent_connect_ctx {
SockAddr *addr;
};
Socket *agent_connect(void *vctx, Plug *plug)
{
agent_connect_ctx *ctx = (agent_connect_ctx *)vctx;
return sk_new(sk_addr_dup(ctx->addr), 0, false, false, false, false, plug);
}
agent_connect_ctx *agent_get_connect_ctx(void)
{
const char *path = agent_socket_path();
if (!path)
return NULL;
agent_connect_ctx *ctx = snew(agent_connect_ctx);
ctx->addr = unix_sock_addr(path);
return ctx;
}
void agent_free_connect_ctx(agent_connect_ctx *ctx)
{
sk_addr_free(ctx->addr);
sfree(ctx);
}
agent_pending_query *agent_query(
strbuf *query, void **out, int *outlen,
void (*callback)(void *, void *, int), void *callback_ctx)
{
const char *name;
int sock;
struct sockaddr_un addr;
int done;
agent_pending_query *conn;
name = agent_socket_path();
if (!name || strlen(name) >= sizeof(addr.sun_path))
goto failure;
sock = socket(PF_UNIX, SOCK_STREAM, 0);
if (sock < 0) {
perror("socket(PF_UNIX)");
exit(1);
}
cloexec(sock);
addr.sun_family = AF_UNIX;
strcpy(addr.sun_path, name);
if (connect(sock, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
close(sock);
goto failure;
}
strbuf_finalise_agent_query(query);
for (done = 0; done < query->len ;) {
int ret = write(sock, query->s + done,
query->len - done);
if (ret <= 0) {
close(sock);
goto failure;
}
done += ret;
}
conn = snew(agent_pending_query);
conn->fd = sock;
conn->retbuf = conn->sizebuf;
conn->retsize = 4;
conn->retlen = 0;
conn->callback = callback;
conn->callback_ctx = callback_ctx;
if (!callback) {
/*
* Bodge to permit making deliberately synchronous agent
* requests. Used by Unix Pageant in command-line client mode,
* which is legit because it really is true that no other part
* of the program is trying to get anything useful done
* simultaneously. But this special case shouldn't be used in
* any more general program.
*/
no_nonblock(conn->fd);
while (!agent_try_read(conn))
/* empty loop body */;
*out = conn->retbuf;
*outlen = conn->retlen;
sfree(conn);
return NULL;
}
/*
* Otherwise do it properly: add conn to the tree of agent
* connections currently in flight, return 0 to indicate that the
* response hasn't been received yet, and call the callback when
* select_result comes back to us.
*/
if (!agent_pending_queries)
agent_pending_queries = newtree234(agent_conncmp);
add234(agent_pending_queries, conn);
uxsel_set(sock, SELECT_R, agent_select_result);
return conn;
failure:
*out = NULL;
*outlen = 0;
return NULL;
}