The trouble with this reasoning is that the capabilities "ask editor to read file on disk and provide contents to LSP server" and "ask editor to write file on disk with contents specified by LSP server" are dual-use.[1] Their existence is not necessarily a security problem in and of itself, and they might well have been added to the protocol on the assumption that the LSP server is trusted and runs with the ambient authority of the invoking user anyway. It becomes a security problem when we introduce a sandbox, and there's something untrusted, inside the sandbox, that either is, or can masquerade as, an LSP server, to an editor outside the sandbox.
Leveraging any of these gaps from a build script either requires control of the crate, LSP and possibly IDE, or a zero-day.
I thought we were assuming the entire crate to be malicious, since that's the whole point of sandboxing the build, to protect the exterior of the sandbox from malicious code in a crate being built.
The main point I take though is that the vector repo-contents -> internet is not well covered by this case. That's certainly a member of the shortlist for "not covered".
Yeah, absolutely. I would worry about that before I worried about LSP servers puppeting editors.
to reiterate, I don't know if the LSP protocol provides LSP servers with these capabilities, but I'm assuming it does for purpose of this discussion ↩︎
Thanks for the clarification; I indeed missed the importance of the "and". But I don't think you need all three of the things on your list for the exploit I described. I think you need:
a malicious crate being built inside a sandbox
the IDE-to-LSP-server communication channel being exposed to code running inside the sandbox
an IDE that exposes "read/write arbitrary file with user ambient authority" to LSP servers (which is not technically a vulnerability in the IDE or the protocol)
If you consider the LSP server a trusted component, only containerized for ease of installation, then you can probably avoid this exploit by moving it to a separate container (that's no longer considered a sandbox), thus not needing to poke a hole in the sandbox for the LSP socket. However, then the LSP server needs to know how to run build steps inside the sandbox container. I don't know if rust-analyzer is flexible enough for that.
None of these will fully replace build scripts and proc macros. So while they are great initiatives, they do not substantially change the need for sandboxing.
And I do think it is a need. I hate how paranoid I have to be about even checking source code in an IDE or otherwise locally, and I am far from sure that my hand-crafted sandbox setup is totally airtight...
When most people talk about sandboxing, it comes across as the more general problem of "dangerous things might run on my system". I think it was this thread is the first I heard of the more specific use case of "I want to review the code before running it unprotected".
A potential alternative to sandboxing for proc-macros is build-std where we could statically determine that a proc-macro is alloc-only (yes, something would need to be figured out for FFI).
Note that for either solution, last I heard from project security folks, they do not want to officially consider the compiler to be a security barrier against untrusted code.
There will be proc-macros that won't fit into either solution and there are still some build scripts after efforts of reducing their use. The scope of what is left is up for debate for how much we allow for exposing through sandboxing. At #t-cargo > build script and proc macro allow list @ , I have started exploring the idea of allow lists for the remaining proc macros as well as build scripts. One idea there for build scripts is package authors information cargo when a build script is optional (e.g. some view anyhow build script this way), reducing the amount of build scripts to be allowlisted even further.
Happy to register this as a use-case then.
I might not want to run (most of) it at all, but just understand it while debugging a crater regression or a Miri report or something like that -- to then copy-paste the relevant parts and run those.
Yeah, I don't think this is merely a library problem, you'd need to ban extern and link and inline asm at the very least. "This crate does nothing beyond what alloc can do" is a full language dialect of enforcement.
You'd almost surely need to ban all unsafe code. If OS-level protections aren't enabled, it would just be a matter of casting a function pointer to a writable data pointer and overwritting with the desired machine code.
But even something like "write XOR execute" memory protection doesn't help much. The ability to easily clobber the stack would make it straightforward to use return-oriented programming to hijack control flow. And the attacker could even intentionally add functions with lots of ROP gadgets.
Banning unsafe code is not enough. You'd also need to ban safe code that has undefined behavior (which is nontrivial to do given how many soundness bugs Rust has, some of which are very hard to fix); otherwise, safe code could be used to do the same thing that unsafe code does.
I've thought for a while that rustc should have an option to redo the type and lifetime check post-monomorphization, as a method of guarding against soundness bugs. You would need something like that for banning unsafe code to have any value. However, I suspect it's hard to implement given the current compiler architecture (and even that would only catch type-checking-related unsoundnesses, not other sorts of miscompile).
I might not want to run (most of) it at all, but just understand it while debugging a crater regression or a Miri report or something like that -- to then copy-paste the relevant parts and run those.