Update access(2) to have modern semantics with respect to X_OK and

the superuser.  access(2) will now only indicate success for X_OK on
non-directories if there is at least one execute bit set on the file.
OK deraadt@ thib@ otto@

11 files changed, 52 insertions, 39 deletions

diff --git a/lib/libc/sys/access.2 b/lib/libc/sys/access.2
index b2217493c3c..701b0c923f7 100644
--- a/lib/libc/sys/access.2
+++ b/lib/libc/sys/access.2
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: access.2,v 1.12 2007/05/31 19:19:32 jmc Exp $
+.\"	$OpenBSD: access.2,v 1.13 2008/06/09 23:38:37 millert Exp $
 .\"	$NetBSD: access.2,v 1.7 1995/02/27 12:31:44 cgd Exp $
 .\"
 .\" Copyright (c) 1980, 1991, 1993
@@ -30,7 +30,7 @@
 .\"
 .\"     @(#)access.2	8.2 (Berkeley) 4/1/94
 .\"
-.Dd $Mdocdate: May 31 2007 $
+.Dd $Mdocdate: June 9 2008 $
 .Dt ACCESS 2
 .Os
 .Sh NAME
@@ -69,13 +69,20 @@ and the real group access list
 (including the real group ID) is
 used in place of the effective ID for verifying permission.
 .Pp
-Even if a process has appropriate privileges and indicates success for
-.Dv X_OK ,
-the file may not actually have execute permission bits set.
-Likewise for
+If the invoking process has superuser privileges,
+.Fn access
+will always indicate success for
 .Dv R_OK
 and
-.Dv W_OK .
+.Dv W_OK ,
+regardless of the actual file permission bits.
+Likewise, for
+.Dv X_OK ,
+if the file has any of the execute bits set and
+.Fa path
+is not a directory,
+.Fn
+will indicate success.
 .Sh RETURN VALUES
 If
 .Fa path
@@ -131,4 +138,5 @@ function conforms to
 .St -p1003.1-90 .
 .Sh CAVEATS
 .Fn access
-is a potential security hole and should never be used.
+should never be used for actual access control.
+Doing so can result in a time of check vs. time of use security hole.
diff --git a/sys/isofs/cd9660/cd9660_vnops.c b/sys/isofs/cd9660/cd9660_vnops.c
index 1e2ec604a39..4936f1e46ab 100644
--- a/sys/isofs/cd9660/cd9660_vnops.c
+++ b/sys/isofs/cd9660/cd9660_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cd9660_vnops.c,v 1.45 2008/05/08 17:45:45 thib Exp $	*/
+/*	$OpenBSD: cd9660_vnops.c,v 1.46 2008/06/09 23:38:37 millert Exp $	*/
 /*	$NetBSD: cd9660_vnops.c,v 1.42 1997/10/16 23:56:57 christos Exp $	*/
 
 /*-
@@ -215,8 +215,8 @@ cd9660_access(v)
 	struct vop_access_args *ap = v;
 	struct iso_node *ip = VTOI(ap->a_vp);
 
-	return (vaccess(ip->inode.iso_mode & ALLPERMS, ip->inode.iso_uid,
-	    ip->inode.iso_gid, ap->a_mode, ap->a_cred));
+	return (vaccess(ap->a_vp->v_type, ip->inode.iso_mode & ALLPERMS,
+	    ip->inode.iso_uid, ip->inode.iso_gid, ap->a_mode, ap->a_cred));
 }
 
 int
diff --git a/sys/isofs/udf/udf_vnops.c b/sys/isofs/udf/udf_vnops.c
index 1ab4c8eec18..5b096a7b8eb 100644
--- a/sys/isofs/udf/udf_vnops.c
+++ b/sys/isofs/udf/udf_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: udf_vnops.c,v 1.31 2008/05/08 17:45:45 thib Exp $	*/
+/*	$OpenBSD: udf_vnops.c,v 1.32 2008/06/09 23:38:37 millert Exp $	*/
 
 /*
  * Copyright (c) 2001, 2002 Scott Long <scottl@freebsd.org>
@@ -222,8 +222,8 @@ udf_access(void *v)
 
 	mode = udf_permtomode(up);
 
-	return (vaccess(mode, up->u_fentry->uid, up->u_fentry->gid, a_mode,
-	    ap->a_cred));
+	return (vaccess(vp->v_type, mode, up->u_fentry->uid, up->u_fentry->gid,
+	    a_mode, ap->a_cred));
 }
 
 static int mon_lens[2][12] = {
diff --git a/sys/kern/sysv_ipc.c b/sys/kern/sysv_ipc.c
index 80ad3822179..b0b46a1ea15 100644
--- a/sys/kern/sysv_ipc.c
+++ b/sys/kern/sysv_ipc.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: sysv_ipc.c,v 1.5 2005/12/13 10:33:14 jsg Exp $	*/
+/*	$OpenBSD: sysv_ipc.c,v 1.6 2008/06/09 23:38:37 millert Exp $	*/
 /*	$NetBSD: sysv_ipc.c,v 1.10 1995/06/03 05:53:28 mycroft Exp $	*/
 
 /*
@@ -54,8 +54,8 @@ ipcperm(struct ucred *cred, struct ipc_perm *perm, int mode)
 		return (EPERM);
 	}
 
-	if (vaccess(perm->mode, perm->uid, perm->gid, mode, cred) == 0 ||
-	    vaccess(perm->mode, perm->cuid, perm->cgid, mode, cred) == 0)
+	if (vaccess(VNON, perm->mode, perm->uid, perm->gid, mode, cred) == 0 ||
+	    vaccess(VNON, perm->mode, perm->cuid, perm->cgid, mode, cred) == 0)
 		return (0);
 	return (EACCES);
 }
diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c
index 93b4774dd5a..3ac320e5bc1 100644
--- a/sys/kern/vfs_subr.c
+++ b/sys/kern/vfs_subr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfs_subr.c,v 1.166 2008/05/07 14:08:37 thib Exp $	*/
+/*	$OpenBSD: vfs_subr.c,v 1.167 2008/06/09 23:38:37 millert Exp $	*/
 /*	$NetBSD: vfs_subr.c,v 1.53 1996/04/22 01:39:13 christos Exp $	*/
 
 /*
@@ -1520,14 +1520,19 @@ vfs_export_lookup(struct mount *mp, struct netexport *nep, struct mbuf *nam)
  * while acc_mode and cred are from the VOP_ACCESS parameter list
  */
 int
