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 7b70192dd3b0d39ef93322cb6fa03c4e42a1340e..4e7d6d1f097116a5686c0a64e529f21d6e5ac617 100644 (file)
--- 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 eb874ab0bbdec9df7a8f3d86d923fc39d9d685b1..76b12cb9f10ac1dfaf4e67b1dd2d9be214fc9a46 100644 (file)
--- 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 {