Consider warning about trusting Address::system

[complexspaces]

Good afternoon,

Today zbus::Address has a convenient system() function on it which attempts to resolve where the socket for system-level services on the current platform/host are. However, this is primarily reliant on reading an environment variable:

zbus/zbus/src/address/mod.rs

Lines 90 to 91 in 0b3b6b1

	pub fn system() -> Result<Self> {
	match env::var("DBUS_SYSTEM_BUS_ADDRESS") {

As you probably know, environment variables can be manipulated in a lot of ways by software running even with same user privileges and attempting to start other apps in weird ways. For example this resulted in CVE-2012-3524 for libdbus back in 2012. This case was primarily about attacking setuid programs but the general concern of handling untrusted data in sensitive contexts remains.

I would primarily recommend adding an obvious warning to the documentation of the aforementioned function, but also all of the other public methods that return something from the "system" to note that zbus doesn't do any kind of security checks on the returned connection, and that individual users are responsible for validating if it is actually owned by the "system" or not. As a secondary recommendation, you could also detect specifically running in a "privileged" binary at runtime and use a well-known hardcoded path instead of an untrusted environment read.

[zeenix]

For example this resulted in CVE-2012-3524 for libdbus back in 2012.

But the CVE summary says:

libdbus maintainers state that this is a vulnerability in the applications that do not cleanse environment variables, not in libdbus itself: "we do not support use of libdbus in setuid binaries that do not sanitize their environment before their first call into libdbus."

However, I don't have any objections to what you're proposing. It should be noted that this is not a security issue in zbus and any changes resulting from this issue will purely to save users from making dangerous mistakes.

As a secondary recommendation, you could also detect specifically running in a "privileged" binary at runtime and use a well-known hardcoded path instead of an untrusted environment read.

Sure but I think this should be part of the spec then. If this makes sense, it makes sense for all D-Bus libraries out there. If we just do this on our own, we'll actually be going against the spec, which says that:

The address of the system message bus is given in the DBUS_SYSTEM_BUS_ADDRESS environment variable. If that variable is not set, applications should try to connect to the well-known address unix:path=/var/run/dbus/system_bus_socket

My interpretation here is that if the env is set, applications should use it. So that would be another reason I'd like this to be part of the spec before we implement it in zbus.

[zeenix]

I don't have any objections to what you're proposing.

@complexspaces would you be providing the PR?

[complexspaces]

@zeenix Sorry for the delay, I got a bit busy. Please have a look at #1319 when time permits, it adds the documentation originally proposed.

[zeenix]

@zeenix Sorry for the delay, I got a bit busy. Please have a look at #1319 when time permits, it adds the documentation originally proposed.

Np. Great job with the PR. Just address the nitpicks and we're done. :)