install: create destination file with safer modes before copy

src/uu/install/src/install.rs

@@ -13,9 +13,12 @@
 use std::error::Error;
 use std::fmt::{Debug, Display};
 use std::fs;
+#[cfg(not(unix))]
 use std::fs::File;
 use std::os::unix::fs::MetadataExt;
 #[cfg(unix)]
+use std::os::unix::fs::OpenOptionsExt;
+#[cfg(unix)]
 use std::os::unix::prelude::OsStrExt;
 use std::path::{Path, PathBuf, MAIN_SEPARATOR};
 use std::process;
@@ -750,27 +753,52 @@
 fn copy_file(from: &Path, to: &Path) -> UResult<()> {
  // fs::copy fails if destination is a invalid symlink.
  // so lets just remove all existing files at destination before copy.
- if let Err(e) = fs::remove_file(to) {
- if e.kind() != std::io::ErrorKind::NotFound {
- show_error!(
- "Failed to remove existing file {}. Error: {:?}",
- to.display(),
- e
- );
+ let remove_destination = || {
+ if let Err(e) = fs::remove_file(to) {
+ if e.kind() != std::io::ErrorKind::NotFound {
+ show_error!(
+ "Failed to remove existing file {}. Error: {:?}",
+ to.display(),
+ e
+ );
+ }
  }
+ };
+ remove_destination();
+
+ // create the destination file first. Using safer mode on unix to avoid
+ // potential unsafe mode between time-of-creation and time-of-chmod.
+ #[cfg(unix)]
+ let creation = fs::OpenOptions::new()
+ .write(true)
+ .create_new(true)
+ .mode(0o600)
+ .open(to);
+ #[cfg(not(unix))]
+ let creation = File::create(to);
+
+ if let Err(e) = creation {
+ show_error!(
+ "Failed to create destination file {}. Error: {:?}",
+ to.display(),
+ e
+ );
+ return Err(InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), e).into());
  }
 
- if from.as_os_str() == "/dev/null" {
- /* workaround a limitation of fs::copy
- * https://github.com/rust-lang/rust/issues/79390
- */
- if let Err(err) = File::create(to) {
+ // drop the file to close the fd of creation
+ drop(creation);
+
+ /* workaround a limitation of fs::copy: skip copy if source is /dev/null
+ * https://github.com/rust-lang/rust/issues/79390
+ */
+ if from.as_os_str() != "/dev/null" {
+ if let Err(err) = fs::copy(from, to) {
+ remove_destination();
  return Err(
  InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into(),
  );
  }
- } else if let Err(err) = fs::copy(from, to) {
- return Err(InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into());
  }
  Ok(())
 }

tests/by-util/test_install.rs

@@ -1717,3 +1717,41 @@ fn test_install_root_combined() {
  run_and_check(&["-Cv", "c", "d"], "d", 0, 0);
  run_and_check(&["-Cv", "c", "d"], "d", 0, 0);
 }
+
+#[cfg(all(unix, feature = "chmod"))]
+#[test]
+fn test_install_copy_failures() {
+ let scene = TestScenario::new(util_name!());
+
+ let at = &scene.fixtures;
+
+ let file1 = "source_file";
+ let file2 = "target_file";
+
+ at.touch(file1);
+ scene.ccmd("chmod").arg("000").arg(file1).succeeds();
+
+ // if source file is not readable, it will raise a permission error.
+ // since we create the file with mode 0600 before `fs::copy`, if the
+ // copy failed, the file should be removed.
+ scene
+ .ucmd()
+ .arg(file1)
+ .arg(file2)
+ .arg("--mode=400")
+ .fails()
+ .stderr_contains("permission denied");
+ assert!(!at.file_exists(file2));
+
+ // if source file is good to copy, it should succeed and set the
+ // destination file permissions accordingly
+ scene.ccmd("chmod").arg("644").arg(file1).succeeds();
+ scene
+ .ucmd()
+ .arg(file1)
+ .arg(file2)
+ .arg("--mode=400")
+ .succeeds();
+ assert!(at.file_exists(file2));
+ assert_eq!(0o100_400_u32, at.metadata(file2).permissions().mode());
+}