2005-05-04 Dmitry V. Levin Fix race conditions in code which sets ownership and mode of regular files in copy-in and copy-pass modes (CAN-2005-1111). * src/copyin.c (create_final_defers, copyin_regular_file): Use fchown(2) instead of chown(2) and fchmod(2) instead of chmod(2) to change file mode. Close file descriptor after changing file permissions. * src/copypass.c(process_copy_pass): Likewise. diff -uprk.orig cpio-2.6.orig/src/copyin.c cpio-2.6/src/copyin.c --- cpio-2.6.orig/src/copyin.c 2005-05-04 15:48:33 +0000 +++ cpio-2.6/src/copyin.c 2005-05-04 16:57:39 +0000 @@ -389,18 +389,15 @@ create_final_defers () continue; } - if (close (out_file_des) < 0) - error (0, errno, "%s: close", d->header.c_name); - /* File is now copied; set attributes. */ if (!no_chown_flag) - if ((chown (d->header.c_name, + if ((fchown (out_file_des, set_owner_flag ? set_owner : d->header.c_uid, set_group_flag ? set_group : d->header.c_gid) < 0) && errno != EPERM) error (0, errno, "%s: chown", d->header.c_name); - /* chown may have turned off some permissions we wanted. */ - if (chmod (d->header.c_name, (int) d->header.c_mode) < 0) + /* fchown may have turned off some permissions we wanted. */ + if (fchmod (out_file_des, (int) d->header.c_mode) < 0) error (0, errno, "%s: chmod", d->header.c_name); if (retain_time_flag) { @@ -408,6 +405,9 @@ create_final_defers () if (utime (d->header.c_name, ×) < 0) error (0, errno, "%s: utime", d->header.c_name); } + + if (close (out_file_des) < 0) + error (0, errno, "%s: close", d->header.c_name); } } @@ -557,8 +557,6 @@ copyin_regular_file (struct new_cpio_hea write (out_file_des, "", 1); delayed_seek_count = 0; } - if (close (out_file_des) < 0) - error (0, errno, "%s: close", file_hdr->c_name); if (archive_format == arf_crcascii) { @@ -569,14 +567,14 @@ copyin_regular_file (struct new_cpio_hea /* File is now copied; set attributes. */ if (!no_chown_flag) - if ((chown (file_hdr->c_name, + if ((fchown (out_file_des, set_owner_flag ? set_owner : file_hdr->c_uid, set_group_flag ? set_group : file_hdr->c_gid) < 0) && errno != EPERM) error (0, errno, "%s: chown", file_hdr->c_name); - /* chown may have turned off some permissions we wanted. */ - if (chmod (file_hdr->c_name, (int) file_hdr->c_mode) < 0) + /* fchown may have turned off some permissions we wanted. */ + if (fchmod (out_file_des, (int) file_hdr->c_mode) < 0) error (0, errno, "%s: chmod", file_hdr->c_name); if (retain_time_flag) @@ -589,6 +587,9 @@ copyin_regular_file (struct new_cpio_hea if (utime (file_hdr->c_name, ×) < 0) error (0, errno, "%s: utime", file_hdr->c_name); } + + if (close (out_file_des) < 0) + error (0, errno, "%s: close", file_hdr->c_name); tape_skip_padding (in_file_des, file_hdr->c_filesize); if (file_hdr->c_nlink > 1 diff -uprk.orig cpio-2.6.orig/src/copypass.c cpio-2.6/src/copypass.c --- cpio-2.6.orig/src/copypass.c 2005-05-04 15:48:33 +0000 +++ cpio-2.6/src/copypass.c 2005-05-04 16:57:32 +0000 @@ -184,18 +184,16 @@ process_copy_pass () } if (close (in_file_des) < 0) error (0, errno, "%s: close", input_name.ds_string); - if (close (out_file_des) < 0) - error (0, errno, "%s: close", output_name.ds_string); /* Set the attributes of the new file. */ if (!no_chown_flag) - if ((chown (output_name.ds_string, + if ((fchown (out_file_des, set_owner_flag ? set_owner : in_file_stat.st_uid, set_group_flag ? set_group : in_file_stat.st_gid) < 0) && errno != EPERM) error (0, errno, "%s: chown", output_name.ds_string); - /* chown may have turned off some permissions we wanted. */ - if (chmod (output_name.ds_string, in_file_stat.st_mode) < 0) + /* fchown may have turned off some permissions we wanted. */ + if (fchmod (out_file_des, in_file_stat.st_mode) < 0) error (0, errno, "%s: chmod", output_name.ds_string); if (reset_time_flag) { @@ -219,6 +217,8 @@ process_copy_pass () if (utime (output_name.ds_string, ×) < 0) error (0, errno, "%s: utime", output_name.ds_string); } + if (close (out_file_des) < 0) + error (0, errno, "%s: close", output_name.ds_string); warn_if_file_changed(input_name.ds_string, in_file_stat.st_size, in_file_stat.st_mtime); }