Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Considerations for a setuid wrapper

Tags:

c

setuid

A Python extension I've written requires root access to do a single hardware initialisation call. I'd rather not run the whole script as root just for this one call in my extension, so I would like to write a wrapper to do this initialisation before dropping to user privileges and running the actual script.

I intend for this wrapper to be run via sudo, eg

$ sudo devwrap python somescript.py

I was considering something like (updated to fix a couple of bugs):

int main(int argc, char * argv[])
{
  if(argc < 2) return 0;

  int res = do_hardware_init();
  if(res != OK_VALUE)
  {
    // Print error message
    return HW_ERR;
  }

  const char *sudo_uid = getenv("SUDO_UID");
  if(sudo_uid)
  {
      int real_uid = (int) strtol(sudo_uid, NULL, 0);
      setuid(real_uid);
  }

  return execvp(argv[1], &argv[1]); // No return if successful

}

So I have three questions:

  1. Does this look sane? I don't usually need to mess with the *uid() calls, so I'm not familiar with the usual pitfalls. The execvp call also looks a little bizarre, but as far as I can see it has arguments in the right place).
  2. The execvp man page says that "The environ array should not be accessed directly by the application" - does this make the getenv call a bad idea?
  3. Is there a better call than execvp, so I can do sudo devwrap somescript.py (note absence of "python")
like image 518
detly Avatar asked Nov 15 '22 05:11

detly


1 Answers

  1. Sort of sane...more below.
  2. Using getenv() is not accessing the environ array directly - that is clean. Accessing the environ array directly means 'strcpy(buffer, environ[3])` or something like that.
  3. If the script starts with a shebang (#!/usr/bin/env python, perhaps), you can do what you want already - the somescript.py has to be executable, of course.

The problems I see with the first part depend on how you handle errors from the hardware initialization. If the omitted error handling does not exit, then you could get a core dump (or segfault) when not run via 'sudo' because you run strtol() on a null pointer. If do_hardware_init() itself exits on failure, there's no problem, unless the user finds a way to subvert the environment from 'sudo'. I really think you should validate the environment and exit with an error if SUDO_UID is not set plausibly. Would root want to run this extension?

I've not looked at the specification of sudo to see that it does set the SUDO_UID environment variable - I assume you're correct on that.

What are the ramifications of the user typing this?

sudo devwrap ls

It does the hardware initialization, resets the UID, and then runs ls - probably not too harmful, but maybe not what you had in mind. Does that matter? Can you control that at all?

If the argument count is less than two, you should probably give an error exit, not a successful exit.


It is very unconventional to require people to run extensions via 'sudo'.

Are you sure there isn't another way to achieve it? What are the requirements on the initialization? Is it done once for all processes, or once per process (so the chaining is crucial)?


Can you simply make the devwrap program SUID root? You would then need to reset the UID differently:

setgid(getgid());
setuid(getuid());

That removes any SGID-ness and any SUID-ness before executing the command. It's pretty hard to do significant damage. It isn't clear that the setgid() call is mandatory if the program is installed without SGID, but it does no harm either.

like image 143
Jonathan Leffler Avatar answered Dec 13 '22 15:12

Jonathan Leffler