diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 331a50f67..8f5d381fe 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -13,9 +13,12 @@ use filetime::{set_file_times, FileTime}; 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 perform_backup(to: &Path, b: &Behavior) -> UResult> { 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(()) } diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index f1e3302e1..6fa9cc62f 100644 --- a/tests/by-util/test_install.rs +++ b/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()); +}