[sudo-workers] Re: sudo 1.6.8rc2 still has NFS/root bug

Todd C. Miller Todd.Miller at courtesan.com
Wed Jul 21 16:34:00 EDT 2004


In message <20040721180141.GA3799 at turtle.science-computing.de>
	so spake Harald Koenig (koenig):

> IMHO (if I'm not totally wrong) this desgin is _wrong_! 
> all access tests to the command path should be done 
> _always_and_only_ with PERM_RUNAS!
> 
> if runas_pw->pw_uid is 0 (root), nothing changes, and
> it it's non-zero/root, then there is no reason at all
> to run those tests with PERM_ROOT first, no matter if the
> command path is on a local fs or on NFS etc.

Yes, I think this makes sense.  The following patch uses PERM_RUNAS
while parsing sudoers.

Index: parse.c
===================================================================
RCS file: /home/cvs/courtesan/sudo/parse.c,v
retrieving revision 1.157
diff -u -r1.157 parse.c
--- parse.c	8 Jul 2004 00:15:37 -0000	1.157
+++ parse.c	21 Jul 2004 20:31:58 -0000
@@ -109,9 +109,6 @@
     int error, nopass;
     enum def_tupple pwcheck;
 
-    /* Become sudoers file owner */
-    set_perms(PERM_SUDOERS);
-
     /* We opened _PATH_SUDOERS in check_sudoers() so just rewind it. */
     rewind(sudoers_fp);
     yyin = sudoers_fp;
@@ -124,16 +121,18 @@
     if (pwflag > 0)
 	keepall = TRUE;
 
-    /* Need to be root while stat'ing things in the parser. */
-    set_perms(PERM_ROOT);
+    /* Need to be runas user while stat'ing things in the parser. */
+    set_perms(PERM_RUNAS);
     error = yyparse();
 
     /* Close the sudoers file now that we are done with it. */
     (void) fclose(sudoers_fp);
     sudoers_fp = NULL;
 
-    if (error || parse_error)
+    if (error || parse_error) {
+	set_perms(PERM_ROOT);
 	return(VALIDATE_ERROR);
+    }
 
     /*
      * The pw options may have changed during sudoers parse so we
@@ -185,6 +184,7 @@
 	    top--;
 	}
 	if (found) {
+	    set_perms(PERM_ROOT);
 	    if (nopass == -1)
 		nopass = 0;
 	    return(VALIDATE_OK | nopass);
@@ -197,6 +197,7 @@
 		    /*
 		     * User was granted access to cmnd on host as user.
 		     */
+		    set_perms(PERM_ROOT);
 		    return(VALIDATE_OK |
 			(no_passwd == TRUE ? FLAG_NOPASS : 0) |
 			(no_execve == TRUE ? FLAG_NOEXEC : 0));
@@ -205,6 +206,7 @@
 		    /*
 		     * User was explicitly denied access to cmnd on host.
 		     */
+		    set_perms(PERM_ROOT);
 		    return(VALIDATE_NOT_OK |
 			(no_passwd == TRUE ? FLAG_NOPASS : 0) |
 			(no_execve == TRUE ? FLAG_NOEXEC : 0));
@@ -213,6 +215,7 @@
 	    top--;
 	}
     }
+    set_perms(PERM_ROOT);
 
     /*
      * The user was neither explicitly granted nor denied access.
@@ -233,7 +236,7 @@
     char *path;
     char *sudoers_args;
 {
-    int plen, error;
+    int plen;
     static struct stat cst;
     struct stat pst;
     DIR *dirp;
@@ -267,15 +270,8 @@
 
     /* Only need to stat cmnd once since it never changes */
     if (cst.st_dev == 0) {
-	if ((error = stat(cmnd, &cst))) {
-	    if (runas_pw->pw_uid != 0) {
-		set_perms(PERM_RUNAS);
-		error = stat(cmnd, &cst);
-		set_perms(PERM_ROOT);
-	    }
-	    if (error)
-		return(FALSE);
-	}
+	if (stat(cmnd, &cst) == -1)
+	    return(FALSE);
 	if ((cmnd_base = strrchr(cmnd, '/')) == NULL)
 	    cmnd_base = cmnd;
 	else



More information about the sudo-workers mailing list