From 3ebcbc11efcd2a0faf9182d71e0e2f10b93d8781 Mon Sep 17 00:00:00 2001
From: Andrew Tridgell <andrew@tridgell.net>
Date: Fri, 12 Jun 2026 15:52:39 +1000
Subject: [PATCH] clientserver: harden daemon name-converter responses

Backport of the 3.5.0 daemon name-converter hardening:
 - reject an empty or non-numeric namecvt response instead of letting it
   collapse to uid/gid 0 (Mitchell Benjamin finding 2);
 - parse the id with overflow/sign checking;
 - reject unsafe (newline/control) tokens in the converter response.

Tests: daemon-namecvt-empty-response, daemon-namecvt-newline-token.

Reported-by: Mitchell Benjamin <mitchell@revampstudio.com.au>
(cherry picked from commit 91b09b6e38e7930efe04d8792025cb3a94816e61)
---
 clientserver.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/clientserver.c b/clientserver.c
index cc59663a..c88d02f4 100644
--- a/clientserver.c
+++ b/clientserver.c
@@ -1224,14 +1224,20 @@ static int rsync_module(int f_in, int f_out, int i, const char *addr, const char
 	return 0;
 }
 
+static BOOL namecvt_safe_token(const char *s);
+
 BOOL namecvt_call(const char *cmd, const char **name_p, id_t *id_p)
 {
 	char buf[1024];
 	int got, len;
 
-	if (*name_p)
+	if (*name_p) {
+		if (!namecvt_safe_token(*name_p)) {
+			rprintf(FERROR, "invalid name-converter token: %s\n", *name_p);
+			return False;
+		}
 		len = snprintf(buf, sizeof buf, "%s %s\n", cmd, *name_p);
-	else
+	} else
 		len = snprintf(buf, sizeof buf, "%s %ld\n", cmd, (long)*id_p);
 	if (len >= (int)sizeof buf) {
 		rprintf(FERROR, "namecvt_call() request was too large.\n");
@@ -1248,14 +1254,43 @@ BOOL namecvt_call(const char *cmd, const char **name_p, id_t *id_p)
 	if (!read_line_old(namecvt_fd_ans, buf, sizeof buf, 0))
 		return False;
 
-	if (*name_p)
-		*id_p = (id_t)atol(buf);
-	else
+	if (*name_p) {
+		/* Name-to-id branch.  An unknown name returns an empty line
+		 * (support/nameconvert), and atol("") = 0 would silently map every
+		 * daemon-unknown name to root.  strtoul() alone isn't strict enough
+		 * (accepts a leading '-' or whitespace, won't catch id_t overflow),
+		 * so check explicitly: every byte 0-9, no ERANGE, and the value fits
+		 * id_t (unsigned int, narrower than unsigned long on LP64).  On
+		 * failure recv_add_id() falls back to the sender's numeric id. */
+		const char *p;
+		unsigned long v;
+		if (!*buf)
+			return False;
+		for (p = buf; *p; p++) {
+			if (*p < '0' || *p > '9')
+				return False;
+		}
+		errno = 0;
+		v = strtoul(buf, NULL, 10);
+		if (errno == ERANGE || v > (unsigned long)(id_t)-1)
+			return False;
+		*id_p = (id_t)v;
+	} else
 		*name_p = strdup(buf);
 
 	return True;
 }
 
+static BOOL namecvt_safe_token(const char *s)
+{
+	for (; *s; s++) {
+		unsigned char ch = (unsigned char)*s;
+		if (ch < ' ' || ch == 0x7f)
+			return False;
+	}
+	return True;
+}
+
 /* send a list of available modules to the client. Don't list those
    with "list = False". */
 static void send_listing(int fd)