sort: create temporary files private to the owner

src/uu/sort/Cargo.toml

@@ -35,6 +35,7 @@ thiserror = { workspace = true }
 uucore = { workspace = true, features = [
   "fs",
   "parser-size",
+  "safe-copy",
   "version-cmp",
   "i18n-decimal",
   "i18n-collator",

src/uu/sort/src/merge.rs

@@ -16,7 +16,7 @@ use std::{
     collections::BinaryHeap,
     ffi::{OsStr, OsString},
     fs::{self, File},
-    io::{BufWriter, Read, Write},
+    io::{self, BufWriter, Read, Write},
     iter,
     path::{Path, PathBuf},
     process::{Child, ChildStdin, ChildStdout, Command, Stdio},
@@ -51,8 +51,13 @@ fn replace_output_file_in_input_files(
                 if let Some(copy) = &copy {
                     *file = copy.clone().into_os_string();
                 } else {
-                    let (_file, copy_path) = tmp_dir.next_file()?;
-                    fs::copy(&output_path, &copy_path)
+                    // Write through the descriptor `next_file` just opened rather
+                    // than `fs::copy`, which would put the source's permission bits
+                    // on the temporary file and undo the 0600 it was created with.
+                    let (mut copy_file, copy_path) = tmp_dir.next_file()?;
+                    let mut source = File::open(&output_path)
+                        .map_err(|error| SortError::OpenTmpFileFailed { error })?;
+                    io::copy(&mut source, &mut copy_file)
                         .map_err(|error| SortError::OpenTmpFileFailed { error })?;
                     *file = copy_path.clone().into_os_string();
                     copy = Some(copy_path);
@@ -387,7 +392,7 @@ impl FileMerger<'_> {
         &mut self,
         writer: &mut impl Write,
         settings: &GlobalSettings,
-    ) -> std::io::Result<bool> {
+    ) -> io::Result<bool> {
         if let Some(file) = self.heap.peek() {
             let prev = self.prev.replace(PreviousLine {
                 chunk: file.current_chunk.clone(),
@@ -639,3 +644,33 @@ impl<R: Read + Send> MergeInput for PlainMergeInput<R> {
         &mut self.inner
     }
 }
+
+#[cfg(all(test, unix))]
+mod tests {
+    use super::*;
+    use std::os::unix::fs::PermissionsExt;
+
+    /// When the output file is also an input it is copied to a temporary file
+    /// first. That copy must stay private to the owner: `fs::copy` would carry the
+    /// source's 0644 over and undo what `next_file` created the file with.
+    #[test]
+    fn output_copy_keeps_the_tmp_file_private() {
+        let dir = tempfile::tempdir().unwrap();
+        let out = dir.path().join("out");
+        fs::write(&out, b"a\n").unwrap();
+        fs::set_permissions(&out, fs::Permissions::from_mode(0o644)).unwrap();
+
+        let mut tmp_dir = TmpDirWrapper::new(dir.path().to_owned());
+        let mut files = vec![out.clone().into_os_string()];
+        replace_output_file_in_input_files(&mut files, Some(out.as_os_str()), &mut tmp_dir)
+            .unwrap();
+
+        let copy = Path::new(&files[0]);
+        assert_ne!(copy, out, "the input was not replaced by a copy");
+        assert_eq!(
+            fs::metadata(copy).unwrap().permissions().mode() & 0o777,
+            0o600
+        );
+        assert_eq!(fs::read(copy).unwrap(), b"a\n");
+    }
+}

src/uu/sort/src/tmp_dir.rs

@@ -13,6 +13,9 @@ use std::{
     sync::{Arc, LazyLock, Mutex},
 };
 
+#[cfg(unix)]
+use std::{fs::Permissions, os::unix::fs::PermissionsExt};
+
 use tempfile::TempDir;
 use uucore::error::UResult;
 #[cfg(not(any(target_os = "redox", target_os = "wasi")))]
@@ -112,14 +115,18 @@ impl TmpDirWrapper {
     fn init_tmp_dir(&mut self) -> UResult<()> {
         assert!(self.temp_dir.is_none());
         assert_eq!(self.size, 0);
-        self.temp_dir = Some(
-            tempfile::Builder::new()
-                .prefix("uutils_sort")
-                .tempdir_in(&self.parent_path)
-                .map_err(|_| SortError::TmpFileCreationFailed {
-                    path: self.parent_path.clone(),
-                })?,
-        );
+        // The chunks hold the whole input, so keep them out of reach of other
+        // local users instead of leaving the mode to the umask. GNU sort (9.11)
+        // creates its temporaries 0600 whatever the umask.
+        let mut builder = tempfile::Builder::new();
+        builder.prefix("uutils_sort");
+        #[cfg(unix)]
+        builder.permissions(Permissions::from_mode(0o700));
+        self.temp_dir = Some(builder.tempdir_in(&self.parent_path).map_err(|_| {
+            SortError::TmpFileCreationFailed {
+                path: self.parent_path.clone(),
+            }
+        })?);
 
         let path = self.temp_dir.as_ref().unwrap().path().to_owned();
         let state = HANDLER_STATE.clone();
@@ -145,8 +152,16 @@ impl TmpDirWrapper {
         let file_name = self.size.to_string();
         self.size += 1;
         let path = self.temp_dir.as_ref().unwrap().path().join(file_name);
+        // Only the owner may read a chunk. `nofollow` because nothing should
+        // exist at the path yet: a symlink there is hostile, not something to
+        // write through.
+        #[cfg(unix)]
+        let file = uucore::safe_copy::create_dest_restrictive(&path, true);
+        #[cfg(not(unix))]
+        let file = File::create(&path);
+
         Ok((
-            File::create(&path).map_err(|error| SortError::OpenTmpFileFailed { error })?,
+            file.map_err(|error| SortError::OpenTmpFileFailed { error })?,
             path,
         ))
     }
@@ -195,3 +210,46 @@ fn remove_tmp_dir(path: &Path) -> std::io::Result<()> {
     }
     std::fs::remove_dir(path)
 }
+
+#[cfg(all(test, unix))]
+mod tests {
+    use super::TmpDirWrapper;
+    use std::os::unix::fs::PermissionsExt;
+
+    fn mode(path: &std::path::Path) -> u32 {
+        std::fs::metadata(path).unwrap().permissions().mode() & 0o777
+    }
+
+    /// Restores the process umask on drop, so a panic in the test cannot leak the
+    /// value into the rest of the binary.
+    struct UmaskGuard(libc::mode_t);
+
+    impl UmaskGuard {
+        fn set(mask: libc::mode_t) -> Self {
+            // SAFETY: umask(2) has no failure mode; it returns the previous value.
+            Self(unsafe { libc::umask(mask) })
+        }
+    }
+
+    impl Drop for UmaskGuard {
+        fn drop(&mut self) {
+            unsafe { libc::umask(self.0) };
+        }
+    }
+
+    #[test]
+    fn tmp_files_are_private_regardless_of_umask() {
+        // Pin a permissive umask: under 0077 the umask alone would produce 0700 and
+        // 0600, so the assertions would hold for a broken implementation too. The
+        // guard restores it, and the only other test here that creates files sets
+        // the modes it cares about explicitly.
+        let _umask = UmaskGuard::set(0o022);
+
+        let parent = tempfile::tempdir().unwrap();
+        let mut wrapper = TmpDirWrapper::new(parent.path().to_owned());
+        let (_file, path) = wrapper.next_file().unwrap();
+
+        assert_eq!(mode(path.parent().unwrap()), 0o700);
+        assert_eq!(mode(&path), 0o600);
+    }
+}

src/uucore/Cargo.toml

@@ -183,7 +183,7 @@ proc-info = ["tty", "walkdir"]
 quoting-style = ["i18n-common"]
 ranges = []
 ringbuffer = []
-safe-copy = []
+safe-copy = ["rustix/fs"]
 safe-traversal = ["libc", "nix/fs", "nix/dir", "nix/user"]
 selinux = ["dep:selinux"]
 smack = ["xattr"]