fixed a race condition in rsync that opened a security hole. The
authorAndrew Tridgell <tridge@samba.org>
Thu, 18 Jun 1998 12:17:23 +0000 (12:17 +0000)
committerAndrew Tridgell <tridge@samba.org>
Thu, 18 Jun 1998 12:17:23 +0000 (12:17 +0000)
temporary files were being created with the same permissions as the
original file. So if the file was setuid but not owned by the user
doing the transfer then there was a window of opportunity for a
malicious user to execute it with the wrong permissions while it was
being transferred.

Thanks to snabb@epipe.fi for pointing this out.

rsync.c
rsync.h

diff --git a/rsync.c b/rsync.c
index 1ec4c77dc53508c86abd8a9aa03b0dcec21b3822..8e7b74c92082736f223f08166bc09d8c58737be5 100644 (file)
--- a/rsync.c
+++ b/rsync.c
@@ -886,10 +886,18 @@ int recv_files(int f_in,struct file_list *flist,char *local_name,int f_gen)
                        continue;
                }
 
                        continue;
                }
 
-               fd2 = do_open(fnametmp,O_WRONLY|O_CREAT|O_EXCL,file->mode);
+               /* we initially set the perms without the
+                  setuid/setgid bits to ensure that there is no race
+                  condition. They are then correctly updated after
+                  the lchown. Thanks to snabb@epipe.fi for pointing
+                  this out */
+               fd2 = do_open(fnametmp,O_WRONLY|O_CREAT|O_EXCL,
+                             file->mode & ACCESSPERMS);
+
                if (fd2 == -1 && relative_paths && errno == ENOENT && 
                    create_directory_path(fnametmp) == 0) {
                if (fd2 == -1 && relative_paths && errno == ENOENT && 
                    create_directory_path(fnametmp) == 0) {
-                       fd2 = do_open(fnametmp,O_WRONLY|O_CREAT|O_EXCL,file->mode);
+                       fd2 = do_open(fnametmp,O_WRONLY|O_CREAT|O_EXCL,
+                                     file->mode & ACCESSPERMS);
                }
                if (fd2 == -1) {
                        rprintf(FERROR,"open %s : %s\n",fnametmp,strerror(errno));
                }
                if (fd2 == -1) {
                        rprintf(FERROR,"open %s : %s\n",fnametmp,strerror(errno));
diff --git a/rsync.h b/rsync.h
index 345144d5876563a8c4a04f2d414be64c967860a9..deb20a23581df6b8e55828e6164fe0158a23f3f9 100644 (file)
--- a/rsync.h
+++ b/rsync.h
@@ -437,3 +437,6 @@ extern int errno;
 
 #define IS_DEVICE(mode) (S_ISCHR(mode) || S_ISBLK(mode) || S_ISSOCK(mode) || S_ISFIFO(mode))
 
 
 #define IS_DEVICE(mode) (S_ISCHR(mode) || S_ISBLK(mode) || S_ISSOCK(mode) || S_ISFIFO(mode))
 
+#ifndef ACCESSPERMS
+#define ACCESSPERMS 0777
+#endif