mirror of
https://github.com/RsyncProject/rsync.git
synced 2026-05-24 23:05:52 -04:00
socket: enforce socketpair_tcp()'s anti-hijack guarantee
socketpair_tcp() fakes a connected socket pair via a loopback TCP self-connect (socket -> bind 127.0.0.1:0 -> listen -> connect -> accept), used by sock_exec() for RSYNC_CONNECT_PROG. Its comment has long promised that "nobody else can attach to the socket, or if they do that this function fails", but nothing actually verified it: the code accept()ed whatever connection arrived first without checking it was the one our own connect() made. Between listen() and accept() the ephemeral loopback port is connectable by any local user. With backlog 1 a same-host attacker who races a connection in before our connect() lands could have their socket returned by accept(), handing them one end of the rsync protocol stream. The exposure is small (loopback only, random ephemeral port, sub-millisecond window, local users only), but the promised guarantee was simply not enforced. Enforce it: after the connection is established, require that the peer address of the accepted end (fd[0]) equals the local address of our connecting end (fd[1]), and that both are 127.0.0.1. A hijacked connection has a different source port and is rejected (errno EPERM, fail closed). The legitimate self-connect always matches, so there is no behaviour change for the normal path. Verified: rebuilds clean with -Wall -W; the full testsuite still passes in both transports (pipe `make check` 57/3, `runtests.py --use-tcp` 59/1) -- the pipe transport exercises this code path on every daemon test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
30
socket.c
30
socket.c
@@ -739,8 +739,12 @@ void set_socket_options(int fd, char *options)
|
||||
|
||||
/* This is like socketpair but uses tcp. The function guarantees that nobody
|
||||
* else can attach to the socket, or if they do that this function fails and
|
||||
* the socket gets closed. Returns 0 on success, -1 on failure. The resulting
|
||||
* file descriptors are symmetrical. Currently only for RSYNC_CONNECT_PROG. */
|
||||
* the socket gets closed. The anti-hijack guarantee is enforced after the
|
||||
* accept() below: a local attacker who races a connection in on the loopback
|
||||
* listener before our own connect() lands would be detected by the peer-vs-
|
||||
* local address comparison and the function fails. Returns 0 on success, -1
|
||||
* on failure. The resulting file descriptors are symmetrical. Currently
|
||||
* only for RSYNC_CONNECT_PROG. */
|
||||
static int socketpair_tcp(int fd[2])
|
||||
{
|
||||
int listener;
|
||||
@@ -792,6 +796,28 @@ static int socketpair_tcp(int fd[2])
|
||||
goto failed;
|
||||
}
|
||||
|
||||
/* Confirm that the connection we accepted is the one we just made, and
|
||||
* not one a local attacker raced in on the loopback listener before our
|
||||
* own connect() completed. The peer of the accepted end (fd[0]) must be
|
||||
* the local address of our connecting end (fd[1]), and both must be
|
||||
* loopback. If they differ, someone else connected first; fail closed. */
|
||||
{
|
||||
struct sockaddr_in accepted_peer, our_local;
|
||||
socklen_t plen = sizeof accepted_peer;
|
||||
socklen_t llen = sizeof our_local;
|
||||
|
||||
if (getpeername(fd[0], (struct sockaddr *)&accepted_peer, &plen) != 0
|
||||
|| getsockname(fd[1], (struct sockaddr *)&our_local, &llen) != 0
|
||||
|| accepted_peer.sin_family != AF_INET
|
||||
|| our_local.sin_family != AF_INET
|
||||
|| accepted_peer.sin_addr.s_addr != htonl(INADDR_LOOPBACK)
|
||||
|| our_local.sin_addr.s_addr != htonl(INADDR_LOOPBACK)
|
||||
|| accepted_peer.sin_port != our_local.sin_port) {
|
||||
errno = EPERM;
|
||||
goto failed;
|
||||
}
|
||||
}
|
||||
|
||||
/* all OK! */
|
||||
return 0;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user