>Number:         17053
>Category:       kern
>Synopsis:       patch to add O_NOFOLLOW to open(2)
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sun May 26 14:19:00 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator:     xs@kittenz.org
>Release:        NetBSD 1.6A (from 26th May)
>Organization:
>Environment:
/sys/kern/vfs_vnops.c:
     $NetBSD: vfs_vnops.c,v 1.54 2002/03/17 19:41:08 atatat Exp $
/sys/sys/fcntl.h:
     $NetBSD: fcntl.h,v 1.20 2001/12/07 07:09:30 jdolecek Exp $
/sys/kern/kern_sig.c:
     $NetBSD: kern_sig.c,v 1.120 2002/03/08 20:48:40 thorpej Exp $
System: NetBSD stasis 1.6A NetBSD 1.6A (STASIS) #2: Sun May 26 21:15:02 BST 2002 xs@stasis:/usr/src/sys/arch/i386/compile/STASIS i386
Architecture: i386
Machine: i386
>Description:
	NetBSD's open(2) lacks an O_NOFOLLOW or similar flag when O_CREAT is
	not set. Without this flag it is not possible to open a file that
	should not be a symlink without a race condition under some
	circumstances. (Such as the file residing in an untrusted or world
	writable directory.)

>How-To-Repeat:
	Currently something like the following must be used:

	struct stat sb;
	char fname[];
	int fd;

	lstat(fname, &sb);
	if (S_ISLNK(sb.st_mode))
		err(1, "symlink!");
	/* race occurs here, because file can be replaced with symlink
	 * after check
	 */
	fd = open(fname, O_RDONLY, 0);
	fstat(fd, &sb);
	if (!S_ISREG(sb.st_mode))
		...;			/* too late. */

	Post patch:

	struct stat sb;
	char fname[];
	int fd;

	fd = open(fname, O_RDONLY|O_NOFOLLOW, 0);
	/* open fails if path is symlink */
	fstat(fd, &sb);
	if (!S_ISREG(sb.st_mode))
		...;
>Fix:

Index: sys/kern/vfs_vnops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/vfs_vnops.c,v
retrieving revision 1.54
diff -c -r1.54 vfs_vnops.c
*** vfs_vnops.c	2002/03/17 19:41:08	1.54
--- vfs_vnops.c	2002/05/26 21:00:55
***************
*** 91,97 ****
  		ndp->ni_cnd.cn_nameiop = CREATE;
  		ndp->ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF;
  		if ((fmode & O_EXCL) == 0 &&
! 		    ((fmode & FNOSYMLINK) == 0))
  			ndp->ni_cnd.cn_flags |= FOLLOW;
  		if ((error = namei(ndp)) != 0)
  			return (error);
--- 91,97 ----
  		ndp->ni_cnd.cn_nameiop = CREATE;
  		ndp->ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF;
  		if ((fmode & O_EXCL) == 0 &&
! 		    ((fmode & O_NOFOLLOW) == 0))
  			ndp->ni_cnd.cn_flags |= FOLLOW;
  		if ((error = namei(ndp)) != 0)
  			return (error);
***************
*** 120,137 ****
  				error = EEXIST;
  				goto bad;
  			}
- 			if (ndp->ni_vp->v_type == VLNK) {
- 				error = EFTYPE;
- 				goto bad;
- 			}
  			fmode &= ~O_CREAT;
  		}
  	} else {
  		ndp->ni_cnd.cn_nameiop = LOOKUP;
! 		ndp->ni_cnd.cn_flags = FOLLOW | LOCKLEAF;
  		if ((error = namei(ndp)) != 0)
  			return (error);
  		vp = ndp->ni_vp;
  	}
  	if (vp->v_type == VSOCK) {
  		error = EOPNOTSUPP;
--- 120,139 ----
  				error = EEXIST;
  				goto bad;
  			}
  			fmode &= ~O_CREAT;
  		}
  	} else {
  		ndp->ni_cnd.cn_nameiop = LOOKUP;
! 		ndp->ni_cnd.cn_flags = LOCKLEAF;
! 		if ((fmode & O_NOFOLLOW) == 0)
! 			ndp->ni_cnd.cn_flags |= FOLLOW;
  		if ((error = namei(ndp)) != 0)
  			return (error);
  		vp = ndp->ni_vp;
+ 	}
+ 	if (ndp->ni_vp->v_type == VLNK) {
+ 		error = EFTYPE;
+ 		goto bad;
  	}
  	if (vp->v_type == VSOCK) {
  		error = EOPNOTSUPP;
Index: sys/kern/kern_sig.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/kern_sig.c,v
retrieving revision 1.120
diff -c -r1.120 kern_sig.c
*** kern_sig.c	2002/03/08 20:48:40	1.120
--- kern_sig.c	2002/05/26 21:00:57
***************
*** 1368,1374 ****
  		return error;
  
  	NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name, p);
