From f62c17e3786ac6643981d9ec68a1cd130ffcf149 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Wed, 2 May 2001 08:33:18 +0000 Subject: [PATCH] use mkstemp on systems where it is secure --- Makefile.in | 7 ++----- acconfig.h | 1 + configure.in | 22 ++++++++++++++++++++++ lib/compat.c | 1 + receiver.c | 26 +++++++++++--------------- syscall.c | 29 +++++++++++++++++++++-------- 6 files changed, 58 insertions(+), 28 deletions(-) diff --git a/Makefile.in b/Makefile.in index 53a91bab..3bf7d492 100644 --- a/Makefile.in +++ b/Makefile.in @@ -54,17 +54,14 @@ install-strip: $(MAKE) INSTALLCMD='$(INSTALLCMD) -s' install rsync: $(OBJS) - @-echo " Note: The link command may give a warning about use of mktemp." - @-echo " The warning may be ignored because rsync uses this function in a safe way." $(CC) $(CFLAGS) $(LDFLAGS) -o rsync $(OBJS) $(LIBS) Makefile: Makefile.in configure config.status echo "WARNING: You need to run ./config.status --recheck" -# It's OK if this fails, because we don't require people to have -# autoconf installed. +# don't actually run autoconf, just issue a warning configure: configure.in - -cd $(srcdir) && autoconf + echo "WARNING: you need to rerun autoconf" rsync.1: rsync.yo yodl2man -o rsync.1 rsync.yo diff --git a/acconfig.h b/acconfig.h index 7028d74a..4033a94c 100644 --- a/acconfig.h +++ b/acconfig.h @@ -13,3 +13,4 @@ #undef REPLACE_INET_ATON #undef HAVE_GETTIMEOFDAY_TZ #undef HAVE_SOCKETPAIR +#undef HAVE_SECURE_MKSTEMP diff --git a/configure.in b/configure.in index cbd1f242..d8dd6c03 100644 --- a/configure.in +++ b/configure.in @@ -231,6 +231,28 @@ if test x"$rsync_cv_HAVE_GETTIMEOFDAY_TZ" = x"yes"; then AC_DEFINE(HAVE_GETTIMEOFDAY_TZ) fi +AC_CACHE_CHECK([for secure mkstemp],rsync_cv_HAVE_SECURE_MKSTEMP,[ +AC_TRY_RUN([#include +#include +#include +#include +main() { + struct stat st; + char tpl[20]="/tmp/test.XXXXXX"; + int fd = mkstemp(tpl); + if (fd == -1) exit(1); + unlink(tpl); + if (fstat(fd, &st) != 0) exit(1); + if ((st.st_mode & 0777) != 0600) exit(1); + exit(0); +}], +rsync_cv_HAVE_SECURE_MKSTEMP=yes, +rsync_cv_HAVE_SECURE_MKSTEMP=no, +rsync_cv_HAVE_SECURE_MKSTEMP=cross)]) +if test x"$rsync_cv_HAVE_SECURE_MKSTEMP" = x"yes"; then + AC_DEFINE(HAVE_SECURE_MKSTEMP) +fi + AC_CACHE_CHECK([for broken inet_ntoa],rsync_cv_REPLACE_INET_NTOA,[ AC_TRY_RUN([ diff --git a/lib/compat.c b/lib/compat.c index ef57d900..426eee2a 100644 --- a/lib/compat.c +++ b/lib/compat.c @@ -103,6 +103,7 @@ { size_t len = strlen(s); size_t ret = len; + if (bufsize <= 0) return 0; if (len >= bufsize) len = bufsize-1; memcpy(d, s, len); d[len] = 0; diff --git a/receiver.c b/receiver.c index 7be95889..0249d8a4 100644 --- a/receiver.c +++ b/receiver.c @@ -304,6 +304,7 @@ int recv_files(int f_in,struct file_list *flist,char *local_name,int f_gen) int fd1,fd2; STRUCT_STAT st; char *fname; + char template[MAXPATHLEN]; char fnametmp[MAXPATHLEN]; char *fnamecmp; char fnamecmpbuf[MAXPATHLEN]; @@ -412,17 +413,7 @@ int recv_files(int f_in,struct file_list *flist,char *local_name,int f_gen) continue; } - /* mktemp is deliberately used here instead of mkstemp. - because O_EXCL is used on the open, the race condition - is not a problem or a security hole, and we want to - control the access permissions on the created file. */ - if (NULL == do_mktemp(fnametmp)) { - rprintf(FERROR,"mktemp %s failed\n",fnametmp); - receive_data(f_in,buf,-1,NULL,file->length); - if (buf) unmap_file(buf); - if (fd1 != -1) close(fd1); - continue; - } + strlcpy(template, fnametmp, sizeof(template)); /* we initially set the perms without the setuid/setgid bits to ensure that there is no race @@ -430,16 +421,21 @@ int recv_files(int f_in,struct file_list *flist,char *local_name,int f_gen) the lchown. Thanks to snabb@epipe.fi for pointing this out. We also set it initially without group access because of a similar race condition. */ - fd2 = do_open(fnametmp,O_WRONLY|O_CREAT|O_EXCL, - file->mode & INITACCESSPERMS); + fd2 = do_mkstemp(fnametmp, file->mode & INITACCESSPERMS); + if (fd2 == -1) { + rprintf(FERROR,"mkstemp %s failed\n",fnametmp); + receive_data(f_in,buf,-1,NULL,file->length); + if (buf) unmap_file(buf); + continue; + } /* in most cases parent directories will already exist because their information should have been previously transferred, but that may not be the case with -R */ if (fd2 == -1 && relative_paths && errno == ENOENT && create_directory_path(fnametmp) == 0) { - fd2 = do_open(fnametmp,O_WRONLY|O_CREAT|O_EXCL, - file->mode & INITACCESSPERMS); + strlcpy(fnametmp, template, sizeof(fnametmp)); + fd2 = do_mkstemp(fnametmp, file->mode & INITACCESSPERMS); } if (fd2 == -1) { rprintf(FERROR,"cannot create %s : %s\n",fnametmp,strerror(errno)); diff --git a/syscall.c b/syscall.c index 3780bac5..a56eb793 100644 --- a/syscall.c +++ b/syscall.c @@ -113,14 +113,27 @@ int do_mkdir(char *fname, mode_t mode) return mkdir(fname, mode); } -char *do_mktemp(char *template) -{ - if (dry_run) return NULL; - if (read_only) {errno = EROFS; return NULL;} - - /* TODO: Replace this with a good builtin mkstemp, perhaps - * from OpenBSD. Some glibc versions are buggy. */ - return mktemp(template); +/* like mkstemp but forces permissions */ +int do_mkstemp(char *template, mode_t perms) +{ + if (dry_run) return -1; + if (read_only) {errno = EROFS; return -1;} + +#if defined(HAVE_SECURE_MKSTEMP) && defined(HAVE_FCHMOD) + { + int fd = mkstemp(template); + if (fd == -1) return -1; + if (fchmod(fd, perms) != 0) { + close(fd); + unlink(template); + return -1; + } + return fd; + } +#else + if (!mktemp(template)) return -1; + return open(template, O_RDWR|O_EXCL|O_CREAT, perms); +#endif } int do_stat(const char *fname, STRUCT_STAT *st) -- 2.34.1