From e7986502cb38e8a6f02f4bc393e697b9f6a7a359 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Wed, 29 Jul 2026 13:07:58 +1000 Subject: [PATCH] auth: parse "auth users" with conf_strtok so a leading comma means commas only auth_server() tokenised on commas AND whitespace, ignoring the documented comma-only form, so an entry containing a space was torn in two: the rule the administrator wrote never matched, and a rule they never wrote appeared from its tail. For "@Group Name:deny" that means the deny is skipped and a later :rw entry can match instead -- an authorization bypass for a member of the denied group. conf_strtok() already implements the documented behaviour and the daemon's gid field already uses it (clientserver.c); this consumer was missed when that one was fixed. Reported by Andres Berbescu. Refs #137. --- authenticate.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/authenticate.c b/authenticate.c index 51240bc9..3376bb1e 100644 --- a/authenticate.c +++ b/authenticate.c @@ -347,7 +347,13 @@ char *auth_server(int f_in, int f_out, int module, const char *host, users = strdup(users); - for (tok = strtok(users, " ,\t"); tok; tok = strtok(NULL, " ,\t")) { + /* conf_strtok() honours the documented leading-comma form: a value that + * starts with a comma splits on commas ALONE, so an entry may contain + * spaces -- which is how a group name with a space is written. Splitting + * on whitespace here tore such an entry apart, so the rule the admin wrote + * never matched and a rule they never wrote appeared from its tail. The + * daemon's gid field already uses this parser (clientserver.c). */ + for (tok = conf_strtok(users); tok; tok = conf_strtok(NULL)) { char *opts; /* See if the user appended :deny, :ro, or :rw. */ if ((opts = strchr(tok, ':')) != NULL) {