From 233f43e82ec1dc790655b5a15e13116637814456 Mon Sep 17 00:00:00 2001 From: Kamil Dudka Date: Tue, 4 Apr 2017 12:33:06 +0200 Subject: [PATCH] use upstream patch to prevent symlink attack ... while creating a backup --- 0001-nano-2.8.0-backup-futimens.patch | 126 ++++++++++++++++++ ...timens-if-available-instead-of-utime.patch | 96 ------------- nano.spec | 9 +- 3 files changed, 131 insertions(+), 100 deletions(-) create mode 100644 0001-nano-2.8.0-backup-futimens.patch delete mode 100644 0002-use-futimens-if-available-instead-of-utime.patch diff --git a/0001-nano-2.8.0-backup-futimens.patch b/0001-nano-2.8.0-backup-futimens.patch new file mode 100644 index 0000000..1716850 --- /dev/null +++ b/0001-nano-2.8.0-backup-futimens.patch @@ -0,0 +1,126 @@ +From 48bc217a9e0cc5c6ad494cff925185912740dbb4 Mon Sep 17 00:00:00 2001 +From: Kamil Dudka +Date: Tue, 4 Apr 2017 09:29:31 +0200 +Subject: [PATCH] backup: prevent a symlink attack by operating on the file + descriptor + +Use futimens() instead of utime() to change the timestamps on a backup +file. Otherwise, a non-privileged user could create an arbitrary symlink +with the name of the backup file and in this way fool a privileged user +to call utime() on the attacker-chosen file. + +Upstream-commit: 70bcf752dcc82d1eed04ba4f900ed69ce2b97500 +Signed-off-by: Kamil Dudka +--- + src/files.c | 24 ++++++++++++++---------- + src/proto.h | 2 +- + 2 files changed, 15 insertions(+), 11 deletions(-) + +diff --git a/src/files.c b/src/files.c +index 033b963..df2627c 100644 +--- a/src/files.c ++++ b/src/files.c +@@ -1541,12 +1541,14 @@ void init_backup_dir(void) + + /* Read from inn, write to out. We assume inn is opened for reading, + * and out for writing. We return 0 on success, -1 on read error, or -2 +- * on write error. */ +-int copy_file(FILE *inn, FILE *out) ++ * on write error. inn is always closed by this function, out is closed ++ * only if close_out is true. */ ++int copy_file(FILE *inn, FILE *out, bool close_out) + { + int retval = 0; + char buf[BUFSIZ]; + size_t charsread; ++ int (*flush_out_fnc)(FILE *) = (close_out) ? fclose : fflush; + + assert(inn != NULL && out != NULL && inn != out); + +@@ -1564,7 +1566,7 @@ int copy_file(FILE *inn, FILE *out) + + if (fclose(inn) == EOF) + retval = -1; +- if (fclose(out) == EOF) ++ if (flush_out_fnc(out) == EOF) + retval = -2; + + return retval; +@@ -1655,13 +1657,13 @@ bool write_file(const char *name, FILE *f_open, bool tmp, + int backup_fd; + FILE *backup_file; + char *backupname; +- struct utimbuf filetime; ++ static struct timespec filetime[2]; + int copy_status; + int backup_cflags; + + /* Save the original file's access and modification times. */ +- filetime.actime = openfile->current_stat->st_atime; +- filetime.modtime = openfile->current_stat->st_mtime; ++ filetime[0].tv_sec = openfile->current_stat->st_atime; ++ filetime[1].tv_sec = openfile->current_stat->st_mtime; + + if (f_open == NULL) { + /* Open the original file to copy to the backup. */ +@@ -1790,7 +1792,7 @@ bool write_file(const char *name, FILE *f_open, bool tmp, + #endif + + /* Copy the file. */ +- copy_status = copy_file(f, backup_file); ++ copy_status = copy_file(f, backup_file, FALSE); + + if (copy_status != 0) { + statusline(ALERT, _("Error reading %s: %s"), realname, +@@ -1799,7 +1801,8 @@ bool write_file(const char *name, FILE *f_open, bool tmp, + } + + /* And set its metadata. */ +- if (utime(backupname, &filetime) == -1 && !ISSET(INSECURE_BACKUP)) { ++ if (futimens(backup_fd, filetime) == -1 && !ISSET(INSECURE_BACKUP)) { ++ fclose(backup_file); + if (prompt_failed_backupwrite(backupname)) + goto skip_backup; + statusline(HUSH, _("Error writing backup file %s: %s"), +@@ -1811,6 +1814,7 @@ bool write_file(const char *name, FILE *f_open, bool tmp, + goto cleanup_and_exit; + } + ++ fclose(backup_file); + free(backupname); + } + +@@ -1867,7 +1871,7 @@ bool write_file(const char *name, FILE *f_open, bool tmp, + } + } + +- if (f_source == NULL || copy_file(f_source, f) != 0) { ++ if (f_source == NULL || copy_file(f_source, f, TRUE) != 0) { + statusline(ALERT, _("Error writing temp file: %s"), + strerror(errno)); + unlink(tempname); +@@ -1975,7 +1979,7 @@ bool write_file(const char *name, FILE *f_open, bool tmp, + goto cleanup_and_exit; + } + +- if (copy_file(f_source, f) == -1) { ++ if (copy_file(f_source, f, TRUE) == -1) { + statusline(ALERT, _("Error writing %s: %s"), realname, + strerror(errno)); + goto cleanup_and_exit; +diff --git a/src/proto.h b/src/proto.h +index 0250ad6..d8255a9 100644 +--- a/src/proto.h ++++ b/src/proto.h +@@ -298,7 +298,7 @@ void init_backup_dir(void); + int delete_lockfile(const char *lockfilename); + int write_lockfile(const char *lockfilename, const char *origfilename, bool modified); + #endif +-int copy_file(FILE *inn, FILE *out); ++int copy_file(FILE *inn, FILE *out, bool close_out); + bool write_file(const char *name, FILE *f_open, bool tmp, + kind_of_writing_type method, bool nonamechange); + #ifndef NANO_TINY +-- +2.9.3 + diff --git a/0002-use-futimens-if-available-instead-of-utime.patch b/0002-use-futimens-if-available-instead-of-utime.patch deleted file mode 100644 index c528c07..0000000 --- a/0002-use-futimens-if-available-instead-of-utime.patch +++ /dev/null @@ -1,96 +0,0 @@ -From 23510b930ea31f7de8005e2f0ff6cab7062b4e26 Mon Sep 17 00:00:00 2001 -From: Kamil Dudka -Date: Thu, 19 Aug 2010 15:23:06 +0200 -Subject: [PATCH 2/2] use futimens() if available, instead of utime() - ---- - configure.ac | 1 + - src/files.c | 46 +++++++++++++++++++++++++++++++++++----------- - 2 files changed, 36 insertions(+), 11 deletions(-) - -diff --git a/configure.ac b/configure.ac -index 66f8ee3..f4975d3 100644 ---- a/configure.ac -+++ b/configure.ac -@@ -468,6 +468,7 @@ int main(void) - - - dnl Checks for functions. -+AC_CHECK_FUNCS(futimens) - - if test "x$enable_utf8" != xno; then - AC_CHECK_FUNCS(iswalnum iswpunct mblen mbstowcs mbtowc wctomb) -diff --git a/src/files.c b/src/files.c -index 99cc1b8..9a1bdcc 100644 ---- a/src/files.c -+++ b/src/files.c -@@ -1570,6 +1570,29 @@ int copy_file(FILE *inn, FILE *out) - return retval; - } - -+#ifdef HAVE_FUTIMENS -+/* set atime/mtime by file descriptor */ -+int utime_wrap(int fd, const char *filename, struct utimbuf *ut) -+{ -+ struct timespec times[2]; -+ (void) filename; -+ -+ times[0].tv_sec = ut->actime; -+ times[1].tv_sec = ut->modtime; -+ times[0].tv_nsec = 0L; -+ times[1].tv_nsec = 0L; -+ -+ return futimens(fd, times); -+} -+#else -+/* set atime/mtime by file name */ -+int utime_wrap(int fd, const char *filename, struct utimbuf *ut) -+{ -+ (void) fd; -+ return utime(filename, ut); -+} -+#endif -+ - /* Write a file out to disk. If f_open isn't NULL, we assume that it is - * a stream associated with the file, and we don't try to open it - * ourselves. If tmp is TRUE, we set the umask to disallow anyone else -@@ -1789,17 +1812,9 @@ bool write_file(const char *name, FILE *f_open, bool tmp, - fprintf(stderr, "Backing up %s to %s\n", realname, backupname); - #endif - -- /* Copy the file. */ -- copy_status = copy_file(f, backup_file); -- -- if (copy_status != 0) { -- statusline(ALERT, _("Error reading %s: %s"), realname, -- strerror(errno)); -- goto cleanup_and_exit; -- } -- -- /* And set its metadata. */ -- if (utime(backupname, &filetime) == -1 && !ISSET(INSECURE_BACKUP)) { -+ /* Set backup's file metadata. */ -+ if (utime_wrap(backup_fd, backupname, &filetime) == -1 -+ && !ISSET(INSECURE_BACKUP)) { - if (prompt_failed_backupwrite(backupname)) - goto skip_backup; - statusline(HUSH, _("Error writing backup file %s: %s"), -@@ -1811,6 +1826,15 @@ bool write_file(const char *name, FILE *f_open, bool tmp, - goto cleanup_and_exit; - } - -+ /* Copy the file. */ -+ copy_status = copy_file(f, backup_file); -+ -+ if (copy_status != 0) { -+ statusline(ALERT, _("Error reading %s: %s"), realname, -+ strerror(errno)); -+ goto cleanup_and_exit; -+ } -+ - free(backupname); - } - --- -1.7.4 - diff --git a/nano.spec b/nano.spec index 5d19ce0..87fe6b1 100644 --- a/nano.spec +++ b/nano.spec @@ -7,10 +7,9 @@ URL: https://www.nano-editor.org Source: https://www.nano-editor.org/dist/v2.8/%{name}-%{version}.tar.gz Source2: nanorc -# http://lists.gnu.org/archive/html/nano-devel/2010-08/msg00005.html -Patch2: 0002-use-futimens-if-available-instead-of-utime.patch +# backup: prevent a symlink attack by operating on the file descriptor +Patch1: 0001-nano-2.8.0-backup-futimens.patch -BuildRequires: automake BuildRequires: file-devel BuildRequires: gettext-devel BuildRequires: git @@ -27,7 +26,6 @@ GNU nano is a small and friendly text editor. %prep %autosetup -S git -autoreconf -v %build mkdir build @@ -83,6 +81,9 @@ exit 0 %{_datadir}/nano %changelog +* Tue Apr 04 2017 Kamil Dudka - 2.8.0-2 +- use upstream patch to prevent symlink attack while creating a backup + * Fri Mar 31 2017 Kamil Dudka - 2.8.0-1 - new upstream release