socket: reject over-long proxy response line

fixes a one byte stack overflow when using RSYNC_PROXY with a
malicious proxy.

Reach: only when RSYNC_PROXY is set and a malicious or MITM'd
proxy returns the pathological response.  The byte written is
always '\0' and the attacker doesn't choose the offset, so impact
is corruption of one adjacent stack byte and possible later
misbehaviour or crash -- no information disclosure beyond the
existing rprintf of buffer contents.

Reported by Aisle Research via Michal Ruprich

diff --git a/socket.c b/socket.c
index c2075adf8bb81cd05bc8342fdd37666704b7563e..6a8f6f4ae1393fc880700e825cfac3d09d10cc2f 100644 (file)
--- a/socket.c
+++ b/socket.c
@@ -47,21 +47,23 @@ static struct sigaction sigact;
 
 static int sock_exec(const char *prog);
 
+#define PROXY_BUF_SIZE 1024
+
 /* Establish a proxy connection on an open socket to a web proxy by using the
  * CONNECT method.  If proxy_user and proxy_pass are not NULL, they are used to
  * authenticate to the proxy using the "Basic" proxy-authorization protocol. */
 static int establish_proxy_connection(int fd, char *host, int port, char *proxy_user, char *proxy_pass)
 {
-       char *cp, buffer[1024];
-       char *authhdr, authbuf[1024];
+       char *cp, buffer[PROXY_BUF_SIZE + 1];
+       char *authhdr, authbuf[PROXY_BUF_SIZE + 1];
        int len;
 
        if (proxy_user && proxy_pass) {
-               stringjoin(buffer, sizeof buffer,
+               stringjoin(buffer, PROXY_BUF_SIZE,
                         proxy_user, ":", proxy_pass, NULL);
                len = strlen(buffer);
 
-               if ((len*8 + 5) / 6 >= (int)sizeof authbuf - 3) {
+               if ((len*8 + 5) / 6 >= PROXY_BUF_SIZE - 3) {
                        rprintf(FERROR,
                                "authentication information is too long\n");
                        return -1;
@@ -74,14 +76,14 @@ static int establish_proxy_connection(int fd, char *host, int port, char *proxy_
                authhdr = "";
        }
 
-       len = snprintf(buffer, sizeof buffer, "CONNECT %s:%d HTTP/1.0%s%s\r\n\r\n", host, port, authhdr, authbuf);
-       assert(len > 0 && len < (int)sizeof buffer);
+       len = snprintf(buffer, PROXY_BUF_SIZE, "CONNECT %s:%d HTTP/1.0%s%s\r\n\r\n", host, port, authhdr, authbuf);
+       assert(len > 0 && len < PROXY_BUF_SIZE);
        if (write(fd, buffer, len) != len) {
                rsyserr(FERROR, errno, "failed to write to proxy");
                return -1;
        }
 
-       for (cp = buffer; cp < &buffer[sizeof buffer - 1]; cp++) {
+       for (cp = buffer; cp < &buffer[PROXY_BUF_SIZE - 1]; cp++) {
                if (read(fd, cp, 1) != 1) {
                        rsyserr(FERROR, errno, "failed to read from proxy");
                        return -1;
@@ -90,11 +92,13 @@ static int establish_proxy_connection(int fd, char *host, int port, char *proxy_
                        break;
        }
 
-       if (*cp != '\n')
-               cp++;
-       *cp-- = '\0';
-       if (*cp == '\r')
-               *cp = '\0';
+       if (cp == &buffer[PROXY_BUF_SIZE - 1]) {
+               rprintf(FERROR, "proxy response line too long\n");
+               return -1;
+       }
+       *cp = '\0';
+       if (cp > buffer && cp[-1] == '\r')
+               cp[-1] = '\0';
        if (strncmp(buffer, "HTTP/", 5) != 0) {
                rprintf(FERROR, "bad response from proxy -- %s\n",
                        buffer);
@@ -110,7 +114,7 @@ static int establish_proxy_connection(int fd, char *host, int port, char *proxy_
        }
        /* throw away the rest of the HTTP header */
        while (1) {
-               for (cp = buffer; cp < &buffer[sizeof buffer - 1]; cp++) {
+               for (cp = buffer; cp < &buffer[PROXY_BUF_SIZE]; cp++) {
                        if (read(fd, cp, 1) != 1) {
                                rsyserr(FERROR, errno,
                                        "failed to read from proxy");

diff --git a/testsuite/proxy-response-line-too-long.test b/testsuite/proxy-response-line-too-long.test
new file mode 100755 (executable)
index 0000000..7f55c43
--- /dev/null
+++ b/testsuite/proxy-response-line-too-long.test
@@ -0,0 +1,128 @@
+#!/bin/sh
+
+# Copyright (C) 2026 by Andrew Tridgell
+
+# This program is distributable under the terms of the GNU GPL (see
+# COPYING).
+
+# Regression test for the off-by-one stack OOB write in
+# establish_proxy_connection() in socket.c when a malicious or
+# man-in-the-middle HTTP proxy returns a first response line of
+# 1023+ bytes without a '\n' terminator.
+#
+# Pre-fix: the read loop walked buffer[0..sizeof-2] one byte at a
+# time, then post-loop logic did "if (*cp != '\n') cp++; *cp-- =
+# '\0';".  If no newline arrived before the loop filled the buffer,
+# cp was left at &buffer[sizeof-1] (never written by the loop),
+# *cp held stale stack bytes, and cp++ pushed cp one past the array.
+# The null-termination then wrote one byte out of bounds on the
+# stack.  AddressSanitizer reports stack-buffer-overflow at the
+# null-termination site.
+#
+# Post-fix: the bound-exhaustion case is detected by position and
+# rejected with an "proxy response line too long" message, so no
+# OOB write occurs and rsync exits with a non-signal status.
+
+. "$suitedir/rsync.fns"
+
+command -v python3 >/dev/null 2>&1 || test_skipped "python3 not available"
+
+workdir="$scratchdir/workdir"
+mkdir -p "$workdir"
+cd "$workdir"
+
+port_file="$workdir/port"
+proxy_log="$workdir/proxy.log"
+
+# A minimal TCP listener: binds to an ephemeral port on 127.0.0.1,
+# writes the chosen port to $port_file *before* accept() so the test
+# can synchronise without a sleep, accepts one connection, reads
+# until end-of-headers or 64 KiB, sends exactly 1023 bytes of 'X'
+# with no '\n', then closes.
+python3 - "$port_file" >"$proxy_log" 2>&1 <<'PYEOF' &
+import socket, sys, os
+port_file = sys.argv[1]
+s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+s.bind(("127.0.0.1", 0))
+port = s.getsockname()[1]
+tmp = port_file + ".tmp"
+with open(tmp, "w") as fp:
+    fp.write("%d\n" % port)
+os.rename(tmp, port_file)   # atomic visibility to the shell side
+s.listen(1)
+conn, _ = s.accept()
+conn.settimeout(5)
+try:
+    data = b""
+    while b"\r\n\r\n" not in data and len(data) < 65536:
+        chunk = conn.recv(8192)
+        if not chunk:
+            break
+        data += chunk
+except socket.timeout:
+    pass
+conn.sendall(b"X" * 1023)    # exactly the buffer-1 trigger size
+try:
+    conn.shutdown(socket.SHUT_RDWR)
+except OSError:
+    pass
+conn.close()
+s.close()
+PYEOF
+proxy_pid=$!
+
+# Wait up to ~10s for the listener to publish its port.
+i=0
+while [ ! -s "$port_file" ] && [ $i -lt 10 ]; do
+    sleep 1
+    i=$((i + 1))
+done
+
+if [ ! -s "$port_file" ]; then
+    kill "$proxy_pid" 2>/dev/null
+    cat "$proxy_log" >&2 2>/dev/null
+    test_fail "proxy listener never published a port"
+fi
+
+port=`cat "$port_file"`
+case "$port" in
+    *[!0-9]*|"") kill "$proxy_pid" 2>/dev/null; test_fail "bogus port from listener: '$port'" ;;
+esac
+
+# Run rsync through the malicious proxy.  Any rsync:// URL works:
+# the proxy intercepts the CONNECT and never forwards anywhere.
+rsync_err="$workdir/rsync.err"
+
+# rsync MUST exit non-zero here (the proxy is misbehaving).
+# Use `|| status=$?` so we capture the real exit code under `sh -e`;
+# `if ! cmd; then status=$?` would only ever see 0 because the `!`
+# is the last command before `$?`.
+status=0
+RSYNC_PROXY="127.0.0.1:$port" \
+    $RSYNC rsync://example.invalid:873/whatever/ "$workdir/out/" \
+    >/dev/null 2>"$rsync_err" || status=$?
+
+# Reap the listener.
+wait "$proxy_pid" 2>/dev/null || true
+
+# 1. rsync must not have crashed (SIGSEGV/SIGABRT report >= 128).
+if [ "$status" -ge 128 ]; then
+    cat "$rsync_err" >&2
+    test_fail "rsync killed by signal (status=$status) -- possible stack OOB regression"
+fi
+
+# 2. rsync must have actually exited non-zero (i.e. saw the bad proxy).
+if [ "$status" -eq 0 ]; then
+    cat "$rsync_err" >&2
+    test_fail "rsync returned success despite malformed proxy response"
+fi
+
+# 3. The new error message must appear.
+if ! grep -q "proxy response line too long" "$rsync_err"; then
+    cat "$rsync_err" >&2
+    test_fail "expected 'proxy response line too long' in rsync stderr"
+fi
+
+echo "OK: over-long proxy response line rejected cleanly without crashing"
+exit 0