Skip to content

Commit

Permalink
fs::copy() set file mode early
Browse files Browse the repository at this point in the history 
A convenience method like fs::copy() should try to prevent pitfalls a
normal user doesn't think about.

In case of an empty umask, setting the file mode early prevents
temporarily world readable or even writeable files,
because the default mode is 0o666.

In case the target is a named pipe or special device node, setting the
file mode can lead to unwanted side effects, like setting permissons on
`/dev/stdout` or for root setting permissions on `/dev/null`.

copy_file_range() returns EINVAL, if the destination is a FIFO/pipe or
a device like "/dev/null", so fallback to io::copy, too.

Use `fcopyfile` on MacOS instead of `copyfile`.

Fixes: #26933
Fixed: #37885
  • Loading branch information
@haraldh
haraldh committed Mar 23, 2019
1 parent 7cf074a commit cf8347b
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 59 deletions.
129 changes: 74 additions & 55 deletions src/libstd/sys/unix/fs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -827,30 +827,54 @@ pub fn canonicalize(p: &Path) -> io::Result<PathBuf> {
Ok(PathBuf::from(OsString::from_vec(buf)))
}

fn open_and_set_permissions(
from: &Path,
to: &Path,
) -> io::Result<(crate::fs::File, crate::fs::File, u64, crate::fs::Metadata)> {
use crate::fs::{File, OpenOptions};
use crate::os::unix::fs::{OpenOptionsExt, PermissionsExt};

let reader = File::open(from)?;
let (perm, len) = {
let metadata = reader.metadata()?;
if !metadata.is_file() {
return Err(Error::new(
ErrorKind::InvalidInput,
"the source path is not an existing regular file",
));
}
(metadata.permissions(), metadata.len())
};
let writer = OpenOptions::new()
// create the file with the correct mode right away
.mode(perm.mode())
.write(true)
.create(true)
.truncate(true)
.open(to)?;
let writer_metadata = writer.metadata()?;
if writer_metadata.is_file() {
// Set the correct file permissions, in case the file already existed.
// Don't set the permissions on already existing non-files like
// pipes/FIFOs or device nodes.
writer.set_permissions(perm)?;
}
Ok((reader, writer, len, writer_metadata))
}

#[cfg(not(any(target_os = "linux",
target_os = "android",
target_os = "macos",
target_os = "ios")))]
pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
use crate::fs::File;
if !from.is_file() {
return Err(Error::new(ErrorKind::InvalidInput,
"the source path is not an existing regular file"))
}
let (mut reader, mut writer, _, _) = open_and_set_permissions(from, to)?;

let mut reader = File::open(from)?;
let mut writer = File::create(to)?;
let perm = reader.metadata()?.permissions();

let ret = io::copy(&mut reader, &mut writer)?;
writer.set_permissions(perm)?;
Ok(ret)
io::copy(&mut reader, &mut writer)
}

#[cfg(any(target_os = "linux", target_os = "android"))]
pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
use crate::cmp;
use crate::fs::File;
use crate::sync::atomic::{AtomicBool, Ordering};

// Kernel prior to 4.5 don't have copy_file_range
Expand All @@ -876,17 +900,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
)
}

if !from.is_file() {
return Err(Error::new(ErrorKind::InvalidInput,
"the source path is not an existing regular file"))
}

let mut reader = File::open(from)?;
let mut writer = File::create(to)?;
let (perm, len) = {
let metadata = reader.metadata()?;
(metadata.permissions(), metadata.size())
};
let (mut reader, mut writer, len, _) = open_and_set_permissions(from, to)?;

