Re: [Nano-devel] [PATCH] pull in the futimens module from gnulib

[Benno Schulenberg]

Hello Kamil,

On Fri, Mar 31, 2017, at 13:45, Kamil Dudka wrote:
> ... and use futimens() instead of utime() to prevent a symlink attack
> while creating a backup file.  Otherwise, a non-privileged user could
> create an arbitrary symlink with name of the backup file and this way
> fool a privileged user to call utime() on the attacker-chosen file.

How exactly does the use of futimens prevent a symlink attack?

You also change the order of things: first setting the metadata
of the copy and then actually doing the copying.  Is this an
essential part of preventing the attack?  Or is this a fix for
something else?

Benno

>       /* Save the original file's access and modification times. */
> -     filetime.actime = openfile->current_stat->st_atime;
> -     filetime.modtime = openfile->current_stat->st_mtime;
> +     filetime[/* atime */ 0].tv_sec = openfile->current_stat->st_atime;
> +     filetime[/* mtime */ 1].tv_sec = openfile->current_stat->st_mtime;
>  
>       if (f_open == NULL) {
>           /* Open the original file to copy to the backup. */
> @@ -1789,17 +1789,8 @@ 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 (futimens(backup_fd, filetime) == -1 && !ISSET(INSECURE_BACKUP)) {
>           if (prompt_failed_backupwrite(backupname))
>               goto skip_backup;
>           statusline(HUSH, _("Error writing backup file %s: %s"),
> @@ -1811,6 +1802,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);
>      }
>  

-- 
http://www.fastmail.com - Send your email first class