From: Todd C. Miller <Todd.Miller@courtesan.com>
Date: Tue, 1 Nov 2016 20:22:32 +0000 (-0600)
Subject: Instead of checking Defaults values after the fact, check them at
X-Git-Tag: SUDO_1_8_19^2~71
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8a48085184412932d2f8ead12b0e1ec1e0fd8027;p=sudo

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 1d6564873..3faeff860 100644
--- 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 f02fd3dd4..7a4567d68 100644
--- 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 c34f96432..cbe69ea7f 100644
--- 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 38bd6104a..fcf7cdc4c 100644
--- 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 9cec2856f..f641c00fe 100644
--- 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 */