From 2c21663b2243359dfd985fbe61937f2fadec2a7f Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Sat, 23 Jul 2016 09:10:48 -0600 Subject: [PATCH] Split set_default_entry() out of set_default() so we can call it from check_defaults() to validate the defaults value. In visudo, suppress warnings from update_defaults() and rely on check_defaults() to provide warnings. --- plugins/sudoers/defaults.c | 149 +++++++++++++++++++++---------------- plugins/sudoers/visudo.c | 16 ++-- 2 files changed, 90 insertions(+), 75 deletions(-) diff --git a/plugins/sudoers/defaults.c b/plugins/sudoers/defaults.c index ea199baad..d0f8c114f 100644 --- a/plugins/sudoers/defaults.c +++ b/plugins/sudoers/defaults.c @@ -197,51 +197,32 @@ dump_defaults(void) debug_return; } -/* - * Sets/clears an entry in the defaults structure - * If a variable that takes a value is used in a boolean - * context with op == 0, disable that variable. - * Eg. you may want to turn off logging to a file for some hosts. - * This is only meaningful for variables that are *optional*. - */ -bool -set_default(const char *var, const char *val, int op, bool quiet) +static bool +set_default_entry(struct sudo_defs_types *def, const char *val, int op, bool quiet) { - struct sudo_defs_types *cur; - int num; - debug_decl(set_default, SUDOERS_DEBUG_DEFAULTS) + debug_decl(set_default_entry, SUDOERS_DEBUG_DEFAULTS) - for (cur = sudo_defs_table, num = 0; cur->name; cur++, num++) { - if (strcmp(var, cur->name) == 0) - break; - } - if (!cur->name) { - if (!quiet) - sudo_warnx(U_("unknown defaults entry `%s'"), var); - debug_return_bool(false); - } - - switch (cur->type & T_MASK) { + switch (def->type & T_MASK) { case T_LOGFAC: - if (!store_syslogfac(val, cur, op)) { + if (!store_syslogfac(val, def, op)) { if (!quiet) { if (val) sudo_warnx(U_("value `%s' is invalid for option `%s'"), - val, var); + val, def->name); else - sudo_warnx(U_("no value specified for `%s'"), var); + sudo_warnx(U_("no value specified for `%s'"), def->name); } debug_return_bool(false); } break; case T_LOGPRI: - if (!store_syslogpri(val, cur, op)) { + if (!store_syslogpri(val, def, op)) { if (!quiet) { if (val) sudo_warnx(U_("value `%s' is invalid for option `%s'"), - val, var); + val, def->name); else - sudo_warnx(U_("no value specified for `%s'"), var); + sudo_warnx(U_("no value specified for `%s'"), def->name); } debug_return_bool(false); } @@ -249,25 +230,27 @@ set_default(const char *var, const char *val, int op, bool quiet) case T_STR: if (!val) { /* Check for bogus boolean usage or lack of a value. */ - if (!ISSET(cur->type, T_BOOL) || op != false) { + if (!ISSET(def->type, T_BOOL) || op != false) { if (!quiet) - sudo_warnx(U_("no value specified for `%s'"), var); + sudo_warnx(U_("no value specified for `%s'"), def->name); debug_return_bool(false); } } - if (ISSET(cur->type, T_PATH) && val && *val != '/') { - if (!quiet) - sudo_warnx(U_("values for `%s' must start with a '/'"), var); + if (ISSET(def->type, T_PATH) && val && *val != '/') { + if (!quiet) { + sudo_warnx(U_("values for `%s' must start with a '/'"), + def->name); + } debug_return_bool(false); } - switch (store_str(val, cur, op)) { + switch (store_str(val, def, op)) { case true: /* OK */ break; case false: if (!quiet) { sudo_warnx(U_("value `%s' is invalid for option `%s'"), - val, var); + val, def->name); } /* FALLTHROUGH */ default: @@ -277,16 +260,16 @@ set_default(const char *var, const char *val, int op, bool quiet) case T_INT: if (!val) { /* Check for bogus boolean usage or lack of a value. */ - if (!ISSET(cur->type, T_BOOL) || op != false) { + if (!ISSET(def->type, T_BOOL) || op != false) { if (!quiet) - sudo_warnx(U_("no value specified for `%s'"), var); + sudo_warnx(U_("no value specified for `%s'"), def->name); debug_return_bool(false); } } - if (!store_int(val, cur, op)) { + if (!store_int(val, def, op)) { if (!quiet) { sudo_warnx(U_("value `%s' is invalid for option `%s'"), - val, var); + val, def->name); } debug_return_bool(false); } @@ -294,16 +277,16 @@ set_default(const char *var, const char *val, int op, bool quiet) case T_UINT: if (!val) { /* Check for bogus boolean usage or lack of a value. */ - if (!ISSET(cur->type, T_BOOL) || op != false) { + if (!ISSET(def->type, T_BOOL) || op != false) { if (!quiet) - sudo_warnx(U_("no value specified for `%s'"), var); + sudo_warnx(U_("no value specified for `%s'"), def->name); debug_return_bool(false); } } - if (!store_uint(val, cur, op)) { + if (!store_uint(val, def, op)) { if (!quiet) { sudo_warnx(U_("value `%s' is invalid for option `%s'"), - val, var); + val, def->name); } debug_return_bool(false); } @@ -311,16 +294,16 @@ set_default(const char *var, const char *val, int op, bool quiet) case T_FLOAT: if (!val) { /* Check for bogus boolean usage or lack of a value. */ - if (!ISSET(cur->type, T_BOOL) || op != false) { + if (!ISSET(def->type, T_BOOL) || op != false) { if (!quiet) - sudo_warnx(U_("no value specified for `%s'"), var); + sudo_warnx(U_("no value specified for `%s'"), def->name); debug_return_bool(false); } } - if (!store_float(val, cur, op)) { + if (!store_float(val, def, op)) { if (!quiet) { sudo_warnx(U_("value `%s' is invalid for option `%s'"), - val, var); + val, def->name); } debug_return_bool(false); } @@ -328,57 +311,59 @@ set_default(const char *var, const char *val, int op, bool quiet) case T_MODE: if (!val) { /* Check for bogus boolean usage or lack of a value. */ - if (!ISSET(cur->type, T_BOOL) || op != false) { + if (!ISSET(def->type, T_BOOL) || op != false) { if (!quiet) - sudo_warnx(U_("no value specified for `%s'"), var); + sudo_warnx(U_("no value specified for `%s'"), def->name); debug_return_bool(false); } } - if (!store_mode(val, cur, op)) { + if (!store_mode(val, def, op)) { if (!quiet) { sudo_warnx(U_("value `%s' is invalid for option `%s'"), - val, var); + val, def->name); } debug_return_bool(false); } break; case T_FLAG: if (val) { - if (!quiet) - sudo_warnx(U_("option `%s' does not take a value"), var); + if (!quiet) { + sudo_warnx(U_("option `%s' does not take a value"), + def->name); + } debug_return_bool(false); } - cur->sd_un.flag = op; - if (cur->callback) - debug_return_bool(cur->callback(&cur->sd_un)); + def->sd_un.flag = op; + if (def->callback) + debug_return_bool(def->callback(&def->sd_un)); break; case T_LIST: if (!val) { /* Check for bogus boolean usage or lack of a value. */ - if (!ISSET(cur->type, T_BOOL) || op != false) { + if (!ISSET(def->type, T_BOOL) || op != false) { if (!quiet) - sudo_warnx(U_("no value specified for `%s'"), var); + sudo_warnx(U_("no value specified for `%s'"), def->name); debug_return_bool(false); } } - if (!store_list(val, cur, op)) { + if (!store_list(val, def, op)) { if (!quiet) { sudo_warnx(U_("value `%s' is invalid for option `%s'"), - val, var); + val, def->name); } debug_return_bool(false); } break; case T_TUPLE: - if (!val && !ISSET(cur->type, T_BOOL)) { + if (!val && !ISSET(def->type, T_BOOL)) { if (!quiet) - sudo_warnx(U_("no value specified for `%s'"), var); + sudo_warnx(U_("no value specified for `%s'"), def->name); debug_return_bool(false); } - if (!store_tuple(val, cur, op)) { + if (!store_tuple(val, def, op)) { if (!quiet) { sudo_warnx(U_("value `%s' is invalid for option `%s'"), - val, var); + val, def->name); } debug_return_bool(false); } @@ -388,6 +373,33 @@ set_default(const char *var, const char *val, int op, bool quiet) debug_return_bool(true); } +/* + * Sets/clears an entry in the defaults structure + * If a variable that takes a value is used in a boolean + * context with op == 0, disable that variable. + * Eg. you may want to turn off logging to a file for some hosts. + * This is only meaningful for variables that are *optional*. + */ +bool +set_default(const char *var, const char *val, int op, bool quiet) +{ + struct sudo_defs_types *cur; + int num; + debug_decl(set_default, SUDOERS_DEBUG_DEFAULTS) + + for (cur = sudo_defs_table, num = 0; cur->name; cur++, num++) { + if (strcmp(var, cur->name) == 0) + break; + } + if (!cur->name) { + if (!quiet) + sudo_warnx(U_("unknown defaults entry `%s'"), var); + debug_return_bool(false); + } + + debug_return_bool(set_default_entry(cur, val, op, quiet)); +} + /* * Set default options to compiled-in values. * Any of these may be overridden at runtime by a "Defaults" file. @@ -712,7 +724,7 @@ update_defaults(int what, bool quiet) bool check_defaults(int what, bool quiet) { - struct sudo_defs_types *cur; + struct sudo_defs_types *cur, tmp; struct defaults *def; bool rc = true; debug_decl(check_defaults, SUDOERS_DEBUG_DEFAULTS) @@ -729,6 +741,11 @@ check_defaults(int what, bool quiet) sudo_warnx(U_("unknown defaults entry `%s'"), def->var); rc = false; } + /* Don't actually set the defaults value, just checking. */ + tmp = *cur; + tmp.callback = NULL; + if (!set_default_entry(&tmp, def->val, def->op, quiet)) + rc = false; } debug_return_bool(rc); } diff --git a/plugins/sudoers/visudo.c b/plugins/sudoers/visudo.c index a6c3b97f0..d02f73c70 100644 --- a/plugins/sudoers/visudo.c +++ b/plugins/sudoers/visudo.c @@ -554,7 +554,6 @@ reparse_sudoers(char *editor, int editor_argc, char **editor_argv, struct sudoersfile *sp, *last; FILE *fp; int ch, oldlocale; - bool ok; debug_decl(reparse_sudoers, SUDOERS_DEBUG_UTIL) /* @@ -581,17 +580,16 @@ reparse_sudoers(char *editor, int editor_argc, char **editor_argv, parse_error = true; errorfile = sp->path; } - ok = update_defaults(SETDEF_GENERIC|SETDEF_HOST, quiet); - if (!check_defaults(SETDEF_ALL & ~(SETDEF_GENERIC|SETDEF_HOST), quiet)) - ok = false; - sudoers_setlocale(oldlocale, NULL); fclose(sudoersin); if (!parse_error) { - if (!ok || check_aliases(strict, quiet) != 0) { + (void) update_defaults(SETDEF_GENERIC|SETDEF_HOST, true); + if (!check_defaults(SETDEF_ALL, quiet) || + check_aliases(strict, quiet) != 0) { parse_error = true; errorfile = NULL; } } + sudoers_setlocale(oldlocale, NULL); /* * Got an error, prompt the user for what to do now. @@ -923,9 +921,9 @@ check_syntax(const char *sudoers_file, bool quiet, bool strict, bool oldperms) errorfile = sudoers_file; } if (!parse_error) { - if (!update_defaults(SETDEF_GENERIC|SETDEF_HOST, quiet) || - !check_defaults(SETDEF_ALL & ~(SETDEF_GENERIC|SETDEF_HOST), quiet) - || check_aliases(strict, quiet) != 0) { + (void) update_defaults(SETDEF_GENERIC|SETDEF_HOST, true); + if (!check_defaults(SETDEF_ALL, quiet) || + check_aliases(strict, quiet) != 0) { parse_error = true; errorfile = NULL; } -- 2.40.0