ariadne
/
smol
2
1
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

2010-04-09 Chris Allegretta <chrisa@asty.org> 

* files.c (do_writeout): Better security fixes for backup file writing, 
          mangled from submission by Dan Rosenberg <dan.j.rosenberg at gmail>.



git-svn-id: svn://svn.savannah.gnu.org/nano/trunk/nano@4496 35c25a1d-7b9e-4130-9fde-d3aeb78583b8
master
Chris Allegretta 2010-04-09 15:01:51 +00:00
parent dbbf9a052f
commit 7f61a6cb79
2 changed files with 28 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
ChangeLog
View File

@ -1,4 +1,8 @@
2010-04-02 Chris Allegretta <chrisa@asty.org> 2010-04-09 Chris Allegretta <chrisa@asty.org>
* files.c (do_writeout): Better security fixes for backup file writing,
mangled from submission by Dan Rosenberg <dan.j.rosenberg at gmail>.
2010-04-08 Chris Allegretta <chrisa@asty.org>
* files.c (do_writeout): Previous fixes should not cause a crash * files.c (do_writeout): Previous fixes should not cause a crash
when saving a new file. Discovered by Mike Frysinger <vapier@gentoo.org>. when saving a new file. Discovered by Mike Frysinger <vapier@gentoo.org>.

40
src/files.c
View File

@ -1515,11 +1515,11 @@ bool write_file(const char *name, FILE *f_open, bool tmp, append_type
if (ISSET(BACKUP_FILE) && !tmp && realexists && ((append != if (ISSET(BACKUP_FILE) && !tmp && realexists && ((append !=
OVERWRITE || openfile->mark_set) || OVERWRITE || openfile->mark_set) ||
openfile->current_stat->st_mtime == st.st_mtime)) { openfile->current_stat->st_mtime == st.st_mtime)) {
int backup_fd;
FILE *backup_file; FILE *backup_file;
char *backupname; char *backupname;
struct utimbuf filetime; struct utimbuf filetime;
int copy_status; int copy_status;
struct stat backupst;
/* Save the original file's access and modification times. */ /* Save the original file's access and modification times. */
filetime.actime = openfile->current_stat->st_atime; filetime.actime = openfile->current_stat->st_atime;
@ -1589,27 +1589,36 @@ bool write_file(const char *name, FILE *f_open, bool tmp, append_type
sprintf(backupname, "%s~", realname); sprintf(backupname, "%s~", realname);
} }
if (stat(backupname, &backupst) != -1 && /* First, unlink any existing backups. Next, open the backup
(backupst.st_uid != st.st_uid)) { file with O_CREAT and O_EXCL. If it succeeds, we
statusbar(_("Error writing backup file %s: File owner mismatch"), backupname, have a file descriptor to a new backup file. */
strerror(errno)); if (unlink(backupname) < 0 && errno != ENOENT) {
statusbar(_("Error writing backup file %s: %s"), backupname,
strerror(errno));
free(backupname); free(backupname);
goto cleanup_and_exit; goto cleanup_and_exit;
} }
backup_fd = open(backupname, O_WRONLY | O_CREAT | O_APPEND,
S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
/* Now we've got a safe file stream. If the previous open()
call failed, this will return NULL. */
backup_file = fdopen(backup_fd, "wb");
/* Open the destination backup file. Before we write to it, we if (backup_fd < 0 || backup_file == NULL) {
* set its permissions, so no unauthorized person can read it as statusbar(_("Error writing backup file %s: %s"), backupname,
* we write. */ strerror(errno));
backup_file = fopen(backupname, "wb"); free(backupname);
goto cleanup_and_exit;
}
if (backup_file == NULL || chmod(backupname, if (fchmod(backup_fd, openfile->current_stat->st_mode) == -1 ||
openfile->current_stat->st_mode) == -1) { fchown(backup_fd, openfile->current_stat->st_uid,
openfile->current_stat->st_gid) == -1 ) {
statusbar(_("Error writing backup file %s: %s"), backupname, statusbar(_("Error writing backup file %s: %s"), backupname,
strerror(errno)); strerror(errno));
free(backupname); free(backupname);
if (backup_file != NULL) fclose(backup_file);
fclose(backup_file);
/* If we can't write to the backup, DONT go on, since /* If we can't write to the backup, DONT go on, since
whatever caused the backup file to fail (e.g. disk whatever caused the backup file to fail (e.g. disk
full may well cause the real file write to fail, which full may well cause the real file write to fail, which
@ -1625,10 +1634,7 @@ bool write_file(const char *name, FILE *f_open, bool tmp, append_type
copy_status = copy_file(f, backup_file); copy_status = copy_file(f, backup_file);
/* And set its metadata. */ /* And set its metadata. */
if (copy_status != 0 || chown(backupname, if (copy_status != 0 || utime(backupname, &filetime) == -1) {
openfile->current_stat->st_uid,
openfile->current_stat->st_gid) == -1 ||
utime(backupname, &filetime) == -1) {
if (copy_status == -1) { if (copy_status == -1) {
statusbar(_("Error reading %s: %s"), realname, statusbar(_("Error reading %s: %s"), realname,
strerror(errno)); strerror(errno));