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.

diff --git a/plugins/sudoers/defaults.c b/plugins/sudoers/defaults.c
index ea199baadec583c1409d710b6ddd0a350a408d66..d0f8c114f4f61b1bd924b1b0a3a3928b4ab2ed35 100644 (file)
--- 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 a6c3b97f0659f354bcd4111ff9baa2466a8b03be..d02f73c706bfabcd7ea1bbf9e53fd55e90b0c766 100644 (file)
--- 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;
        }