Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fs::copy is racy on unix #37885

Closed
Stebalien opened this issue Nov 19, 2016 · 0 comments
Closed

fs::copy is racy on unix #37885

Stebalien opened this issue Nov 19, 2016 · 0 comments
Labels
C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Stebalien
Copy link
Contributor

  1. It checks if the source file is a regular file by path before opening it. The file could be replaced between checking and opening.
  2. It sets the permissions by path using chmod instead of by file descriptor using fchmod. It could therefore end up setting permissions on the wrong file.
@Stebalien Stebalien mentioned this issue Nov 19, 2016
Merged
bors added a commit that referenced this issue Nov 22, 2016
@bors
Auto merge of #37886 - Stebalien:set-perm, r=alexcrichton
510d8cc 
Add a method for setting permissions directly on an open file.

On unix like systems, the underlying file corresponding to any given path may change at any time. This function makes it possible to set the permissions of the a file corresponding to a `File` object even if its path changes.

@retep998, what's the best way to do this on Windows? I looked into `SetFileInformationByHandle` but couldn't find a way to do it atomically risking clobbering access time information.

This is a first step towards fixing #37885. This function doesn't *have* to be public but this is useful functionality that should probably be exposed.
bors added a commit that referenced this issue Nov 23, 2016
@bors
Auto merge of #37886 - Stebalien:set-perm, r=alexcrichton
ccdc26f 
Add a method for setting permissions directly on an open file.

On unix like systems, the underlying file corresponding to any given path may change at any time. This function makes it possible to set the permissions of the a file corresponding to a `File` object even if its path changes.

@retep998, what's the best way to do this on Windows? I looked into `SetFileInformationByHandle` but couldn't find a way to do it atomically risking clobbering access time information.

This is a first step towards fixing #37885. This function doesn't *have* to be public but this is useful functionality that should probably be exposed.
@steveklabnik steveklabnik added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed A-libs labels Mar 24, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 26, 2017
@haraldh haraldh mentioned this issue Feb 22, 2019
Closed
@haraldh haraldh mentioned this issue Feb 22, 2019
Closed
haraldh added a commit to haraldh/rust-1 that referenced this issue Feb 22, 2019
@haraldh
fs::copy() linux: handle sparse files and set file mode early
81118c3 
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`.

Not handling sparse files could fill up the users disk very quickly.

Fixes:
rust-lang#26933
rust-lang#37885
rust-lang#58635
@haraldh haraldh mentioned this issue Feb 28, 2019
Merged
haraldh added a commit to haraldh/rust-1 that referenced this issue Mar 1, 2019
@haraldh
fs::copy() linux: set file mode early
fb98ca7 
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.

Fixes: rust-lang#26933
Fixed: rust-lang#37885
haraldh added a commit to haraldh/rust-1 that referenced this issue Mar 5, 2019
@haraldh
fs::copy() set file mode early
5cf5832 
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.

Fixes: rust-lang#26933
Fixed: rust-lang#37885
Centril added a commit to Centril/rust that referenced this issue Mar 10, 2019
@Centril
Rollup merge of rust-lang#58803 - haraldh:fs_copy_fix, r=alexcrichton
8931a2b 
fs::copy() linux: set file mode early

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.

Fixes: rust-lang#26933
Fixed: rust-lang#37885
Centril added a commit to Centril/rust that referenced this issue Mar 10, 2019
@Centril
Rollup merge of rust-lang#58803 - haraldh:fs_copy_fix, r=alexcrichton
6519db8 
fs::copy() linux: set file mode early

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.

Fixes: rust-lang#26933
Fixed: rust-lang#37885
Centril added a commit to Centril/rust that referenced this issue Mar 10, 2019
@Centril
Rollup merge of rust-lang#58803 - haraldh:fs_copy_fix, r=alexcrichton
1dae14b 
fs::copy() linux: set file mode early

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.

Fixes: rust-lang#26933
Fixed: rust-lang#37885
Centril added a commit to Centril/rust that referenced this issue Mar 10, 2019
@Centril
Rollup merge of rust-lang#58803 - haraldh:fs_copy_fix, r=alexcrichton
935602a 
fs::copy() linux: set file mode early

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.

Fixes: rust-lang#26933
Fixed: rust-lang#37885
kennytm added a commit to kennytm/rust that referenced this issue Mar 11, 2019
@kennytm
Rollup merge of rust-lang#58803 - haraldh:fs_copy_fix, r=alexcrichton
58a0e14 
fs::copy() linux: set file mode early

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.

Fixes: rust-lang#26933
Fixed: rust-lang#37885
pietroalbini added a commit to pietroalbini/rust that referenced this issue Mar 12, 2019
@pietroalbini
Rollup merge of rust-lang#58803 - haraldh:fs_copy_fix, r=alexcrichton
5bce584 
fs::copy() unix: set file mode early

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.

Fixes: rust-lang#26933
Fixed: rust-lang#37885
kennytm added a commit to kennytm/rust that referenced this issue Mar 15, 2019
@kennytm
Rollup merge of rust-lang#58803 - haraldh:fs_copy_fix, r=alexcrichton
ad8e2b1 
fs::copy() unix: set file mode early

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.

Fixes: rust-lang#26933
Fixed: rust-lang#37885
haraldh added a commit to haraldh/rust-1 that referenced this issue Mar 23, 2019
@haraldh
fs::copy() set file mode early
cf8347b 
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: rust-lang#26933
Fixed: rust-lang#37885
Centril added a commit to Centril/rust that referenced this issue Mar 28, 2019
@Centril
Rollup merge of rust-lang#58803 - haraldh:fs_copy_fix, r=alexcrichton
83d91d3 
fs::copy() unix: set file mode early

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.

Fixes: rust-lang#26933
Fixed: rust-lang#37885
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@steveklabnik @Stebalien @Mark-Simulacrum @apasel422