Instead of checking Defaults values after the fact, check them at

sudoers parse time.  This makes it possible to display the file and
line number with the problem and for visudo to go right to the
error.

diff --git a/plugins/sudoers/defaults.c b/plugins/sudoers/defaults.c
index 1d6564873f2f7895bd28a3cb5ba07063a2adf543..3faeff86037873ae11e5cd5107a5f98599c640c7 100644 (file)
--- a/plugins/sudoers/defaults.c
+++ b/plugins/sudoers/defaults.c
@@ -697,37 +697,31 @@ update_defaults(int what, bool quiet)
 }
 
 /*
- * Check the defaults entries without actually setting them.
- * Pass in an OR'd list of which default types to check.
+ * Check a defaults entry without actually setting it.
  */
 bool
-check_defaults(int what, bool quiet)
+check_default(struct defaults *def, bool quiet)
 {
-    struct sudo_defs_types *cur, tmp;
-    struct defaults *def;
+    struct sudo_defs_types *cur;
     bool ret = true;
-    debug_decl(check_defaults, SUDOERS_DEBUG_DEFAULTS)
+    debug_decl(check_default, SUDOERS_DEBUG_DEFAULTS)
 
-    TAILQ_FOREACH(def, &defaults, entries) {
-       if (!default_type_matches(def, what))
-           continue;
-       for (cur = sudo_defs_table; cur->name != NULL; cur++) {
-           if (strcmp(def->var, cur->name) == 0)
-               break;
-       }
-       if (cur->name == NULL) {
-           if (!quiet)
-               sudo_warnx(U_("unknown defaults entry `%s'"), def->var);
-           ret = false;
-       } else {
+    for (cur = sudo_defs_table; cur->name != NULL; cur++) {
+       if (strcmp(def->var, cur->name) == 0) {
            /* Don't actually set the defaults value, just checking. */
-           tmp = *cur;
+           struct sudo_defs_types tmp = *cur;
            memset(&tmp.sd_un, 0, sizeof(tmp.sd_un));
            if (!set_default_entry(&tmp, def->val, def->op, quiet, false))
                ret = false;
            free_default(&tmp);
+           break;
        }
     }
+    if (cur->name == NULL) {
+       if (!quiet)
+           sudo_warnx(U_("unknown defaults entry `%s'"), def->var);
+       ret = false;
+    }
     debug_return_bool(ret);
 }
 

diff --git a/plugins/sudoers/defaults.h b/plugins/sudoers/defaults.h
index f02fd3dd406eedbf7adbf13de7b1c9c64864a9fd..7a4567d6897c885ba5682b5bc92c89b06eb729d0 100644 (file)
--- a/plugins/sudoers/defaults.h
+++ b/plugins/sudoers/defaults.h
@@ -107,7 +107,7 @@ struct early_default {
 #define T_PATH         0x200
 
 /*
- * Argument to update_defaults() and check_defaults()
+ * Argument to update_defaults()
  */
 #define SETDEF_GENERIC 0x01
 #define        SETDEF_HOST     0x02
@@ -119,8 +119,10 @@ struct early_default {
 /*
  * Prototypes
  */
+struct defaults;       /* in parse.h */
+
 void dump_default(void);
-bool check_defaults(int what, bool quiet);
+bool check_default(struct defaults *def, bool quiet);
 bool init_defaults(void);
 struct early_default *is_early_default(const char *var);
 bool run_early_defaults(void);

diff --git a/plugins/sudoers/gram.c b/plugins/sudoers/gram.c
index c34f964326f1123eb403343f11c2410450a7d4b1..cbe69ea7f555a689e9b052566e88ae617f647683 100644 (file)
--- a/plugins/sudoers/gram.c
+++ b/plugins/sudoers/gram.c
@@ -799,8 +799,10 @@ new_digest(int digest_type, const char *digest_str)
 static bool
 add_defaults(int type, struct member *bmem, struct defaults *defs)
 {
-    struct defaults *d;
+    struct defaults *d, *next;
     struct member_list *binding;
+    bool binding_used = false;
+    bool ret = true;
     debug_decl(add_defaults, SUDOERS_DEBUG_PARSER)
 
     if (defs != NULL) {
@@ -820,16 +822,34 @@ add_defaults(int type, struct member *bmem, struct defaults *defs)
 
        /*
         * Set type and binding (who it applies to) for new entries.
-        * Then add to the global defaults list.
+        * Then add to the global defaults list if it parses.
         */
-       HLTQ_FOREACH(d, defs, entries) {
-           d->type = type;
-           d->binding = binding;
+       HLTQ_FOREACH_SAFE(d, defs, entries, next) {
+           if (check_default(d, !sudoers_warnings)) {
+               /* Append to defaults list */
+               d->type = type;
+               d->binding = binding;
+               binding_used = true;
+               TAILQ_INSERT_TAIL(&defaults, d, entries);
+           } else {
+               /* Did not parse, warn and free it. */
+               sudoerserror(N_("problem with defaults entries"));
+               free(d->var);
+               free(d->val);
+               free(d);
+               ret = false;    /* XXX - only an error for visudo */
+               continue;
+           }
+       }
+
+       if (!binding_used) {
+           /* No valid Defaults entries, binding unused. */
+           free_members(binding);
+           free(binding);
        }
-       TAILQ_CONCAT_HLTQ(&defaults, defs, entries);
     }
 
-    debug_return_bool(true);
+    debug_return_bool(ret);
 }
 
 /*
@@ -1003,7 +1023,7 @@ init_parser(const char *path, bool quiet)
 
     debug_return_bool(ret);
 }
-#line 954 "gram.c"
+#line 974 "gram.c"
 /* allocate initial stack or double stack size, up to YYMAXDEPTH */
 #if defined(__cplusplus) || defined(__STDC__)
 static int yygrowstack(void)
@@ -2086,7 +2106,7 @@ case 115:
                            }
                        }
 break;
-#line 2037 "gram.c"
+#line 2057 "gram.c"
     }
     yyssp -= yym;
     yystate = *yyssp;

diff --git a/plugins/sudoers/gram.y b/plugins/sudoers/gram.y
index 38bd6104a97831afb226fedce1b51e9d3d90c51f..fcf7cdc4c6e780bc47ad5374260f27c55bfc91a4 100644 (file)
--- a/plugins/sudoers/gram.y
+++ b/plugins/sudoers/gram.y
@@ -955,8 +955,10 @@ new_digest(int digest_type, const char *digest_str)
 static bool
 add_defaults(int type, struct member *bmem, struct defaults *defs)
 {
-    struct defaults *d;
+    struct defaults *d, *next;
     struct member_list *binding;
+    bool binding_used = false;
+    bool ret = true;
     debug_decl(add_defaults, SUDOERS_DEBUG_PARSER)
 
     if (defs != NULL) {
@@ -976,16 +978,34 @@ add_defaults(int type, struct member *bmem, struct defaults *defs)
 
        /*
         * Set type and binding (who it applies to) for new entries.
-        * Then add to the global defaults list.
+        * Then add to the global defaults list if it parses.
         */
-       HLTQ_FOREACH(d, defs, entries) {
-           d->type = type;
-           d->binding = binding;
+       HLTQ_FOREACH_SAFE(d, defs, entries, next) {
+           if (check_default(d, !sudoers_warnings)) {
+               /* Append to defaults list */
+               d->type = type;
+               d->binding = binding;
+               binding_used = true;
+               TAILQ_INSERT_TAIL(&defaults, d, entries);
+           } else {
+               /* Did not parse, warn and free it. */
+               sudoerserror(N_("problem with defaults entries"));
+               free(d->var);
+               free(d->val);
+               free(d);
+               ret = false;    /* XXX - only an error for visudo */
+               continue;
+           }
+       }
+
+       if (!binding_used) {
+           /* No valid Defaults entries, binding unused. */
+           free_members(binding);
+           free(binding);
        }
-       TAILQ_CONCAT_HLTQ(&defaults, defs, entries);
     }
 
-    debug_return_bool(true);
+    debug_return_bool(ret);
 }
 
 /*

diff --git a/plugins/sudoers/visudo.c b/plugins/sudoers/visudo.c
index 9cec2856f05100fcbcc5232640858f4247de3eb7..f641c00fe552a6f9b6bee1bd770a86f7609b3d7a 100644 (file)
--- a/plugins/sudoers/visudo.c
+++ b/plugins/sudoers/visudo.c
@@ -592,8 +592,7 @@ reparse_sudoers(char *editor, int editor_argc, char **editor_argv,
        fclose(sudoersin);
        if (!parse_error) {
            (void) update_defaults(SETDEF_GENERIC|SETDEF_HOST|SETDEF_USER, true);
-           if (!check_defaults(SETDEF_ALL, quiet) ||
-               check_aliases(strict, quiet) != 0) {
+           if (check_aliases(strict, quiet) != 0) {
                parse_error = true;
                free(errorfile);
                errorfile = NULL;       /* don't know which file */
@@ -937,8 +936,7 @@ check_syntax(const char *sudoers_file, bool quiet, bool strict, bool oldperms)
     }
     if (!parse_error) {
        (void) update_defaults(SETDEF_GENERIC|SETDEF_HOST|SETDEF_USER, true);
-       if (!check_defaults(SETDEF_ALL, quiet) ||
-           check_aliases(strict, quiet) != 0) {
+       if (check_aliases(strict, quiet) != 0) {
            parse_error = true;
            free(errorfile);
            errorfile = NULL;   /* don't know which file */