From: Todd C. Miller <Todd.Miller@courtesan.com>
Date: Wed, 19 Jan 2000 19:07:24 +0000 (+0000)
Subject: Fix sudoers locking in visudo.  We now lock the sudoers file itself, not
X-Git-Tag: SUDO_1_6_2~7
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d9790399b3140d96843aa9f6260917bad66b83ca;p=sudo

Fix sudoers locking in visudo.  We now lock the sudoers file itself, not
the temp file (since locking the temp file can foul up editors).  The
previous locking scheme didn't work because the fd was closed too early.
---

diff --git a/CHANGES b/CHANGES
index 8b2383294..a3730f0d9 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1236,3 +1236,6 @@ Sudo 1.6.1 released.
 390) Systems that return RLIM_INFINITY for RLIMIT_NOFILE (like AIX)
      would loop for a very loing time during sudo startup.  A value of
      RLIM_INFINITY is now ignored (getdtablesize/sysconf is used instead).
+
+391) Locking in visudo was broken.  We now lock the sudoers file, not the
+     sudoers temp file, which should be safe.
diff --git a/config.h.in b/config.h.in
index 768ea731f..f6e5b15de 100644
--- a/config.h.in
+++ b/config.h.in
@@ -251,6 +251,9 @@
 /* Define if you have flock(2).  */
 #undef HAVE_FLOCK
 
+/* Define if you have fstat(2).  */
+#undef HAVE_FSTAT
+
 /* Define if you have setrlimit(2).  */
 #undef HAVE_SETRLIMIT
 
diff --git a/configure b/configure
index ebbc19e5e..876186a68 100755
--- a/configure
+++ b/configure
@@ -5413,7 +5413,7 @@ EOF
 
 ;;
 esac
-for ac_func in strchr strrchr memchr memcpy memset sysconf sigaction tzset seteuid strftime setrlimit initgroups
+for ac_func in strchr strrchr memchr memcpy memset sysconf sigaction tzset seteuid strftime setrlimit initgroups fstat
 do
 echo $ac_n "checking for $ac_func""... $ac_c" 1>&6
 echo "configure:5420: checking for $ac_func" >&5
diff --git a/configure.in b/configure.in
index 5379d08ee..35b5eb908 100644
--- a/configure.in
+++ b/configure.in
@@ -1343,7 +1343,7 @@ esac
 dnl
 dnl Function checks
 dnl
-AC_CHECK_FUNCS(strchr strrchr memchr memcpy memset sysconf sigaction tzset seteuid strftime setrlimit initgroups)
+AC_CHECK_FUNCS(strchr strrchr memchr memcpy memset sysconf sigaction tzset seteuid strftime setrlimit initgroups fstat)
 if test -n "$SECUREWARE"; then
     AC_CHECK_FUNCS(bigcrypt)
     AC_CHECK_FUNCS(set_auth_parameters)
diff --git a/visudo.c b/visudo.c
index 68f69957d..f972e3c5c 100644
--- a/visudo.c
+++ b/visudo.c
@@ -169,44 +169,44 @@ main(argc, argv)
 #endif /* ENV_EDITOR */
 
     /*
-     * Open sudoers temp file and grab a lock.
+     * Open sudoers, lock it and stat it.  
+     * sudoers_fd must remain open throughout in order to hold the lock.
      */
-    stmp_fd = open(stmp, O_WRONLY | O_CREAT, 0600);
-    if (stmp_fd < 0) {
-	(void) fprintf(stderr, "%s: %s\n", Argv[0], strerror(errno));
-	exit(1);
+    sudoers_fd = open(sudoers, O_RDWR | O_CREAT);
+    if (sudoers_fd == -1) {
+	(void) fprintf(stderr, "%s: %s: %s\n", Argv[0], sudoers,
+	    strerror(errno));
+	Exit(-1);
     }
-    if (!lock_file(stmp_fd, SUDO_TLOCK)) {
+    if (!lock_file(sudoers_fd, SUDO_TLOCK)) {
 	(void) fprintf(stderr, "%s: sudoers file busy, try again later.\n",
 	    Argv[0]);
 	exit(1);
     }
-#ifdef HAVE_FTRUNCATE
-    if (ftruncate(stmp_fd, 0) == -1) {
+#ifdef HAVE_FSTAT
+    if (fstat(sudoers_fd, &sudoers_sb) == -1) {
 #else
-    if (truncate(stmp, 0) == -1) {
+    if (stat(sudoers, &sudoers_sb) == -1) {
 #endif
-	(void) fprintf(stderr, "%s: can't truncate %s: %s\n", Argv[0],
-	    stmp, strerror(errno));
+	(void) fprintf(stderr, "%s: can't stat %s: %s\n",
+	    Argv[0], sudoers, strerror(errno));
 	Exit(-1);
     }
 
+    /*
+     * Open sudoers temp file.
+     */
+    stmp_fd = open(stmp, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+    if (stmp_fd < 0) {
+	(void) fprintf(stderr, "%s: %s: %s\n", Argv[0], stmp, strerror(errno));
+	exit(1);
+    }
+
     /* Install signal handlers to clean up stmp if we are killed. */
     setup_signals();
 
-    (void) memset(&sudoers_sb, 0, sizeof(sudoers_sb));
-    if (stat(sudoers, &sudoers_sb) == -1 && errno != ENOENT) {
-	(void) fprintf(stderr, "%s: %s\n", Argv[0], strerror(errno));
-	Exit(-1);
-    }
-    sudoers_fd = open(sudoers, O_RDONLY);
-    if (sudoers_fd == -1 && errno != ENOENT) {
-	(void) fprintf(stderr, "%s: %s\n", Argv[0], strerror(errno));
-	Exit(-1);
-    }
-
     /* Copy sudoers -> stmp and reset the mtime */
-    if (sudoers_fd != -1) {
+    if (sudoers_sb.st_size) {
 	while ((n = read(sudoers_fd, buf, sizeof(buf))) > 0)
 	    if (write(stmp_fd, buf, n) != n) {
 		(void) fprintf(stderr, "%s: Write failed: %s\n", Argv[0],
@@ -214,10 +214,10 @@ main(argc, argv)
 		Exit(-1);
 	    }
 
-	(void) close(sudoers_fd);
-    }
-    (void) close(stmp_fd);
-    (void) touch(stmp, sudoers_sb.st_mtime);
+	(void) close(stmp_fd);
+	(void) touch(stmp, sudoers_sb.st_mtime);
+    } else
+	(void) close(stmp_fd);
 
     /*
      * Edit the temp file and parse it (for sanity checking)
@@ -369,7 +369,7 @@ main(argc, argv)
 	}
     }
 
-    return(0);
+    exit(0);
 }
 
 /*