In principle yes we should do this, and initial patches for Bug #52202 did, but we had to back that change out because it broke gnome-keyring: <https://bugs.freedesktop.org/show_bug.cgi?id=52202#c24>.

This whole thing is a compromise, because the D-Bus maintainers are of the opinion that privilege-escalating[1] applications that don't sanitize their environment through a whitelist (like pkexec does) are just wrong; but at the same time, we recognize that the bad privilege-escalating apps aren't all going to be fixed any time soon, since they include things like traditional su(1) loading PAM modules.

Using secure_getenv goes more towards the "second-guess the app and try to survive whatever hostile environment we're running in" end of the scale: it provides better mitigation for badly written privilege-escalating things, at the cost of breaking the ability for privilege-escalating things where the author has actually thought about this stuff (like gnome-keyring) to use our standard APIs.

[1] by which I mean: setuid, setgid, filesystem capabilities, ...

There is really something not right with _dbus_check_setuid() then. My binary doesn't have setuid but _dbus_check_setuid() thinks it has.

[root@axis-00408cecff49 /mnt/flash/root]8060# ls -al /usr/bin/client 
-rwxr-xr-x    1 root     root         78644 Oct 10  2014 /usr/bin/client

Any known issue there?

(All pseudocode in this comment should be assumed to be a root privilege escalation vulnerability. For clarity, I am writing extremely insecure code. Do not do this.)

If your executable is run by a setuid thing that has not dropped privileges, that counts as setuid for that rather simple check. For instance, this pseudocode:

    /* client-wrapper, run by uid 'mortal', 4755 root:root */
    main ()
    {
      if (resuid (&ruid, &euid, &suid) != 0)
        abort();

      if (ruid != uid of mortal)
        abort();

      if (euid != 0)
        abort();

      system ("/usr/bin/client");
    }

would run 'client' with ruid != euid, resulting in _dbus_check_setuid considering itself to be setuid; whereas this:

    /* client-wrapper, run by uid 'mortal', 4755 root:root */
    main ()
    {
      if (resuid (&ruid, &euid, &suid) != 0)
        abort();

      if (ruid != uid of mortal)
        abort();

      if (euid != 0)
        abort();

      /* retain root privileges */
      /* THIS IS VERY UNSAFE, do not do this without taking appropriate
         precautions, in particular clearing the environment */
      if (setresuid (0, 0, 0) != 0)
        abort();

      system ("/usr/bin/client");
    }

or this:

    /* client-wrapper, run by uid 'mortal', 4755 root:root */
    main ()
    {
      if (resuid (&ruid, &euid, &suid) != 0)
        abort();

      if (ruid != uid of mortal)
        abort();

      if (euid != 0)
        abort();

      /* drop privileges */
      if (setresuid (ruid, ruid, ruid) != 0)
        abort();

      system ("/usr/bin/client");
    }

would not be setuid according to _dbus_check_setuid().

(Clarification: please read resuid() as getresuid().)

(In reply to Umut Tezduyar from comment #2)
> There is really something not right with _dbus_check_setuid() then.

What are the process's uids? It has at least a real uid, an effective uid and a saved uid (all returned by getresuid()), and there are probably more.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/114.