Set `TMPDIR` for build scripts and `rustc`

[madsmtm]

Problem

A while back, I had to debug the way the compiler is linking #[used] symbols on macOS (the symbols.o trick, see rust-lang/rust#133832 for the specific issue). This was made harder by the fact that the file is a temporary file that the compiler saved to the user-local temporary directory which looks something like /var/folders/x2/ngp6bLf575g4w4g3ctlqt5140000gn/T/ on macOS (took me a while to even understand what that directory was in the first place tbh).

I managed to narrow things down using -Csave-temps=yes, which made the compiler at least not delete the temporary file, but it was still more difficult to work with than it had to be.

If the compiler had put this file somewhere under the build directory (target), it would've been easier for me to find and work with. I suspect the same is true for other temporary files that the compiler (or linker) produces.

---

Additionally, I'm working on sandboxing build scripts and procedural macros, and there's sort of a dual issue with allowing access to /tmp (or /var/folders/x2/ngp6bLf575g4w4g3ctlqt5140000gn/T/):

• We don't want people to put anything important in /tmp, because untrusted (but sandboxed) code running under the same user will be able to read and modify it. Currently the compiler does this, e.g. symbols.o is vulnerable to a MITM attack where a process replaces it with a malicious object file, and thereby injects arbitrary code into whatever the user was compiling.
• We don't want malicious build scripts and proc-macros to get access to /tmp, because software elsewhere (such as the user's browser) might unknowingly store sensitive data in there.

Note: I'm not filing this as a security advisory, as things like Xcode have the same problem - nobody does trust boundaries in this way, the trust boundary the OS provides is between users. It's still annoying for cargo-sandbox's use-case though (because it wants to redefine where that trust boundary lies).

Proposed Solution

Set TMPDIR on Unixes and TMP+TEMP on Windows, such that code running at build time that wants to access stuff in a temporary directory will do so inside the build directory. I have put up a PR in #16426 for that.

Note that there's nothing stopping someone from reading or writing to /tmp (unless you enable some sort of sandboxing or jailing); this environment variable is only a hint for well-behaved programs.

We could consider passing -Csave-temps=yes to the compiler in the future, though I'm unsure of how to deal with unbounded growth of the target directory? Though that's already a problem IIUC?

Notes

This would also make it easier to clean up after compilation is finished, as cargo clean would make sure to remove temporary files as well. rustc generally cleans up any temporary files it uses, but if it segfaults, it will not get the chance to. The same is true for the linker and C compilers running in build scripts etc.

If we were to do the same this when running integration tests as well, the need for special-casing those with CARGO_TARGET_TMPDIR (introduced in #9375) would probably disappear.

See also rust-lang/rust#138475 and #4350.

[madsmtm]

@rustbot label A-build-execution A-build-scripts A-security A-debugging

[ChrisDenton]

This was made harder by the fact that the file is a temporary file that the compiler saved to the user-local temporary directory

This sounds like a compiler issue tbh.

[madsmtm]

This was made harder by the fact that the file is a temporary file that the compiler saved to the user-local temporary directory

This sounds like a compiler issue tbh.

Probably true, but it's not one that I know how to solve without exposing the output location of the symbols.o file via some flag? Which is problematic, because we aren't really settled on that design. Besides, a quick look reveals that tempfile is used in ~10 different places in the compiler, all of these would similarly have to be assessed to avoid similar problems in the future.

And again, the linker, build scripts and proc macros also suffer from this.

[epage]

If you are wrapping Cargo, what benefit is there for cargo to override these than you too?

What went into the design of CARGO_TARGET_TMPDIR? e.g. was this considered and rejected?

We also have several new and upcoming features that need to be kept in mind

• build-dir vs target-dir
• old build-dir layout vs new
• shared caches, requiring the new build-dir layout's unit dir to be a cacheable unit
• granular locking

[epage]

In #9661 (comment), the idea is brought up for a build script scratch space. cc uses OUT_DIR for this but do we want to have that in the cacheable unit?

CC @ranger-ross