From: Chris Allegretta <chrisa@asty.org>
Date: Mon, 4 Dec 2000 05:15:39 +0000 (+0000)
Subject: Okay, forget O_EXCL, do lots of other obscure checks instead =)
X-Git-Tag: v0.9.23~12
X-Git-Url: https://git.wh0rd.org/?a=commitdiff_plain;h=07f9ee059fdb91d16cb72f17fdfb9832a1fa13c5;p=nano.git

Okay, forget O_EXCL, do lots of other obscure checks instead =)


git-svn-id: svn://svn.savannah.gnu.org/nano/trunk/nano@381 35c25a1d-7b9e-4130-9fde-d3aeb78583b8
---

diff --git a/ChangeLog b/ChangeLog
index 7b70192d..4e7d6d1f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,9 +1,9 @@
 CVS code -
 - files.c:
   write_file()
-	- Added O_EXCL to open call if tmp is set, more security which hopefully
-	  fixes any remaining security issues.
 	- Added tmp check to TMP_OPT section (how apropriate).
+	- Added new consistency checking code from securityfocus 
+	  article by Oliver Friedrichs.
 - winio.c:
   edit_add()
 	- Off by one display error (fix by Rocco Corsi).
diff --git a/files.c b/files.c
index eb874ab0..76b12cb9 100644
--- a/files.c
+++ b/files.c
@@ -301,8 +301,8 @@ int write_file(char *name, int tmp)
     long size, lineswritten = 0;
     char buf[PATH_MAX + 1];
     filestruct *fileptr;
-    int fd, mask = 0, realexists;
-    struct stat st;
+    int fd, mask = 0, realexists, anyexists;
+    struct stat st, st2;
     static char *realname = NULL;
 
     if (!strcmp(name, "")) {
@@ -327,24 +327,18 @@ int write_file(char *name, int tmp)
     /* Check to see if the file is a regular file and FOLLOW_SYMLINKS is
        set.  If so then don't do the delete and recreate code which would
        cause unexpected behavior */
-    lstat(realname, &st);
+    anyexists = lstat(realname, &st);
 
-    /* New case: if the file exists, just give up.  Easy way out of
-	all security issues */
-    if (tmp && realexists != -1)
+    /* New case: if the file exists, just give up */
+    if (tmp && anyexists != -1)
 	 return -1;
     else if (ISSET(FOLLOW_SYMLINKS) || !S_ISLNK(st.st_mode)) {
 
-	/* If tmp is set, use O_EXCL, more security, YAY! */
-	if (tmp)
-	    fd = open(realname, O_WRONLY | O_CREAT | O_EXCL | O_TRUNC,
-		       S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
-		       S_IWOTH);
-	else
-	    fd = open(realname, O_WRONLY | O_CREAT | O_TRUNC,
-		       S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
-		       S_IWOTH);
-	/* Open the file and truncate it.  Trust the symlink. */
+	fd = open(realname, O_WRONLY | O_CREAT | O_TRUNC,
+			S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
+			S_IWOTH);
+
+	/* First, just give up if we couldn't even open the file */
 	if (fd == -1) {
 	    if (!tmp && ISSET(TEMP_OPT)) {
 		UNSET(TEMP_OPT);
@@ -355,6 +349,25 @@ int write_file(char *name, int tmp)
 	    free(realname);
 	    return -1;
 	}
+
+	/* Now we fstat() the file, to make sure it's the same file still!
+	   Thanks to Oliver Friedrichs(?) for this code from securityfocus */
+
+ 	if (fstat(fd, &st2) != 0) {
+	    close(fd);
+	    return -1; 
+	}
+      
+	/* Here we make sure the inode and device numbers are the
+	 * same in the file we actually opened, compared to the file
+	 * we performed the initial lstat() call on.
+	 */
+        
+	if (st.st_ino != st2.st_ino || st.st_dev != st2.st_dev) {
+	    close(fd);
+	    return -1;
+	}   
+
     }
     /* Don't follow symlink.  Create new file. */
     else {