rpms/nano
2
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

use upstream patch to prevent symlink attack

Browse Source 
... while creating a backup
This commit is contained in:
Kamil Dudka 2017-04-04 12:33:06 +02:00
parent 88b7b6e9ff
commit 233f43e82e
3 changed files with 131 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

126
0001-nano-2.8.0-backup-futimens.patch Normal file
View File

@ -0,0 +1,126 @@
From 48bc217a9e0cc5c6ad494cff925185912740dbb4 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
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 <kdudka@redhat.com>
---
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

96
0002-use-futimens-if-available-instead-of-utime.patch
View File

@ -1,96 +0,0 @@
From 23510b930ea31f7de8005e2f0ff6cab7062b4e26 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
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

9
nano.spec
View File

@ -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 <kdudka@redhat.com> - 2.8.0-2
- use upstream patch to prevent symlink attack while creating a backup
* Fri Mar 31 2017 Kamil Dudka <kdudka@redhat.com> - 2.8.0-1
- new upstream release