diff --git a/MANIFEST b/MANIFEST index b98dd825e..e93d3a9e0 100644 --- a/MANIFEST +++ b/MANIFEST @@ -341,6 +341,8 @@ plugins/sudoers/regress/visudo/test2.sh plugins/sudoers/regress/visudo/test3.err.ok plugins/sudoers/regress/visudo/test3.out.ok plugins/sudoers/regress/visudo/test3.sh +plugins/sudoers/regress/visudo/test4.out.ok +plugins/sudoers/regress/visudo/test4.sh plugins/sudoers/set_perms.c plugins/sudoers/sha2.c plugins/sudoers/sha2.h diff --git a/plugins/sudoers/alias.c b/plugins/sudoers/alias.c index 8ed4851d0..ccedd0347 100644 --- a/plugins/sudoers/alias.c +++ b/plugins/sudoers/alias.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2004-2005, 2007-2012 + * Copyright (c) 2004-2005, 2007-2013 * Todd C. Miller * * Permission to use, copy, modify, and distribute this software for any @@ -49,7 +49,6 @@ * Globals */ struct rbtree *aliases; -unsigned int alias_seqno; /* * Comparison function for the red-black tree. @@ -75,35 +74,48 @@ alias_compare(const void *v1, const void *v2) /* * Search the tree for an alias with the specified name and type. * Returns a pointer to the alias structure or NULL if not found. + * Caller is responsible for calling alias_put() on the returned + * alias to mark it as unused. */ struct alias * -alias_find(char *name, int type) +alias_get(char *name, int type) { struct alias key; struct rbnode *node; struct alias *a = NULL; - debug_decl(alias_find, SUDO_DEBUG_ALIAS) + debug_decl(alias_get, SUDO_DEBUG_ALIAS) key.name = name; key.type = type; if ((node = rbfind(aliases, &key)) != NULL) { /* - * Compare the global sequence number with the one stored - * in the alias. If they match then we've seen this alias - * before and found a loop. + * Check whether this alias is already in use. + * If so, we've detected a loop. If not, set the flag, + * which the caller should clear with a call to alias_put(). */ a = node->data; - if (a->seqno == alias_seqno) { + if (a->used) { errno = ELOOP; debug_return_ptr(NULL); } - a->seqno = alias_seqno; + a->used = true; } else { errno = ENOENT; } debug_return_ptr(a); } +/* + * Clear the "used" flag in an alias once the caller is done with it. + */ +void +alias_put(struct alias *a) +{ + debug_decl(alias_put, SUDO_DEBUG_ALIAS) + a->used = false; + debug_return; +} + /* * Add an alias to the aliases redblack tree. * Returns NULL on success and an error string on failure. @@ -118,7 +130,7 @@ alias_add(char *name, int type, struct member *members) a = ecalloc(1, sizeof(*a)); a->name = name; a->type = type; - /* a->seqno = 0; */ + /* a->used = false; */ list2tq(&a->members, members); if (rbinsert(aliases, a)) { snprintf(errbuf, sizeof(errbuf), N_("Alias `%s' already defined"), name); diff --git a/plugins/sudoers/match.c b/plugins/sudoers/match.c index cfd3df3ca..ac393fc4b 100644 --- a/plugins/sudoers/match.c +++ b/plugins/sudoers/match.c @@ -109,13 +109,13 @@ static bool command_matches_normal(char *, char *, struct sudo_digest *); * Check for user described by pw in a list of members. * Returns ALLOW, DENY or UNSPEC. */ -static int -_userlist_matches(struct passwd *pw, struct member_list *list) +int +userlist_matches(struct passwd *pw, struct member_list *list) { struct member *m; struct alias *a; int rval, matched = UNSPEC; - debug_decl(_userlist_matches, SUDO_DEBUG_MATCH) + debug_decl(userlist_matches, SUDO_DEBUG_MATCH) tq_foreach_rev(list, m) { switch (m->type) { @@ -131,10 +131,11 @@ _userlist_matches(struct passwd *pw, struct member_list *list) matched = !m->negated; break; case ALIAS: - if ((a = alias_find(m->name, USERALIAS)) != NULL) { - rval = _userlist_matches(pw, &a->members); + if ((a = alias_get(m->name, USERALIAS)) != NULL) { + rval = userlist_matches(pw, &a->members); if (rval != UNSPEC) matched = m->negated ? !rval : rval; + alias_put(a); break; } /* FALLTHROUGH */ @@ -149,20 +150,13 @@ _userlist_matches(struct passwd *pw, struct member_list *list) debug_return_bool(matched); } -int -userlist_matches(struct passwd *pw, struct member_list *list) -{ - alias_seqno++; - return _userlist_matches(pw, list); -} - /* * Check for user described by pw in a list of members. * If both lists are empty compare against def_runas_default. * Returns ALLOW, DENY or UNSPEC. */ -static int -_runaslist_matches(struct member_list *user_list, +int +runaslist_matches(struct member_list *user_list, struct member_list *group_list, struct member **matching_user, struct member **matching_group) { @@ -171,7 +165,7 @@ _runaslist_matches(struct member_list *user_list, int rval; int user_matched = UNSPEC; int group_matched = UNSPEC; - debug_decl(_runaslist_matches, SUDO_DEBUG_MATCH) + debug_decl(runaslist_matches, SUDO_DEBUG_MATCH) if (runas_pw != NULL) { /* If no runas user or runas group listed in sudoers, use default. */ @@ -192,11 +186,12 @@ _runaslist_matches(struct member_list *user_list, user_matched = !m->negated; break; case ALIAS: - if ((a = alias_find(m->name, RUNASALIAS)) != NULL) { - rval = _runaslist_matches(&a->members, &empty, + if ((a = alias_get(m->name, RUNASALIAS)) != NULL) { + rval = runaslist_matches(&a->members, &empty, matching_user, NULL); if (rval != UNSPEC) user_matched = m->negated ? !rval : rval; + alias_put(a); break; } /* FALLTHROUGH */ @@ -229,11 +224,12 @@ _runaslist_matches(struct member_list *user_list, group_matched = !m->negated; break; case ALIAS: - if ((a = alias_find(m->name, RUNASALIAS)) != NULL) { - rval = _runaslist_matches(&empty, &a->members, + if ((a = alias_get(m->name, RUNASALIAS)) != NULL) { + rval = runaslist_matches(&empty, &a->members, NULL, matching_group); if (rval != UNSPEC) group_matched = m->negated ? !rval : rval; + alias_put(a); break; } /* FALLTHROUGH */ @@ -261,27 +257,17 @@ _runaslist_matches(struct member_list *user_list, debug_return_int(UNSPEC); } -int -runaslist_matches(struct member_list *user_list, - struct member_list *group_list, struct member **matching_user, - struct member **matching_group) -{ - alias_seqno++; - return _runaslist_matches(user_list ? user_list : &empty, - group_list ? group_list : &empty, matching_user, matching_group); -} - /* * Check for host and shost in a list of members. * Returns ALLOW, DENY or UNSPEC. */ -static int -_hostlist_matches(struct member_list *list) +int +hostlist_matches(struct member_list *list) { struct member *m; struct alias *a; int rval, matched = UNSPEC; - debug_decl(_hostlist_matches, SUDO_DEBUG_MATCH) + debug_decl(hostlist_matches, SUDO_DEBUG_MATCH) tq_foreach_rev(list, m) { switch (m->type) { @@ -297,10 +283,11 @@ _hostlist_matches(struct member_list *list) matched = !m->negated; break; case ALIAS: - if ((a = alias_find(m->name, HOSTALIAS)) != NULL) { - rval = _hostlist_matches(&a->members); + if ((a = alias_get(m->name, HOSTALIAS)) != NULL) { + rval = hostlist_matches(&a->members); if (rval != UNSPEC) matched = m->negated ? !rval : rval; + alias_put(a); break; } /* FALLTHROUGH */ @@ -315,23 +302,16 @@ _hostlist_matches(struct member_list *list) debug_return_bool(matched); } -int -hostlist_matches(struct member_list *list) -{ - alias_seqno++; - return _hostlist_matches(list); -} - /* * Check for cmnd and args in a list of members. * Returns ALLOW, DENY or UNSPEC. */ -static int -_cmndlist_matches(struct member_list *list) +int +cmndlist_matches(struct member_list *list) { struct member *m; int matched = UNSPEC; - debug_decl(_cmndlist_matches, SUDO_DEBUG_MATCH) + debug_decl(cmndlist_matches, SUDO_DEBUG_MATCH) tq_foreach_rev(list, m) { matched = cmnd_matches(m); @@ -341,13 +321,6 @@ _cmndlist_matches(struct member_list *list) debug_return_bool(matched); } -int -cmndlist_matches(struct member_list *list) -{ - alias_seqno++; - return _cmndlist_matches(list); -} - /* * Check cmnd and args. * Returns ALLOW, DENY or UNSPEC. @@ -365,11 +338,11 @@ cmnd_matches(struct member *m) matched = !m->negated; break; case ALIAS: - alias_seqno++; - if ((a = alias_find(m->name, CMNDALIAS)) != NULL) { - rval = _cmndlist_matches(&a->members); + if ((a = alias_get(m->name, CMNDALIAS)) != NULL) { + rval = cmndlist_matches(&a->members); if (rval != UNSPEC) matched = m->negated ? !rval : rval; + alias_put(a); } break; case COMMAND: diff --git a/plugins/sudoers/parse.c b/plugins/sudoers/parse.c index f840d740c..f99f2d514 100644 --- a/plugins/sudoers/parse.c +++ b/plugins/sudoers/parse.c @@ -754,7 +754,7 @@ _print_member(struct lbuf *lbuf, char *name, int type, int negated, } break; case ALIAS: - if ((a = alias_find(name, alias_type)) != NULL) { + if ((a = alias_get(name, alias_type)) != NULL) { tq_foreach_fwd(&a->members, m) { if (m != tq_first(&a->members)) lbuf_append(lbuf, "%s", separator); @@ -762,6 +762,7 @@ _print_member(struct lbuf *lbuf, char *name, int type, int negated, negated ? !m->negated : m->negated, separator, alias_type); } + alias_put(a); break; } /* FALLTHROUGH */ @@ -775,7 +776,6 @@ _print_member(struct lbuf *lbuf, char *name, int type, int negated, static void print_member(struct lbuf *lbuf, struct member *m, int alias_type) { - alias_seqno++; _print_member(lbuf, m->name, m->type, m->negated, ", ", alias_type); } @@ -783,6 +783,5 @@ static void print_member2(struct lbuf *lbuf, struct member *m, const char *separator, int alias_type) { - alias_seqno++; _print_member(lbuf, m->name, m->type, m->negated, separator, alias_type); } diff --git a/plugins/sudoers/parse.h b/plugins/sudoers/parse.h index 965c6b963..a892e97b3 100644 --- a/plugins/sudoers/parse.h +++ b/plugins/sudoers/parse.h @@ -162,7 +162,7 @@ struct runascontainer { struct alias { char *name; /* alias name */ unsigned short type; /* {USER,HOST,RUNAS,CMND}ALIAS */ - unsigned short seqno; /* sequence number */ + bool used; /* "used" flag for cycle detection */ struct member_list members; /* list of alias members */ }; @@ -184,37 +184,43 @@ struct defaults { extern struct userspec_list userspecs; extern struct defaults_list defaults; -/* - * Alias sequence number to avoid loops. - */ -extern unsigned int alias_seqno; - -/* - * Prototypes - */ -char *alias_add(char *, int, struct member *); -bool addr_matches(char *); -int cmnd_matches(struct member *); -int cmndlist_matches(struct member_list *); -bool command_matches(char *, char *, struct sudo_digest *); -int hostlist_matches(struct member_list *); -bool hostname_matches(char *, char *, char *); -bool netgr_matches(char *, char *, char *, char *); +/* alias.c */ bool no_aliases(void); -int runaslist_matches(struct member_list *, struct member_list *, struct member **, struct member **); -int userlist_matches(struct passwd *, struct member_list *); -bool usergr_matches(char *, char *, struct passwd *); -bool userpw_matches(char *, char *, struct passwd *); -bool group_matches(char *, struct group *); -struct alias *alias_find(char *, int); -struct alias *alias_remove(char *, int); -void alias_free(void *); -void alias_apply(int (*)(void *, void *), void *); +char *alias_add(char *name, int type, struct member *members); +int alias_compare(const void *a1, const void *a2); +struct alias *alias_get(char *name, int type); +struct alias *alias_remove(char *name, int type); +void alias_apply(int (*func)(void *, void *), void *cookie); +void alias_free(void *a); +void alias_put(struct alias *a); void init_aliases(void); -void init_lexer(void); + +/* gram.c */ void init_parser(const char *, bool); -int alias_compare(const void *, const void *); + +/* match_addr.c */ +bool addr_matches(char *n); + +/* match.c */ +bool command_matches(char *sudoers_cmnd, char *sudoers_args, struct sudo_digest *digest); +bool group_matches(char *sudoers_group, struct group *gr); +bool hostname_matches(char *shost, char *lhost, char *pattern); +bool netgr_matches(char *netgr, char *lhost, char *shost, char *user); +bool usergr_matches(char *group, char *user, struct passwd *pw); +bool userpw_matches(char *sudoers_user, char *user, struct passwd *pw); +int cmnd_matches(struct member *m); +int cmndlist_matches(struct member_list *list); +int hostlist_matches(struct member_list *list); +int runaslist_matches(struct member_list *user_list, struct member_list *group_list, struct member **matching_user, struct member **matching_group); +int userlist_matches(struct passwd *pw, struct member_list *list); + +/* toke.c */ +void init_lexer(void); + +/* hexchar.c */ int hexchar(const char *s); + +/* base64.c */ size_t base64_decode(const char *str, unsigned char *dst, size_t dsize); #endif /* _SUDOERS_PARSE_H */ diff --git a/plugins/sudoers/regress/visudo/test4.out.ok b/plugins/sudoers/regress/visudo/test4.out.ok new file mode 100644 index 000000000..e5c355c2e --- /dev/null +++ b/plugins/sudoers/regress/visudo/test4.out.ok @@ -0,0 +1 @@ +stdin: parsed OK diff --git a/plugins/sudoers/regress/visudo/test4.sh b/plugins/sudoers/regress/visudo/test4.sh new file mode 100755 index 000000000..6f66b664d --- /dev/null +++ b/plugins/sudoers/regress/visudo/test4.sh @@ -0,0 +1,14 @@ +#!/bin/sh +# +# Test cycle detection and duplicate entries. +# Prior to sudo 1.8.7 this resulted in a false positive. +# + +./visudo -csf - <members, m) { if (m->type == ALIAS) errors += check_alias(m->name, type, strict, quiet); } + alias_put(a); } else { if (!quiet) { char *fmt; @@ -1137,26 +1137,22 @@ check_aliases(bool strict, bool quiet) tq_foreach_fwd(&userspecs, us) { tq_foreach_fwd(&us->users, m) { if (m->type == ALIAS) { - alias_seqno++; errors += check_alias(m->name, USERALIAS, strict, quiet); } } tq_foreach_fwd(&us->privileges, priv) { tq_foreach_fwd(&priv->hostlist, m) { if (m->type == ALIAS) { - alias_seqno++; errors += check_alias(m->name, HOSTALIAS, strict, quiet); } } tq_foreach_fwd(&priv->cmndlist, cs) { tq_foreach_fwd(&cs->runasuserlist, m) { if (m->type == ALIAS) { - alias_seqno++; errors += check_alias(m->name, RUNASALIAS, strict, quiet); } } if ((m = cs->cmnd)->type == ALIAS) { - alias_seqno++; errors += check_alias(m->name, CMNDALIAS, strict, quiet); } } @@ -1167,7 +1163,6 @@ check_aliases(bool strict, bool quiet) tq_foreach_fwd(&userspecs, us) { tq_foreach_fwd(&us->users, m) { if (m->type == ALIAS) { - alias_seqno++; if (!alias_remove_recursive(m->name, USERALIAS)) errors++; } @@ -1175,7 +1170,6 @@ check_aliases(bool strict, bool quiet) tq_foreach_fwd(&us->privileges, priv) { tq_foreach_fwd(&priv->hostlist, m) { if (m->type == ALIAS) { - alias_seqno++; if (!alias_remove_recursive(m->name, HOSTALIAS)) errors++; } @@ -1183,13 +1177,11 @@ check_aliases(bool strict, bool quiet) tq_foreach_fwd(&priv->cmndlist, cs) { tq_foreach_fwd(&cs->runasuserlist, m) { if (m->type == ALIAS) { - alias_seqno++; if (!alias_remove_recursive(m->name, RUNASALIAS)) errors++; } } if ((m = cs->cmnd)->type == ALIAS) { - alias_seqno++; if (!alias_remove_recursive(m->name, CMNDALIAS)) errors++; } @@ -1216,7 +1208,6 @@ check_aliases(bool strict, bool quiet) tq_foreach_fwd(&d->binding, binding) { for (m = binding; m != NULL; m = m->next) { if (m->type == ALIAS) { - alias_seqno++; if (!alias_remove_recursive(m->name, atype)) errors++; }