! 	error = vn_open(&nd, O_CREAT | FWRITE | FNOSYMLINK, S_IRUSR | S_IWUSR);
  	if (error)
  		return (error);
  	vp = nd.ni_vp;
--- 1368,1374 ----
  		return error;
  
  	NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name, p);
! 	error = vn_open(&nd, O_CREAT | FWRITE | O_NOFOLLOW, S_IRUSR | S_IWUSR);
  	if (error)
  		return (error);
  	vp = nd.ni_vp;
Index: sys/sys/fcntl.h
===================================================================
RCS file: /cvsroot/syssrc/sys/sys/fcntl.h,v
retrieving revision 1.20
diff -c -r1.20 fcntl.h
*** fcntl.h	2001/12/07 07:09:30	1.20
--- fcntl.h	2002/05/26 21:00:57
***************
*** 95,100 ****
--- 95,102 ----
      (_XOPEN_SOURCE - 0) >= 500
  #define	O_SYNC		0x00000080		/* synchronous writes */
  #endif
+ #define	O_NOFOLLOW	0x00000100		/* don't follow symlink for
+ 						   last component */
  #define	O_CREAT		0x00000200		/* create if nonexistent */
  #define	O_TRUNC		0x00000400		/* truncate to zero length */
  #define	O_EXCL		0x00000800		/* error if already exists */
***************
*** 119,137 ****
  
  /* all bits settable during open(2) */
  #define	O_MASK		(O_ACCMODE|O_NONBLOCK|O_APPEND|O_SHLOCK|O_EXLOCK|\
! 			 O_ASYNC|O_SYNC|O_CREAT|O_TRUNC|O_EXCL|O_DSYNC|\
! 			 O_RSYNC|O_NOCTTY|O_ALT_IO)
  
  #define	FMARK		0x00001000	/* mark during gc() */
  #define	FDEFER		0x00002000	/* defer for next gc pass */
  #define	FHASLOCK	0x00004000	/* descriptor holds advisory lock */
- /*
-  * Note: The below is not a flag that can be used in the struct file. 
-  * It's an option that can be passed to vn_open to make sure it doesn't
-  * follow a symlink on the last lookup
-  */
- #define	FNOSYMLINK	0x00080000	/* Don't follow symlink for last
- 					   component */
  /* bits to save after open(2) */
  #define	FMASK		(FREAD|FWRITE|FAPPEND|FASYNC|FFSYNC|FNONBLOCK|FDSYNC|\
  			 FRSYNC|FALTIO)
--- 121,132 ----
  
  /* all bits settable during open(2) */
  #define	O_MASK		(O_ACCMODE|O_NONBLOCK|O_APPEND|O_SHLOCK|O_EXLOCK|\
! 			 O_ASYNC|O_SYNC|O_NOFOLLOW|O_CREAT|O_TRUNC|O_EXCL|\
! 			 O_DSYNC|O_RSYNC|O_NOCTTY|O_ALT_IO)
  
  #define	FMARK		0x00001000	/* mark during gc() */
  #define	FDEFER		0x00002000	/* defer for next gc pass */
  #define	FHASLOCK	0x00004000	/* descriptor holds advisory lock */
  /* bits to save after open(2) */
  #define	FMASK		(FREAD|FWRITE|FAPPEND|FASYNC|FFSYNC|FNONBLOCK|FDSYNC|\
  			 FRSYNC|FALTIO)
Index: lib/libc/sys/open.2
===================================================================
RCS file: /cvsroot/basesrc/lib/libc/sys/open.2,v
retrieving revision 1.22
diff -c -r1.22 open.2
*** open.2	2002/02/08 01:28:20	1.22
--- open.2	2002/05/26 21:00:58
***************
*** 103,108 ****
--- 103,114 ----
  I/O file integrity completion:
  each write will wait for both the file data and file status to be
  committed to stable storage.
+ .It Dv O_NOFOLLOW
+ If the last component of
+ .Fa path
+ is a symbolic link,
+ .Fn open
+ will fail.
  .It Dv O_RSYNC
  If set, read operations will complete at the same level of
  integrity which is in effect for write operations:
***************
*** 218,223 ****
--- 224,235 ----
  does not permit writing.
  .It Bq Er ELOOP
  Too many symbolic links were encountered in translating the pathname.
+ .It Bq Er EFTYPE
+ If
+ .Dv O_NOFOLLOW
+ was set, then the last component of
+ .Fa path
+ was a symbolic link.
  .It Bq Er EISDIR
  The named file is a directory, and the arguments specify
  it is to be opened for writing.
>Release-Note:
>Audit-Trail:
>Unformatted: