Fix use-after-free in check_defaults(), reported by Radovan Sroka

of RedHat.

diff --git a/MANIFEST b/MANIFEST
index 9d9c5278bfd5479b6b7a28647ea8b8e258836c30..37f7a86e0c2984a106d6bab95f792c10cdbef3dd 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -462,6 +462,8 @@ plugins/sudoers/regress/visudo/test4.out.ok
 plugins/sudoers/regress/visudo/test4.sh
 plugins/sudoers/regress/visudo/test5.out.ok
 plugins/sudoers/regress/visudo/test5.sh
+plugins/sudoers/regress/visudo/test6.out.ok
+plugins/sudoers/regress/visudo/test6.sh
 plugins/sudoers/set_perms.c
 plugins/sudoers/solaris_audit.c
 plugins/sudoers/solaris_audit.h

diff --git a/plugins/sudoers/defaults.c b/plugins/sudoers/defaults.c
index 1b562941da265c1f2e11e951240b2d96f4e82890..052682145aaf58e6ee615d44dd1a25057f027277 100644 (file)
--- a/plugins/sudoers/defaults.c
+++ b/plugins/sudoers/defaults.c
@@ -378,6 +378,20 @@ run_early_defaults(void)
     debug_return_bool(rc);
 }
 
+static void
+free_default(struct sudo_defs_types *def)
+{
+    switch (def->type & T_MASK) {
+       case T_STR:
+           free(def->sd_un.str);
+           break;
+       case T_LIST:
+           (void)list_op(NULL, 0, def, freeall);
+           break;
+    }
+    memset(&def->sd_un, 0, sizeof(def->sd_un));
+}
+
 /*
  * Set default options to compiled-in values.
  * Any of these may be overridden at runtime by a "Defaults" file.
@@ -391,18 +405,8 @@ init_defaults(void)
 
     /* Clear any old settings. */
     if (!firsttime) {
-       for (def = sudo_defs_table; def->name; def++) {
-           switch (def->type & T_MASK) {
-               case T_STR:
-                   free(def->sd_un.str);
-                   def->sd_un.str = NULL;
-                   break;
-               case T_LIST:
-                   (void)list_op(NULL, 0, def, freeall);
-                   break;
-           }
-           memset(&def->sd_un, 0, sizeof(def->sd_un));
-       }
+       for (def = sudo_defs_table; def->name != NULL; def++)
+           free_default(def);
     }
 
     /* First initialize the flags. */
@@ -714,8 +718,10 @@ check_defaults(int what, bool quiet)
        }
        /* Don't actually set the defaults value, just checking. */
        tmp = *cur;
+       memset(&tmp.sd_un, 0, sizeof(&tmp.sd_un));
        if (!set_default_entry(&tmp, def->val, def->op, quiet, false))
            rc = false;
+       free_default(&tmp);
     }
     debug_return_bool(rc);
 }