-vaccess(mode_t file_mode, uid_t uid, gid_t gid, mode_t acc_mode,
-    struct ucred *cred)
+vaccess(enum vtype type, mode_t file_mode, uid_t uid, gid_t gid,
+    mode_t acc_mode, struct ucred *cred)
 {
 	mode_t mask;
 
-	/* User id 0 always gets access. */
-	if (cred->cr_uid == 0)
+	/* User id 0 always gets read/write access. */
+	if (cred->cr_uid == 0) {
+		/* For VEXEC, at least one of the execute bits must be set. */
+		if ((acc_mode & VEXEC) && type != VDIR &&
+		    (file_mode & (S_IXUSR|S_IXGRP|S_IXOTH)) == 0)
+			return EACCES;
 		return 0;
+	}
 
 	mask = 0;
 
diff --git a/sys/miscfs/procfs/procfs_vnops.c b/sys/miscfs/procfs/procfs_vnops.c
index fed59a39e80..df7fa971424 100644
--- a/sys/miscfs/procfs/procfs_vnops.c
+++ b/sys/miscfs/procfs/procfs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: procfs_vnops.c,v 1.44 2008/05/09 10:22:50 thib Exp $	*/
+/*	$OpenBSD: procfs_vnops.c,v 1.45 2008/06/09 23:38:37 millert Exp $	*/
 /*	$NetBSD: procfs_vnops.c,v 1.40 1996/03/16 23:52:55 christos Exp $	*/
 
 /*
@@ -620,8 +620,8 @@ procfs_access(void *v)
 	if ((error = VOP_GETATTR(ap->a_vp, &va, ap->a_cred, ap->a_p)) != 0)
 		return (error);
 
-	return (vaccess(va.va_mode, va.va_uid, va.va_gid, ap->a_mode,
-			ap->a_cred));
+	return (vaccess(ap->a_vp->v_type, va.va_mode, va.va_uid, va.va_gid,
+			ap->a_mode, ap->a_cred));
 }
 
 /*
diff --git a/sys/msdosfs/msdosfs_vnops.c b/sys/msdosfs/msdosfs_vnops.c
index aefc39a3ff7..f56064ab3e1 100644
--- a/sys/msdosfs/msdosfs_vnops.c
+++ b/sys/msdosfs/msdosfs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: msdosfs_vnops.c,v 1.66 2008/05/08 17:45:45 thib Exp $	*/
+/*	$OpenBSD: msdosfs_vnops.c,v 1.67 2008/06/09 23:38:37 millert Exp $	*/
 /*	$NetBSD: msdosfs_vnops.c,v 1.63 1997/10/17 11:24:19 ws Exp $	*/
 
 /*-
@@ -230,8 +230,8 @@ msdosfs_access(v)
 		dosmode |= (dosmode & S_IROTH) ? S_IXOTH : 0;
 	}
 
-	return (vaccess(dosmode, pmp->pm_uid, pmp->pm_gid, ap->a_mode,
-	    ap->a_cred));
+	return (vaccess(ap->a_vp->v_type, dosmode, pmp->pm_uid, pmp->pm_gid,
+	    ap->a_mode, ap->a_cred));
 }
 
 int
diff --git a/sys/nfs/nfs_vnops.c b/sys/nfs/nfs_vnops.c
index 7a52ca76de9..be30de9f5f4 100644
--- a/sys/nfs/nfs_vnops.c
+++ b/sys/nfs/nfs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nfs_vnops.c,v 1.83 2008/05/08 17:45:45 thib Exp $	*/
+/*	$OpenBSD: nfs_vnops.c,v 1.84 2008/06/09 23:38:37 millert Exp $	*/
 /*	$NetBSD: nfs_vnops.c,v 1.62.4.1 1996/07/08 20:26:52 jtc Exp $	*/
 
 /*
@@ -3001,8 +3001,8 @@ nfsspec_access(v)
 	if (error)
 		return (error);
 
-	return (vaccess(va.va_mode, va.va_uid, va.va_gid, ap->a_mode,
-	    ap->a_cred));
+	return (vaccess(vp->v_type, va.va_mode, va.va_uid, va.va_gid,
+	    ap->a_mode, ap->a_cred));
 }
 
 /* ARGSUSED */
diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h
index b129d3e5f07..018aa166d95 100644
--- a/sys/sys/vnode.h
+++ b/sys/sys/vnode.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vnode.h,v 1.94 2008/05/08 17:45:45 thib Exp $	*/
+/*	$OpenBSD: vnode.h,v 1.95 2008/06/09 23:38:37 millert Exp $	*/
 /*	$NetBSD: vnode.h,v 1.38 1996/02/29 20:59:05 cgd Exp $	*/
 
 /*
@@ -348,7 +348,7 @@ int	cdevvp(dev_t, struct vnode **);
 struct vnode *checkalias(struct vnode *, dev_t, struct mount *);
 int	getnewvnode(enum vtagtype, struct mount *, int (**vops)(void *),
 	    struct vnode **);
-int	vaccess(mode_t, uid_t, gid_t, mode_t, struct ucred *);
+int	vaccess(enum vtype, mode_t, uid_t, gid_t, mode_t, struct ucred *);
 void	vattr_null(struct vattr *);
 void	vdevgone(int, int, int, enum vtype);
 int	vcount(struct vnode *);
diff --git a/sys/ufs/ext2fs/ext2fs_vnops.c b/sys/ufs/ext2fs/ext2fs_vnops.c
index 8a957ae11bc..9dc8ee6801c 100644
--- a/sys/ufs/ext2fs/ext2fs_vnops.c
+++ b/sys/ufs/ext2fs/ext2fs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ext2fs_vnops.c,v 1.50 2008/05/08 17:45:45 thib Exp $	*/
+/*	$OpenBSD: ext2fs_vnops.c,v 1.51 2008/06/09 23:38:37 millert Exp $	*/
 /*	$NetBSD: ext2fs_vnops.c,v 1.1 1997/06/11 09:34:09 bouyer Exp $	*/
 
 /*
@@ -155,8 +155,8 @@ ext2fs_access(void *v)
 	if ((mode & VWRITE) && (ip->i_e2fs_flags & EXT2_IMMUTABLE))
 		return (EPERM);
 
-	return (vaccess(ip->i_e2fs_mode, ip->i_e2fs_uid, ip->i_e2fs_gid, mode,
-			ap->a_cred));
+	return (vaccess(vp->v_type, ip->i_e2fs_mode, ip->i_e2fs_uid,
+			ip->i_e2fs_gid, mode, ap->a_cred));
 }
 
 /* ARGSUSED */
diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c
index ad7b67a2b47..50289377f04 100644
--- a/sys/ufs/ufs/ufs_vnops.c
+++ b/sys/ufs/ufs/ufs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ufs_vnops.c,v 1.86 2008/01/17 11:31:56 fgsch Exp $	*/
+/*	$OpenBSD: ufs_vnops.c,v 1.87 2008/06/09 23:38:37 millert Exp $	*/
 /*	$NetBSD: ufs_vnops.c,v 1.18 1996/05/11 18:28:04 mycroft Exp $	*/
 
 /*
@@ -252,8 +252,8 @@ ufs_access(void *v)
 	if ((mode & VWRITE) && (DIP(ip, flags) & IMMUTABLE))
 		return (EPERM);
 
-	return (vaccess(DIP(ip, mode), DIP(ip, uid), DIP(ip, gid), mode,
-	    ap->a_cred));
+	return (vaccess(vp->v_type, DIP(ip, mode), DIP(ip, uid), DIP(ip, gid),
+	    mode, ap->a_cred));
 }
 
 /* ARGSUSED */