1

Is using a wrapper to execute a script a good practice ?

In such case can the following set of wrapper / script be considered as safe:

wrapper.c

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>
#include <sys/types.h>

int main(void)
{
    if (setuid(geteuid()) != 0)
    {
        fputs(strerror(errno), stderr);
        return EXIT_FAILURE;
    }
    system("cat script | /bin/bash");
    return EXIT_SUCCESS;
}

script

IFS=""
/bin/echo -n "[Whoami]: "
/usr/bin/whoami

/bin/echo -n "[ls /root]: "
/bin/ls /root

The script file does not contain any shebang. Would it change anything if I'd have added one ?

In the Wrapper, could I have simply called the script as I'd execute any Linux script instead of cat'ing its content into a shell ?

3 Answers 3

2

Given that it provides a gateway to privileged access, using setuid programs should be considered a last resort for providing functionality. This is usually exploited by invoking a further program from the setuid one - something your code does by design. Since there are no unusual permissions on script (it doesn't even need to be executable) there is a lack of transparency in what the code does. Not using an explicit path is a rookie mistake - your code can be compromised simply by changing $PATH

Given that there is a well defined way to provision such privileged access on unix systems, which has been well tested, is widely understood and provides for flexible and granular access control (I'm talking about sudo in case you hadn't guessed) why would anyone want to implement the code above?

2
  • Effectively you're right in the way that I definitely wasn't using what was meant for what I wanted to do. I'll use the sudo command / configuration from now on. Now, just out of curiosity, would the script be considered secure with the additional changes given in the edited version ?
    – Ra'Jiska
    Commented Oct 8, 2017 at 18:56
  • better - although still no path on 'script' means I can get around your security by just changing directory. :)
    – symcbean
    Commented Oct 9, 2017 at 11:00
2

No this is not considered safe, you do not make any attempt to sanitize the environment you are running in. Which will let the attacker manipulate what gets executed as root by manipulating the environment through various environment variables, from PATH to LD_PRELOAD. Specific vulnerabilities vary, and there is some effort to mitigate in dynamic loader but still you should sanitize environment and ensure you are in control of what gets executed in privilege context ensure only the relevant input is passed and nothing else ensure relevant input is valid.

2
  • Alright for path, effectively I did not realise the PATH variable was carried over after a SUID. I also reset the IFS variable from the script and as far as I know it also holds a security issue if the user is able to have access to it. Furthermore regarding the LD_PRELOAD, this post: stackoverflow.com/questions/426230/… ; says it may not be used with SUID.
    – Ra'Jiska
    Commented Oct 8, 2017 at 18:49
  • Don't try finding and plugging all the holes yourselves. sudo did it better (and even sudo has had security issues in the past).
    – Meir Maor
    Commented Oct 8, 2017 at 20:24
0

I created this small utility to create binary executable files, without the need to modify the source every time: https://github.com/thiagorb/suid-wrapper

It doesn't change the fact that you might be opening security holes in your system, but anyways... It seems like it is safe from the LD_PRELOAD vulnerability at least:

  • An example of output for the system "ls" with a custom_malloc implementation

      $ LD_PRELOAD=$(pwd)/custom_malloc.so ls
      alloc.so: calloc internal
      alloc.so: zalloc_internal calledalloc.so: Successfully hooked
      custom_malloc.c  custom_malloc.so
    
  • With a ls -la wrapper, running the generated binary with overridden LD_PRELOAD has no effect (if the owner of the file is not the same user who started the process):

      $ suid-wrapper -o ls "$(which ls)" -- -la && sudo chown root:root ls && sudo chmod +s ls
      $ ./ls
      total 52
      drwxrwxr-x  2 tbarcala tbarcala  4096 Dec 16 12:28 .
      drwxrwxr-x 33 tbarcala tbarcala  4096 Dec  9 11:23 ..
      -rw-rw-r--  1 tbarcala tbarcala  3467 Dec 16 12:18 custom_malloc.c
      -rwxrwxr-x  1 tbarcala tbarcala 16960 Dec 16 12:21 custom_malloc.so
      -rws--s--x  1 root     root     18151 Dec 16 12:28 ls
    

If you want to run shell scripts, to reduce the security risks, make sure the arguments you pass to the command are always full paths, and that all the files/paths that are used are owned by root (or the target user you want). You can see below some examples of how this vulnerability could be explored:

  • If /script.sh is not owned by root, some other user could modify /script.sh and run any code:

      sudo suid-wrapper -o ./root_script.sh $(which bash) /script.sh
    
  • If /home/user/scripts/script.sh is owned by root, but /home/user/scripts is not, the user can rename scripts, and create a new scripts folder which contains a different script.sh:

      sudo suid-wrapper -o ./root_script.sh $(which bash) /home/user/scripts/script.sh
    

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .