Repository navigation
Relative path overrides are still possible via /proc/self #3461
Description
Relative path overrides permit a freshly downloaded source tree to
execute arbitrary code on any rustup command that executes a binary from
the configured toolchain, and its a reasonable tradeoff for us to remove
this feature. Absolute path overrides are kept intact - these were added
to support users of large monorepo tool systems, and can be kept with
reasonable safety.
I think relative paths are a subset of absolute paths, so, security-wise, this is a no-op.
Namely, ./foo/bar relative path is equivalent to /proc/self/cwd/foo/bar absolute path.
Originally posted by @matklad in #3340 (comment)
Reacted by Jon Gjengset
Activity
rbtcollins commentedon Aug 22, 2023
I can see three paths forward:
matklad commentedon Aug 22, 2023
I guess, security wise there's also an angle that the attacker could just guess absolute paths? Like, it's pretty obvious, if one stalks me, that I clone stuff into either
/home/matklad/por/home/matklad/tmp, so, if someone delibirately targets me, they potentially could just hard-code those absolute paths.So I think 3) is a fundamentally incomplete approach.
On the meta level, probably makes sense to ask some relevant security expert in the rust ecosystem for what's best practice here?
djc commentedon Aug 22, 2023
Option (2) sounds good to me, too.
@walterhpearce / @LawnGnome, we'd like your input here!
jonhoo commentedon Aug 22, 2023
cc @lovesegfault who currently maintains the build tooling at AWS given I've since left :)
lovesegfault commentedon Aug 22, 2023
Thanks for the tag, Jon!
Regarding (2), I'm not sure what "the separate trusted directory functionality" is referring to, so it's hard to opine.
I can say that (1) would cause massive churn at AWS, and I'd have to drop just about everything to re-think how we use Rustup. It could lead to us maintaining a fork of Rustup, which I'd like to avoid at all costs.
(3) sounds like "drying ice", but sometimes that's all you can really do, I don't know if that's the case here.
LawnGnome commentedon Aug 22, 2023
(@walterhpearce and I discussed this earlier, but I'm speaking for myself here. I suspect he'll be by with an opinion at some point too.)
I don't see a lot of value in trying to distinguish relative and absolute paths here, honestly — while it might make some non-targeted attacks slightly more difficult, it doesn't help with targeted attacks, and I suspect a creative attacker could still come up with some pretty damaging non-targeted attacks without much effort, even if we try to whack the moles of things like
/proc/selfwith option 3.It sounds like option 1 is also off the table given the monorepo use case.
So, given that, option 2 (including a rollback of the recent relative path change) feels right to me. Am I right in thinking that the "trusted directory" functionality hasn't yet been implemented or designed in any serious way? I think this is personally how I'd want the check to work:
rustupchecks the canonical path of every binary it's going to execute against a prefix allowlist before executing it.rustupuses is only~/.rustup/toolchains(allowing for platform specific differences) and maybe whatever is required for distro-shipped toolchains. (I'm ignorant of the details there. Could that be merged in from/etc/rustup/settings.toml?)rustup toolchain linkwould add that prefix to the allowlist by default.Does that seem reasonable at first blush? I won't pretend to be an expert on all the complications that I'm sure exist with supporting
rustupacross the variety of environments that are out there, but it seems like that would handle the use cases I know of and that have been articulated here.lovesegfault commentedon Aug 22, 2023
@LawnGnome As stated, option (2) would be perfectly workable on our end, though the devil's always in the details.
I'm strongly in favor of rolling back the recent relative path work as well, regardless of the specifics of (2)'s design.
rbtcollins commentedon Jan 22, 2024
We could also exclude the path to the dir, so the toolchain must always be provided in a separate tree ... and then you'll get git repos with
+
+- attractive-dir-to-be-lured-into
So yeah, I think 3 going to be fragile and unsustainable.
rbtcollins commentedon Jan 22, 2024
Theres an RFC for it in cargo, that also speaks to rustup. A lot of design work, and last I recall I had raised some concerns/questions.
And yes, I'd consider any form of configuration a variant of (2).
On the question of rolling back relative-path support - let my hindbrain think about it please, but do feel free to articulate the value proposition for it.