]> git.ipfire.org Git - thirdparty/rsync.git/commitdiff
socket: enforce socketpair_tcp()'s anti-hijack guarantee
authorAndrew Tridgell <andrew@tridgell.net>
Thu, 21 May 2026 02:38:14 +0000 (12:38 +1000)
committerAndrew Tridgell <andrew@tridgell.net>
Fri, 22 May 2026 04:34:52 +0000 (14:34 +1000)
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>
socket.c

index 6a8f6f4ae1393fc880700e825cfac3d09d10cc2f..d5aa0cb7101059ae052c506432286f58e3e19e2c 100644 (file)
--- a/socket.c
+++ b/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;