diff --git a/plugins/sudoers/alias.c b/plugins/sudoers/alias.c index 3adfda8a3..d435400f7 100644 --- a/plugins/sudoers/alias.c +++ b/plugins/sudoers/alias.c @@ -353,20 +353,20 @@ alias_find_used(struct sudoers_parse_tree *parse_tree, struct rbtree *used_alias TAILQ_FOREACH(d, &parse_tree->defaults, entries) { switch (d->type) { case DEFAULTS_HOST: - errors += alias_find_used_members(parse_tree, d->binding, - HOSTALIAS, used_aliases); + errors += alias_find_used_members(parse_tree, + &d->binding->members, HOSTALIAS, used_aliases); break; case DEFAULTS_USER: - errors += alias_find_used_members(parse_tree, d->binding, - USERALIAS, used_aliases); + errors += alias_find_used_members(parse_tree, + &d->binding->members, USERALIAS, used_aliases); break; case DEFAULTS_RUNAS: - errors += alias_find_used_members(parse_tree, d->binding, - RUNASALIAS, used_aliases); + errors += alias_find_used_members(parse_tree, + &d->binding->members, RUNASALIAS, used_aliases); break; case DEFAULTS_CMND: - errors += alias_find_used_members(parse_tree, d->binding, - CMNDALIAS, used_aliases); + errors += alias_find_used_members(parse_tree, + &d->binding->members, CMNDALIAS, used_aliases); break; default: break; diff --git a/plugins/sudoers/cvtsudoers.c b/plugins/sudoers/cvtsudoers.c index fe42c7f60..d8860b518 100644 --- a/plugins/sudoers/cvtsudoers.c +++ b/plugins/sudoers/cvtsudoers.c @@ -1233,7 +1233,6 @@ filter_defaults(struct sudoers_parse_tree *parse_tree, struct member_list runas_aliases = TAILQ_HEAD_INITIALIZER(runas_aliases); struct member_list host_aliases = TAILQ_HEAD_INITIALIZER(host_aliases); struct member_list cmnd_aliases = TAILQ_HEAD_INITIALIZER(cmnd_aliases); - struct member_list *prev_binding = NULL; struct defaults *def, *def_next; struct member *m, *m_next; int alias_type; @@ -1253,8 +1252,10 @@ filter_defaults(struct sudoers_parse_tree *parse_tree, break; case DEFAULTS_USER: if (!ISSET(conf->defaults, CVT_DEFAULTS_USER) || - !userlist_matches_filter(parse_tree, def->binding, conf)) + !userlist_matches_filter(parse_tree, &def->binding->members, + conf)) { keep = false; + } alias_type = USERALIAS; break; case DEFAULTS_RUNAS: @@ -1264,14 +1265,18 @@ filter_defaults(struct sudoers_parse_tree *parse_tree, break; case DEFAULTS_HOST: if (!ISSET(conf->defaults, CVT_DEFAULTS_HOST) || - !hostlist_matches_filter(parse_tree, def->binding, conf)) + !hostlist_matches_filter(parse_tree, &def->binding->members, + conf)) { keep = false; + } alias_type = HOSTALIAS; break; case DEFAULTS_CMND: if (!ISSET(conf->defaults, CVT_DEFAULTS_CMND) || - !cmndlist_matches_filter(parse_tree, def->binding, conf)) + !cmndlist_matches_filter(parse_tree, &def->binding->members, + conf)) { keep = false; + } alias_type = CMNDALIAS; break; default: @@ -1280,12 +1285,16 @@ filter_defaults(struct sudoers_parse_tree *parse_tree, } if (!keep) { - /* Look for aliases used by the binding. */ + /* + * Look for aliases used by the binding. + * Consecutive Defaults can share the same binding. + */ /* XXX - move to function */ - if (alias_type != UNSPEC && def->binding != prev_binding) { - TAILQ_FOREACH_SAFE(m, def->binding, entries, m_next) { + if (alias_type != UNSPEC && + (def_next == NULL || def->binding != def_next->binding)) { + TAILQ_FOREACH_SAFE(m, &def->binding->members, entries, m_next) { if (m->type == ALIAS) { - TAILQ_REMOVE(def->binding, m, entries); + TAILQ_REMOVE(&def->binding->members, m, entries); switch (alias_type) { case USERALIAS: TAILQ_INSERT_TAIL(&user_aliases, m, entries); @@ -1308,18 +1317,7 @@ filter_defaults(struct sudoers_parse_tree *parse_tree, } } TAILQ_REMOVE(&parse_tree->defaults, def, entries); - free_default(def, &prev_binding); - if (prev_binding != NULL) { - /* Remove and free Defaults that share the same binding. */ - while (def_next != NULL && def_next->binding == prev_binding) { - def = def_next; - def_next = TAILQ_NEXT(def, entries); - TAILQ_REMOVE(&parse_tree->defaults, def, entries); - free_default(def, &prev_binding); - } - } - } else { - prev_binding = def->binding; + free_default(def); } } diff --git a/plugins/sudoers/cvtsudoers_csv.c b/plugins/sudoers/cvtsudoers_csv.c index db68524db..1c3da190d 100644 --- a/plugins/sudoers/cvtsudoers_csv.c +++ b/plugins/sudoers/cvtsudoers_csv.c @@ -288,7 +288,7 @@ print_member_list_csv(FILE *fp, struct sudoers_parse_tree *parse_tree, */ static void print_defaults_binding_csv(FILE *fp, struct sudoers_parse_tree *parse_tree, - struct member_list *binding, int type, bool expand_aliases) + struct defaults_binding *binding, int type, bool expand_aliases) { int alias_type; debug_decl(print_defaults_binding_csv, SUDOERS_DEBUG_UTIL); @@ -296,8 +296,8 @@ print_defaults_binding_csv(FILE *fp, struct sudoers_parse_tree *parse_tree, if (type != DEFAULTS) { /* Print each member object in binding. */ alias_type = defaults_to_alias_type(type); - print_member_list_csv(fp, parse_tree, binding, false, alias_type, - expand_aliases); + print_member_list_csv(fp, parse_tree, &binding->members, false, + alias_type, expand_aliases); } debug_return; diff --git a/plugins/sudoers/cvtsudoers_json.c b/plugins/sudoers/cvtsudoers_json.c index 373533026..dfd143d2e 100644 --- a/plugins/sudoers/cvtsudoers_json.c +++ b/plugins/sudoers/cvtsudoers_json.c @@ -371,18 +371,18 @@ print_alias_json(struct sudoers_parse_tree *parse_tree, struct alias *a, void *v */ static void print_binding_json(struct json_container *jsonc, - struct sudoers_parse_tree *parse_tree, struct member_list *binding, + struct sudoers_parse_tree *parse_tree, struct defaults_binding *binding, int type, bool expand_aliases) { struct member *m; debug_decl(print_binding_json, SUDOERS_DEBUG_UTIL); - if (TAILQ_EMPTY(binding)) + if (TAILQ_EMPTY(&binding->members)) debug_return; /* Print each member object in binding. */ sudo_json_open_array(jsonc, "Binding"); - TAILQ_FOREACH(m, binding, entries) { + TAILQ_FOREACH(m, &binding->members, entries) { print_member_json(jsonc, parse_tree, m, defaults_to_word_type(type), expand_aliases); } diff --git a/plugins/sudoers/cvtsudoers_merge.c b/plugins/sudoers/cvtsudoers_merge.c index 55ef2aa14..82cfd2033 100644 --- a/plugins/sudoers/cvtsudoers_merge.c +++ b/plugins/sudoers/cvtsudoers_merge.c @@ -220,48 +220,46 @@ static void alias_rename_defaults(const char *old_name, const char *new_name, int alias_type, struct defaults_list *defaults) { - struct defaults *def; - struct member_list *prev_binding = NULL; + struct defaults *def, *def_next; struct member *m; debug_decl(alias_rename_defaults, SUDOERS_DEBUG_ALIAS); - TAILQ_FOREACH(def, defaults, entries) { - if (def->binding == prev_binding) + TAILQ_FOREACH_SAFE(def, defaults, entries, def_next) { + /* Consecutive Defaults can share the same binding. */ + if (def_next != NULL && def->binding == def_next->binding) continue; + switch (def->type) { case DEFAULTS_USER: if (alias_type != USERALIAS) - goto wrong_type; + continue; break; case DEFAULTS_RUNAS: if (alias_type != RUNASALIAS) - goto wrong_type; + continue; break; case DEFAULTS_HOST: if (alias_type != HOSTALIAS) - goto wrong_type; + continue; break; default: - wrong_type: - prev_binding = NULL; continue; } - if (def->binding != NULL) { - TAILQ_FOREACH(m, def->binding, entries) { - if (m->type != ALIAS) - continue; - if (strcmp(m->name, old_name) == 0) { - char *copy = strdup(new_name); - if (copy == NULL) { - sudo_fatalx(U_("%s: %s"), __func__, - U_("unable to allocate memory")); - } - free(m->name); - m->name = copy; + + /* Rename matching aliases in the binding's member_list. */ + TAILQ_FOREACH(m, &def->binding->members, entries) { + if (m->type != ALIAS) + continue; + if (strcmp(m->name, old_name) == 0) { + char *copy = strdup(new_name); + if (copy == NULL) { + sudo_fatalx(U_("%s: %s"), __func__, + U_("unable to allocate memory")); } + free(m->name); + m->name = copy; } } - prev_binding = def->binding; } debug_return; @@ -493,7 +491,7 @@ defaults_var_matches(struct defaults *d1, struct defaults *d2) if (strcmp(d1->var, d2->var) != 0) debug_return_bool(false); if (d1->type != DEFAULTS) { - if (!member_list_equivalent(d1->binding, d2->binding)) + if (!member_list_equivalent(&d1->binding->members, &d2->binding->members)) debug_return_bool(false); } @@ -618,16 +616,18 @@ merge_defaults(struct sudoers_parse_tree_list *parse_trees, } m->type = WORD; m->negated = false; - TAILQ_INIT(def->binding); - TAILQ_INSERT_TAIL(def->binding, m, entries); + TAILQ_INIT(&def->binding->members); + def->binding->refcnt = 1; + TAILQ_INSERT_TAIL(&def->binding->members, m, entries); def->type = DEFAULTS_HOST; } /* * Only add Defaults entry if not overridden by subsequent sudoers. - * XXX - leaks def if not merged but can't free due to binding. */ - if (!defaults_has_conflict(def, parse_tree)) { + if (defaults_has_conflict(def, parse_tree)) { + free_default(def); + } else { if (def->type != DEFAULTS_HOST) { sudo_warnx(U_("%s:%d:%d: unable to make Defaults \"%s\" host-specific"), def->file, def->line, def->column, def->var); } diff --git a/plugins/sudoers/defaults.c b/plugins/sudoers/defaults.c index 7375b7221..82d189fad 100644 --- a/plugins/sudoers/defaults.c +++ b/plugins/sudoers/defaults.c @@ -714,19 +714,19 @@ default_binding_matches(struct sudoers_parse_tree *parse_tree, case DEFAULTS: debug_return_bool(true); case DEFAULTS_USER: - if (userlist_matches(parse_tree, sudo_user.pw, d->binding) == ALLOW) + if (userlist_matches(parse_tree, sudo_user.pw, &d->binding->members) == ALLOW) debug_return_bool(true); break; case DEFAULTS_RUNAS: - if (runaslist_matches(parse_tree, d->binding, NULL, NULL, NULL) == ALLOW) + if (runaslist_matches(parse_tree, &d->binding->members, NULL, NULL, NULL) == ALLOW) debug_return_bool(true); break; case DEFAULTS_HOST: - if (hostlist_matches(parse_tree, sudo_user.pw, d->binding) == ALLOW) + if (hostlist_matches(parse_tree, sudo_user.pw, &d->binding->members) == ALLOW) debug_return_bool(true); break; case DEFAULTS_CMND: - if (cmndlist_matches(parse_tree, d->binding, NULL, NULL) == ALLOW) + if (cmndlist_matches(parse_tree, &d->binding->members, NULL, NULL) == ALLOW) debug_return_bool(true); break; } diff --git a/plugins/sudoers/fmtsudoers_cvt.c b/plugins/sudoers/fmtsudoers_cvt.c index a7f95d0ad..ad750fa10 100644 --- a/plugins/sudoers/fmtsudoers_cvt.c +++ b/plugins/sudoers/fmtsudoers_cvt.c @@ -194,8 +194,8 @@ sudoers_format_default_line(struct sudo_lbuf *lbuf, alias_type = UNSPEC; break; } - TAILQ_FOREACH(m, d->binding, entries) { - if (m != TAILQ_FIRST(d->binding)) + TAILQ_FOREACH(m, &d->binding->members, entries) { + if (m != TAILQ_FIRST(&d->binding->members)) sudo_lbuf_append(lbuf, ", "); sudoers_format_member(lbuf, parse_tree, m, ", ", alias_type); } diff --git a/plugins/sudoers/gram.c b/plugins/sudoers/gram.c index ca5b9d79e..c06d24818 100644 --- a/plugins/sudoers/gram.c +++ b/plugins/sudoers/gram.c @@ -3421,7 +3421,7 @@ new_default(char *var, char *val, short op) d->val = val; /* d->type = 0; */ d->op = op; - /* d->binding = NULL */ + /* d->binding = NULL; */ d->line = this_lineno; d->column = sudolinebuf.toke_start + 1; d->file = sudo_rcstr_addref(sudoers); @@ -3494,6 +3494,22 @@ new_digest(int digest_type, char *digest_str) debug_return_ptr(digest); } +static void +free_defaults_binding(struct defaults_binding *binding) +{ + debug_decl(free_defaults_binding, SUDOERS_DEBUG_PARSER); + + /* Bindings may be shared among multiple Defaults entries. */ + if (binding != NULL) { + if (--binding->refcnt == 0) { + free_members(&binding->members); + free(binding); + } + } + + debug_return; +} + /* * Add a list of defaults structures to the defaults list. * The binding, if non-NULL, specifies a list of hosts, users, or @@ -3503,7 +3519,7 @@ static bool add_defaults(int type, struct member *bmem, struct defaults *defs) { struct defaults *d, *next; - struct member_list *binding; + struct defaults_binding *binding; bool ret = true; debug_decl(add_defaults, SUDOERS_DEBUG_PARSER); @@ -3519,10 +3535,11 @@ add_defaults(int type, struct member *bmem, struct defaults *defs) } if (bmem != NULL) { parser_leak_remove(LEAK_MEMBER, bmem); - HLTQ_TO_TAILQ(binding, bmem, entries); + HLTQ_TO_TAILQ(&binding->members, bmem, entries); } else { - TAILQ_INIT(binding); + TAILQ_INIT(&binding->members); } + binding->refcnt = 0; /* * Set type and binding (who it applies to) for new entries. @@ -3532,6 +3549,7 @@ add_defaults(int type, struct member *bmem, struct defaults *defs) HLTQ_FOREACH_SAFE(d, defs, entries, next) { d->type = type; d->binding = binding; + binding->refcnt++; TAILQ_INSERT_TAIL(&parsed_policy.defaults, d, entries); } } @@ -3612,30 +3630,23 @@ free_members(struct member_list *members) void free_defaults(struct defaults_list *defs) { - struct member_list *prev_binding = NULL; struct defaults *def; debug_decl(free_defaults, SUDOERS_DEBUG_PARSER); while ((def = TAILQ_FIRST(defs)) != NULL) { TAILQ_REMOVE(defs, def, entries); - free_default(def, &prev_binding); + free_default(def); } debug_return; } void -free_default(struct defaults *def, struct member_list **binding) +free_default(struct defaults *def) { debug_decl(free_default, SUDOERS_DEBUG_PARSER); - if (def->binding != *binding) { - *binding = def->binding; - if (def->binding != NULL) { - free_members(def->binding); - free(def->binding); - } - } + free_defaults_binding(def->binding); sudo_rcstr_delref(def->file); free(def->var); free(def->val); @@ -3775,7 +3786,6 @@ free_cmndspecs(struct cmndspec_list *csl) void free_privilege(struct privilege *priv) { - struct member_list *prev_binding = NULL; struct defaults *def; debug_decl(free_privilege, SUDOERS_DEBUG_PARSER); @@ -3784,7 +3794,7 @@ free_privilege(struct privilege *priv) free_cmndspecs(&priv->cmndlist); while ((def = TAILQ_FIRST(&priv->defaults)) != NULL) { TAILQ_REMOVE(&priv->defaults, def, entries); - free_default(def, &prev_binding); + free_default(def); } free(priv); diff --git a/plugins/sudoers/gram.y b/plugins/sudoers/gram.y index 1dad0608c..fbc7d9caa 100644 --- a/plugins/sudoers/gram.y +++ b/plugins/sudoers/gram.y @@ -1265,7 +1265,7 @@ new_default(char *var, char *val, short op) d->val = val; /* d->type = 0; */ d->op = op; - /* d->binding = NULL */ + /* d->binding = NULL; */ d->line = this_lineno; d->column = sudolinebuf.toke_start + 1; d->file = sudo_rcstr_addref(sudoers); @@ -1338,6 +1338,22 @@ new_digest(int digest_type, char *digest_str) debug_return_ptr(digest); } +static void +free_defaults_binding(struct defaults_binding *binding) +{ + debug_decl(free_defaults_binding, SUDOERS_DEBUG_PARSER); + + /* Bindings may be shared among multiple Defaults entries. */ + if (binding != NULL) { + if (--binding->refcnt == 0) { + free_members(&binding->members); + free(binding); + } + } + + debug_return; +} + /* * Add a list of defaults structures to the defaults list. * The binding, if non-NULL, specifies a list of hosts, users, or @@ -1347,7 +1363,7 @@ static bool add_defaults(int type, struct member *bmem, struct defaults *defs) { struct defaults *d, *next; - struct member_list *binding; + struct defaults_binding *binding; bool ret = true; debug_decl(add_defaults, SUDOERS_DEBUG_PARSER); @@ -1363,10 +1379,11 @@ add_defaults(int type, struct member *bmem, struct defaults *defs) } if (bmem != NULL) { parser_leak_remove(LEAK_MEMBER, bmem); - HLTQ_TO_TAILQ(binding, bmem, entries); + HLTQ_TO_TAILQ(&binding->members, bmem, entries); } else { - TAILQ_INIT(binding); + TAILQ_INIT(&binding->members); } + binding->refcnt = 0; /* * Set type and binding (who it applies to) for new entries. @@ -1376,6 +1393,7 @@ add_defaults(int type, struct member *bmem, struct defaults *defs) HLTQ_FOREACH_SAFE(d, defs, entries, next) { d->type = type; d->binding = binding; + binding->refcnt++; TAILQ_INSERT_TAIL(&parsed_policy.defaults, d, entries); } } @@ -1456,30 +1474,23 @@ free_members(struct member_list *members) void free_defaults(struct defaults_list *defs) { - struct member_list *prev_binding = NULL; struct defaults *def; debug_decl(free_defaults, SUDOERS_DEBUG_PARSER); while ((def = TAILQ_FIRST(defs)) != NULL) { TAILQ_REMOVE(defs, def, entries); - free_default(def, &prev_binding); + free_default(def); } debug_return; } void -free_default(struct defaults *def, struct member_list **binding) +free_default(struct defaults *def) { debug_decl(free_default, SUDOERS_DEBUG_PARSER); - if (def->binding != *binding) { - *binding = def->binding; - if (def->binding != NULL) { - free_members(def->binding); - free(def->binding); - } - } + free_defaults_binding(def->binding); sudo_rcstr_delref(def->file); free(def->var); free(def->val); @@ -1619,7 +1630,6 @@ free_cmndspecs(struct cmndspec_list *csl) void free_privilege(struct privilege *priv) { - struct member_list *prev_binding = NULL; struct defaults *def; debug_decl(free_privilege, SUDOERS_DEBUG_PARSER); @@ -1628,7 +1638,7 @@ free_privilege(struct privilege *priv) free_cmndspecs(&priv->cmndlist); while ((def = TAILQ_FIRST(&priv->defaults)) != NULL) { TAILQ_REMOVE(&priv->defaults, def, entries); - free_default(def, &prev_binding); + free_default(def); } free(priv); diff --git a/plugins/sudoers/parse.c b/plugins/sudoers/parse.c index 70ca7da07..ff305e88f 100644 --- a/plugins/sudoers/parse.c +++ b/plugins/sudoers/parse.c @@ -695,11 +695,11 @@ display_defaults(struct sudoers_parse_tree *parse_tree, struct passwd *pw, TAILQ_FOREACH(d, &parse_tree->defaults, entries) { switch (d->type) { case DEFAULTS_HOST: - if (hostlist_matches(parse_tree, pw, d->binding) != ALLOW) + if (hostlist_matches(parse_tree, pw, &d->binding->members) != ALLOW) continue; break; case DEFAULTS_USER: - if (userlist_matches(parse_tree, pw, d->binding) != ALLOW) + if (userlist_matches(parse_tree, pw, &d->binding->members) != ALLOW) continue; break; case DEFAULTS_RUNAS: @@ -724,7 +724,7 @@ display_bound_defaults_by_type(struct sudoers_parse_tree *parse_tree, int deftype, struct sudo_lbuf *lbuf) { struct defaults *d; - struct member_list *binding = NULL; + struct defaults_binding *binding = NULL; struct member *m; char *dsep; int atype, nfound = 0; @@ -760,8 +760,8 @@ display_bound_defaults_by_type(struct sudoers_parse_tree *parse_tree, if (nfound != 1) sudo_lbuf_append(lbuf, "\n"); sudo_lbuf_append(lbuf, " Defaults%s", dsep); - TAILQ_FOREACH(m, binding, entries) { - if (m != TAILQ_FIRST(binding)) + TAILQ_FOREACH(m, &binding->members, entries) { + if (m != TAILQ_FIRST(&binding->members)) sudo_lbuf_append(lbuf, ", "); sudoers_format_member(lbuf, parse_tree, m, ", ", atype); } diff --git a/plugins/sudoers/parse.h b/plugins/sudoers/parse.h index 549b6d04d..fc26ff2f1 100644 --- a/plugins/sudoers/parse.h +++ b/plugins/sudoers/parse.h @@ -251,6 +251,11 @@ struct runascontainer { struct member *runasgroups; }; +struct defaults_binding { + struct member_list members; + unsigned int refcnt; +}; + struct sudoers_comment { STAILQ_ENTRY(sudoers_comment) entries; char *str; @@ -277,7 +282,7 @@ struct defaults { TAILQ_ENTRY(defaults) entries; char *var; /* variable name */ char *val; /* variable value */ - struct member_list *binding; /* user/host/runas binding */ + struct defaults_binding *binding; /* user/host/runas binding */ char *file; /* file Defaults entry was in */ short type; /* DEFAULTS{,_USER,_RUNAS,_HOST} */ char op; /* true, false, '+', '-' */ @@ -364,7 +369,7 @@ void free_cmndspecs(struct cmndspec_list *csl); void free_privilege(struct privilege *priv); void free_userspec(struct userspec *us); void free_userspecs(struct userspec_list *usl); -void free_default(struct defaults *def, struct member_list **binding); +void free_default(struct defaults *def); void free_defaults(struct defaults_list *defs); void init_parse_tree(struct sudoers_parse_tree *parse_tree, char *lhost, char *shost); void free_parse_tree(struct sudoers_parse_tree *parse_tree); diff --git a/plugins/sudoers/parse_ldif.c b/plugins/sudoers/parse_ldif.c index 40d10cdfc..debdbf32e 100644 --- a/plugins/sudoers/parse_ldif.c +++ b/plugins/sudoers/parse_ldif.c @@ -252,7 +252,8 @@ ldif_store_options(struct sudoers_parse_tree *parse_tree, sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory")); } - TAILQ_INIT(d->binding); + TAILQ_INIT(&d->binding->members); + d->binding->refcnt = 1; d->type = DEFAULTS; d->op = sudo_ldap_parse_option(ls->str, &var, &val); if ((d->var = strdup(var)) == NULL) {