Skip to content

Assert we are not setuid before looking at the environment

Simon McVittie requested to merge wip/smcv/assert-not-setuid into master

This includes/is based on !59 (merged), please review that first.


While reviewing !56 (merged), Emil pointed out that real ICD loaders normally use secure_getenv(), which ignores environment variables if it detects that the process is running with elevated privileges, whereas we are using the GLib equivalent of plain getenv().

This library is not designed to be used in a privileged process, never mind a setuid one: it trusts various aspects of its execution environment, and it executes subprocesses that will have similar behaviour. As such, I'm treating "we are not setuid" as a precondition check rather than as a normal situation, and considering execution of this library in a setuid context to be a programming error.

setuid/setgid/setcap executables inherit their execution environment (environment variables, current working directory, file descriptors, controlling terminal, process group and other inheritable state) from a less-privileged and potentially hostile caller, so they are a privilege boundary, and can only safely use libraries if those libraries are also specifically designed to distrust their caller.

The name of the function only contains "setuid" for brevity, but in fact it checks for all aspects of elevated privileges, including setgid and setcap: the AT_SECURE auxiliary value is the same one used by secure_getenv(). The fallback implementation used if getauxval() fails only checks for setuid and setgid.


If reviewers think this is too much code to justify, alternatives include:

  • slim down these checks to only look at setuid/setgid
  • slim down these checks to just g_return_if_fail() in the library constructor
  • overrule @xexaxo's review on !56 (merged) and don't check at all

/cc @jpwhiting @denittis @xexaxo

Merge request reports

Loading