std::fs::write is not atomic - document it explicitly or make it atomic

[Kixunil]

By not being atomic, std::fs::write() may be a footgun for people trying to create correct/secure code.
As an example, nodejs has a function with same behavior that led to a security risk. In that case creation of the secret token file succeeded but writing to it failed leaving an empty token file which caused application to accept empty authentication token later.

The documentation currently kind of describes the behavior but the implications may not be obvious to people. Adding a note about the risk of empty/corrupted file being left in case of failure could help people avoid issues.

If atomic behavior is undesired due to compatibility reasons, then maybe add another atomic_write() function and suggest it in write() doc. This could be done in a crate obviously but maybe the security/robustness benefits are a good reason to put it into std.

I'm willing to make a PR regardless of the conclusion.

[the8472]

By not being atomic, std::fs::write() may be a footgun for people trying to create correct/secure code.

That isn't limited to fs::write though. Several of the convenience methods shouldn't be used in security-sensitive contexts since they would suffer from lack of atomicity or TOCTOU races. In some contexts even path traversal isn't safe by default.

then maybe add another atomic_write() function

Several crates for atomic file operations exist on crates.io.

Note that the usual atomic file creation dances have a cost, they keep more data on the disk until the operation is complete, which could be an issue on constrained systems. I'm not saying this concern would be a blocker for making fs::write atomic, but it's not without downsides. Additionally the atomic file dances can occasionally leave you with stranded temporary files on the filesystem.

[Kixunil]

@the8472 very good points and I agree 100%. I didn't intend to dismiss those. My idea is to make std more helpful somehow. Whether it's by making operations safer or documentation more explicit is up for discussion.

If I was confident that there's one true solution, I'd just open a PR, not an issue. :)

Several crates for atomic file operations exist on crates.io.

Could you recommend some you're experienced with?

[the8472]

My idea is to make std more helpful somehow. Whether it's by making operations safer or documentation more explicit is up for discussion.

I think improving the documentation would be fine. There just is that niggling doubt that we can never do enough to document all possible pitfalls for all security-critical use-cases. This could basically fill a book.

Could you recommend some you're experienced with?

I don't want to endorse anything. tempfile and atomicwrites both look half-decent in different ways (at least on unix systems) but each lacks some things I would expect from a maximally paranoid implementation. Each already has issues/PRs open about improving the state the things.

[Kixunil]

This could basically fill a book.

I hope it's not that bad. :D

Thanks for pointers!

[the8472]

It's that bad if we're talking about using filesystem APIs in security contexts in general. Atomically and durably overwriting a file in a potentially attacker-controlled directory is a bit smaller in scope, it would merely take up several blog articles or a chapter in the book.

• https://danluu.com/file-consistency/
• https://lwn.net/Articles/752063/
• https://stackoverflow.com/questions/43839309
• https://lwn.net/Articles/767547/
• fs: copy: Use File::set_permissions instead of fs::set_permissions #51213
• fs::copy() unix: set file mode early #58803
• ...

And that's just about linux stuff. Other OSes bring their own complications.

[thomcc]

I would be very opposed to making std::fs::write attempt to be atomic by default. There's no general way of doing that that doesn't come with its own drawbacks, especially when used in publicly accessible directories (Apple's "Secure Coding Guide" contains documentation warning against this).

And this isn't getting into the fact that we can't really make it atomic in the face of crashes in many situations. (To be clear: in general it is that bad)

[the8472]

On platforms that have O_TMPFILE and *at syscalls it is possible, but google tells me that macos does not have the former.

[Kixunil]

especially when used in publicly accessible directories

If you use non-atomic write there you're already screwed.

But intuitively, I agree that documenting is better than making it atomic.