Skip to content

Commit 5066cce

Browse files
committed
Fix evaluation of negative non-unix groups (and netgroups) in sudoUser.
When support was added for negated entries in a sudoUser, the loop invariant in sudo_ldap_check_non_unix_group() was not modified to continue checking event after a positive match was found. Also, the logic to handle a negated match could be triggered by a previous match, not the current one. Reported by Christos Papakonstantinou from Cantina (cantina.xyz)
1 parent 00c6075 commit 5066cce

1 file changed

Lines changed: 15 additions & 12 deletions

File tree

plugins/sudoers/ldap.c

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*
22
* SPDX-License-Identifier: ISC
33
*
4-
* Copyright (c) 2003-2023 Todd C. Miller <[email protected]>
4+
* Copyright (c) 2003-2023, 20255-2026 Todd C. Miller <[email protected]>
55
*
66
* This code is derived from software contributed by Aaron Spangler.
77
*
@@ -315,31 +315,34 @@ sudo_ldap_check_non_unix_group(struct sudoers_context *ctx,
315315
}
316316

317317
/* walk through values */
318-
for (p = bv; *p != NULL && !ret; p++) {
318+
for (p = bv; *p != NULL; p++) {
319319
bool negated = false;
320+
int match = DENY;
320321
const char *val = (*p)->bv_val;
321322

322323
if (*val == '!') {
323324
val++;
324325
negated = true;
325326
}
326327
if (*val == '+') {
327-
if (netgr_matches(nss, val,
328+
match = netgr_matches(nss, val,
328329
def_netgroup_tuple ? ctx->runas.host : NULL,
329-
def_netgroup_tuple ? ctx->runas.shost : NULL, pw->pw_name) == ALLOW)
330-
ret = true;
330+
def_netgroup_tuple ? ctx->runas.shost : NULL, pw->pw_name);
331331
DPRINTF2("ldap sudoUser netgroup '%s%s' ... %s",
332-
negated ? "!" : "", val, ret ? "MATCH!" : "not");
332+
negated ? "!" : "", val, match == ALLOW ? "MATCH!" : "not");
333333
} else {
334334
if (group_plugin_query(pw->pw_name, val + 2, pw))
335-
ret = true;
335+
match = ALLOW;
336336
DPRINTF2("ldap sudoUser non-Unix group '%s%s' ... %s",
337-
negated ? "!" : "", val, ret ? "MATCH!" : "not");
337+
negated ? "!" : "", val, match == ALLOW ? "MATCH!" : "not");
338338
}
339-
/* A negated match overrides all other entries. */
340-
if (ret && negated) {
341-
ret = false;
342-
break;
339+
if (match == ALLOW) {
340+
if (negated) {
341+
/* A negated match overrides all other entries. */
342+
ret = false;
343+
break;
344+
}
345+
ret = true;
343346
}
344347
}
345348

0 commit comments

Comments
 (0)