`std::fs::copy` sets permissions too late

[ben0x539]

std::fs::copy makes a new file, reads permissions of the old file, copies contents of the old file to the new file, and then sets permissions of the new file to those it just read. If used to copy a file that is only readable to the current user into a public directory, there's an opportunity for another user to get ahold of the just-created file before permissions are set and read all the secret data. I think it needs to create the file with the right permissions to begin with.

[Thiez]

How would this work when copying a file that has no write permissions for anyone? :)

[Diggsey]

It will also fail to preserve any additional metadata or alternate data streams attached to the file. The only reliable way I could find of doing this on *nix was to invoke cp which is rather unfortunate...

[Stebalien]

@Diggsey cp also copies permissions after copying the data (so does python incidentally).

However, cp will try to create the target file with the source's mode if it doesn't exist (good). Unfortunately, even if you copy the permissions first, anyone who has the file open will still be able to read it (at least on linux) and, as ACLs can actually remove access to a file, creating the destination file with the source file's mode isn't actually secure. The secure way to do this is to (on linux):

1. Create a temporary file with mode 0. (or 0600 but 0 is simpler)
2. Copy the ACLs. Fail if this doesn't work.
3. Copy the mode. Fail if this doesn't work.
4. rename the temporary file over the target file.
5. Copy the data.

Unfortunately,

1. The whole "create a temporary file and rename" part differs significantly from what cp does and won't work if the user doesn't have write permissions on the target directory (while overwriting the target file might).
2. Copying a file from a filesystem that supports ACLs to a file system that doesn't may fail unexpectedly.

cp specification if anyone is interested: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/cp.html

[nagisa]

How would this work when copying a file that has no write permissions for anyone? :)

Open a file for writing and change permissions before writing anything. While there’s still a window for a malicious party to open the file, it is much smaller.

[Stebalien]

Open a file for writing and change permissions before writing anything.

1. If the file already exists you should just fail and return an error instead of trying to overwrite a read-only file.
2. (On linux, I don't know about windows) If the file doesn't exist, it doesn't matter what mode you create the file with, you will always be able to write to it (as long as you open it for writing):

use std::fs::File;
use std::fs::OpenOptions;
use std::os::unix::fs::OpenOptionsExt;
use std::io::Write;

fn main() {
    let mut f = OpenOptions::new().write(true).create(true).truncate(true).mode(0).open("/tmp/my_f").unwrap();
    f.write(b"test str");
    f.flush();
}

While there’s still a window for a malicious party to open the file, it is much smaller.

<ta mode>
Forget this entire concept; this is not how computer security works. A small window just means that the attacker might have to try a couple of times or, even better, spin up a hundred threads trying to open the file in while loops. You don't minimize races, you eliminate them. I can't express how important this is not only in computer system security but computer systems in general. If something can go wrong, it probably will; if it can go wrong often enough, it definitely will.
</ta mode>

Regardless, in this case this discussion is moot assuming you only make the file writable to the current user because the attacker would have to run as the current user to exploit this (at which point they could do just about anything...).

[Diggsey]

This isn't an issue on windows because it provides CopyFile, CopyFileEx which do the job properly, including preserving metadata and alternate data streams.

[retep998]

@Diggsey We actually do use CopyFileEx on Windows as of #26751, so this is purely a non-Windows issue now.

[steveklabnik]

/cc @rust-lang/libs