let has_copy_file_range = HAS_COPY_FILE_RANGE.load(Ordering::Relaxed);
let mut written = 0u64;
Expand All @@ -896,13 +910,14 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
let copy_result = unsafe {
// We actually don't have to adjust the offsets,
// because copy_file_range adjusts the file offset automatically
cvt(copy_file_range(reader.as_raw_fd(),
ptr::null_mut(),
writer.as_raw_fd(),
ptr::null_mut(),
bytes_to_copy,
0)
)
cvt(copy_file_range(
reader.as_raw_fd(),
ptr::null_mut(),
writer.as_raw_fd(),
ptr::null_mut(),
bytes_to_copy,
0,
))
};
if let Err(ref copy_err) = copy_result {
match copy_err.raw_os_error() {
Expand All @@ -920,24 +935,25 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
Ok(ret) => written += ret as u64,
Err(err) => {
match err.raw_os_error() {
Some(os_err) if os_err == libc::ENOSYS
|| os_err == libc::EXDEV
|| os_err == libc::EPERM => {
// Try fallback io::copy if either:
// - Kernel version is < 4.5 (ENOSYS)
// - Files are mounted on different fs (EXDEV)
// - copy_file_range is disallowed, for example by seccomp (EPERM)
assert_eq!(written, 0);
let ret = io::copy(&mut reader, &mut writer)?;
writer.set_permissions(perm)?;
return Ok(ret)
},
Some(os_err)
if os_err == libc::ENOSYS
|| os_err == libc::EXDEV
|| os_err == libc::EINVAL
|| os_err == libc::EPERM =>
{
// Try fallback io::copy if either:
// - Kernel version is < 4.5 (ENOSYS)
// - Files are mounted on different fs (EXDEV)
// - copy_file_range is disallowed, for example by seccomp (EPERM)
// - copy_file_range cannot be used with pipes or device nodes (EINVAL)
assert_eq!(written, 0);
return io::copy(&mut reader, &mut writer);
}
_ => return Err(err),
}
}
}
}
writer.set_permissions(perm)?;
Ok(written)
}

Expand All @@ -960,9 +976,9 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
type copyfile_flags_t = u32;

extern "C" {
fn copyfile(
from: *const libc::c_char,
to: *const libc::c_char,
fn fcopyfile(
from: libc::c_int,
to: libc::c_int,
state: copyfile_state_t,
flags: copyfile_flags_t,
) -> libc::c_int;
Expand All @@ -988,10 +1004,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
}
}

if !from.is_file() {
return Err(Error::new(ErrorKind::InvalidInput,
"the source path is not an existing regular file"))
}
let (reader, writer, _, writer_metadata) = open_and_set_permissions(from, to)?;

// We ensure that `FreeOnDrop` never contains a null pointer so it is
// always safe to call `copyfile_state_free`
Expand All @@ -1003,12 +1016,18 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
FreeOnDrop(state)
};

let flags = if writer_metadata.is_file() {
COPYFILE_ALL
} else {
COPYFILE_DATA
};

cvt(unsafe {
copyfile(
cstr(from)?.as_ptr(),
cstr(to)?.as_ptr(),
fcopyfile(
reader.as_raw_fd(),
writer.as_raw_fd(),
state.0,
COPYFILE_ALL,
flags,
)
})?;

Expand Down
13 changes: 9 additions & 4 deletions src/test/run-pass/paths-containing-nul.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![allow(deprecated)]
// ignore-cloudabi no files or I/O
// ignore-wasm32-bare no files or I/O
// ignore-emscripten no files

use std::fs;
use std::io;
Expand All @@ -22,14 +23,18 @@ fn main() {
assert_invalid_input("remove_file", fs::remove_file("\0"));
assert_invalid_input("metadata", fs::metadata("\0"));
assert_invalid_input("symlink_metadata", fs::symlink_metadata("\0"));

// If `dummy_file` does not exist, then we might get another unrelated error
let dummy_file = std::env::current_exe().unwrap();

assert_invalid_input("rename1", fs::rename("\0", "a"));
assert_invalid_input("rename2", fs::rename("a", "\0"));
assert_invalid_input("rename2", fs::rename(&dummy_file, "\0"));
assert_invalid_input("copy1", fs::copy("\0", "a"));
assert_invalid_input("copy2", fs::copy("a", "\0"));
assert_invalid_input("copy2", fs::copy(&dummy_file, "\0"));
assert_invalid_input("hard_link1", fs::hard_link("\0", "a"));
assert_invalid_input("hard_link2", fs::hard_link("a", "\0"));
assert_invalid_input("hard_link2", fs::hard_link(&dummy_file, "\0"));
assert_invalid_input("soft_link1", fs::soft_link("\0", "a"));
assert_invalid_input("soft_link2", fs::soft_link("a", "\0"));
assert_invalid_input("soft_link2", fs::soft_link(&dummy_file, "\0"));
assert_invalid_input("read_link", fs::read_link("\0"));
assert_invalid_input("canonicalize", fs::canonicalize("\0"));
assert_invalid_input("create_dir", fs::create_dir("\0"));
Expand Down

0 comments on commit cf8347b

Please sign in to comment.