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.

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);
 }
 

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);

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;

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);
 }
 
 /*

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 */