Running shell script as root via external binary

Question

I'm not an experienced Linux user and I wanted an easy way to run shell scripts as root from a PHP script, I came up with this:

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

int main(int argc, char *argv[])
{
    if (access(argv[1], F_OK) != -1)
    {
        struct stat filestat;

        if (stat(argv[1], &filestat) == 0)
        {
            if ((filestat.st_uid == 0) && (filestat.st_gid == 0) && (filestat.st_mode & S_IXUSR) && (!(filestat.st_mode & S_IWOTH)))
            {
                char* match = strrchr(argv[1], '.');

                if ((match != NULL) && (strcasecmp(match, ".sh") == 0))
                {
                    if (setuid(0) != -1)
                    {
                        execl("/usr/bin/sudo", "/usr/bin/sudo", argv[1], (char*) NULL);
                        return 0;
                    }
                }
            }
        }
    }

    return 1;
}

Any potential security issues with this? If so how can I improve it?

Example Usage:

Lets say I have a script located at:

/some/path/script.sh

With the following in it:

#!/bin/bash
echo $USER

Now lets say I compile the above C code to a binary and place it at:

/some/path/run-as

and do:

chown root:root /some/path/run-as
chmod 6755 /some/path/run-as

Now I run this PHP script owned by www-data (via browser / local apache web server):

<?php
    echo exec('/some/path/run-as /some/path/script.sh');
?>

I expect the script to output 'root' when ran.

@TobySpeight I went ahead and edited the post with example usage to give you an idea of what I'm trying to do, also I'm not too familiar with Linux so I'm not even sure If my idea will work as I haven't tested it yet. — user177906
– user177906, Commented Aug 22, 2018 at 20:36
Welcome to Code Review! I rolled back your last edit. After getting an answer you are not allowed to change your code anymore. This is to ensure that answers do not get invalidated and have to hit a moving target. If you have changed your code you can either post it as an answer (if it would constitute a code review) or ask a new question with your changed code (linking back to this one as reference). Refer to this post for more information — Sᴀᴍ Onᴇᴌᴀ
– Sᴀᴍ Onᴇᴌᴀ ♦, Commented Aug 22, 2018 at 22:57

Toby Speight · Accepted Answer · 2018-08-23 09:48:13Z

This is an ambitious thing to try, with many pitfalls - especially for environments which are Web-accessible. I urge you to learn how to configure and use sudo instead - that's not foolproof, but it is a good deal safer than rolling your own.

That said, I'll make some observations:

We're using argv[1] without checking whether any arguments were supplied (we should test argc >= 2 before using argv[1] - possibly check argc == 2, since we ignore any extra arguments).
There's a time-of-check/time-of-use (TOCTOU) race, because we examine the supplied file and later assume the properties haven't changed (we we get to running it).
Using system causes the supplied argument to be parsed as a shell command, which will do word splitting, expand variables and globs, and more.
Using a shell script from a privileged program is vulnerable to the script being changed between the interpreter being started and it opening the script to read commands (e.g. by switching symlinks).

It's quite easy to exploit this script to execute anything I want on your system, if there's a directory I can write to:

ln -s /bin/true 'ls -l ~root; id; cd; echo rm -rf; true .sh'

```
/some/path/run-as ls*.sh
```

We can avoid using system() (and accept extra arguments if we choose), by using execv() instead:

            if (match && !strcasecmp(match, ".sh"))
            {
                execv(argv[1], argv+1);
                /* if we got here, exec failed */
                /* so fall through to return 1 */
            }

This does still leave us vulnerable to a user creating a symbolic link to a root-owned file and rewriting it to point to the user's file between the access check and the exec(), though.

In short: please use sudo if you really have to run some things as root. Configured properly, it can give tight control on the commands you can execute with elevated privileges. It's not completely attack-proof, but it is much better than writing your own (it has had decades of scrutiny to improve it).

I've edited my original post with some changes, I've decided to strip all non alphanumeric characters (excluding '/' and '.') since I know the paths I use will never include spaces or any special characters. I wasn't sure how to fix the symlink issue, so I decided to just use lstat instead of stat. — user177906
– user177906, Commented Aug 22, 2018 at 22:50
isnt it even better if user set up a group that only have access to the things that the php script needs access to? — baot
– baot, Commented Aug 23, 2018 at 10:01
@baot: Absolutely; much better. I've only scratched at the surface with this answer. — Toby Speight
– Toby Speight, Commented Aug 23, 2018 at 10:10

user177906 · Accepted Answer · 2018-08-22 23:39:04Z

I know It's better to just use sudo, but I still want to do it this way.

Going by Toby Speight's response, I've modified the code a bit:

Ensure argc == 2.
Stripping unnecessary characters from input.
Using lstat instead of stat.

Hopefully this will be good enough.

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <string.h>
#include <strings.h>
#include <ctype.h>

int main(int argc, char *argv[])
{
    if (argc == 2)
    {
        unsigned long i = 0;
        unsigned long j = 0;
        char c;

        while ((c = argv[1][i++]) != '\0')
        {
            if ((isalnum(c)) || (c == '.') || (c == '/'))
            {
                argv[1][j++] = c;
            }
        }

        argv[1][j] = 0;

        if (access(argv[1], F_OK) != -1)
        {
            struct stat filestat;

            if (lstat(argv[1], &filestat) == 0)
            {
                if ((filestat.st_uid == 0) && (filestat.st_gid == 0) && (filestat.st_mode & S_IXUSR) && (!(filestat.st_mode & S_IWOTH)))
                {
                    char* match = strrchr(argv[1], '.');

                    if ((match != NULL) && (strcasecmp(match, ".sh") == 0))
                    {
                        if (setuid(0) != -1)
                        {
                            execv(argv[1], argv+1);
                            return 0;
                        }
                    }
                }
            }
        }
    }

    return 1;
}

You don't want to return zero (success) if exec() returns - that's clearly a failure. I still recommend using sudo instead, which can be configured to permit your specific combination of requesting user, target user and command with much better control and fewer bugs. — Toby Speight
– Toby Speight, Commented Aug 23, 2018 at 7:02
I still want to do it this way. Don't. For example, all the tricks you're going through to seemingly ensure that only a root-owned shell script can be exec'd by your process still has a huge hole in it, based on this. Hopefully this will be good enough. No, it's not. Even if you figure out the vulnerability I just pointed you in the direction of, there are still race conditions that your approach pretty much ignores. Heck, I'd bet a simple LD_PRELOAD could be used to exploit your code. — Andrew Henle
– Andrew Henle, Commented Aug 24, 2018 at 16:41

Stack Exchange Network

Running shell script as root via external binary

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Running shell script as root via external binary

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions