Problem
I wanted to launch a bash script (read: bash not sh script) as a root not as the user calling it, however bash ignore setuid
on scripts, so I chose to write a very small script that takes a script/arguments and call it with setuid
set.
This worked well and I went even further to verify that the script has setuid
set on, executable and setuid()
called on the owner of the file and not as root, to avoid any misuse of the program and I ended up with the program below.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
int main(int argc, char **argv)
{
char *command;
int i, file_owner, size = 0;
struct stat status_buf;
ushort file_mode;
// Check argc
if (argc < 2) {
printf("Usage: %s <script> [arguments]n", argv[0]);
return 1;
}
// Make sure the script does exist
if(fopen(argv[1], "r") == NULL) {
printf("The file %s does not exist.n", argv[1]);
return 1;
}
// Get the attributes of the file
stat(argv[1], &status_buf);
// Get the permissions of the file
file_mode = status_buf.st_mode;
// Make sure it's executable and it's setuid
if(file_mode >> 6 != 567) {
printf("The file %s should be executable and should have setuid set, please chmod it 0106755.n", argv[1]);
return 1;
}
// Get the owner of the script
file_owner = status_buf.st_uid;
// setuid
setuid(file_owner);
// Generate the command
for (i = 1; i < argc; i++) {
size += strlen(argv[i]);
}
command = (char *) malloc( (size + argc + 11) * sizeof(char) );
sprintf(command, "/bin/bash %s", argv[1]);
if (argc > 2) {
for (i = 2; i < argc; i++) {
sprintf(command, "%s %s", command, argv[i]);
}
}
// Execute the command
system(command);
// free memory
free(command);
return 0;
}
The exercise was not only to solve my problem, but it was also a way to get more into C, so what do you suggest? Is there anything I should improve?
Solution
-
Check for errors in
stat
,setuid
,fork
, andexecvp
, to name a few. If the exec fails in the child you should callexit
. -
Do you really want the
p
inexecvp
? This is not guaranteed to be the same as theargv[1]
you juststat
-ed. Ifargv[1]
isls
yourstat
will look for a file at./ls
and it will likely find the program in/bin
. I would useexecve
and either do thatPATH
lookup yourself or simply omit that part and require the user to specify a full path for something inPATH
(eg./bin/ls
instead of justls
). -
The
stat
+ observe state +exec
thing is a race condition. Another process can change the attributes on the file in that timing window. This may or may not be important to you. Given that this is a security-ish program I would say it may very well be. -
Instead of returning 0, you might want to return the child process’s exit code (which you can get with
waitpid
.) You might also want to return nonzero when the functions I mention in #1 fail. This way a shell script or something calling you programmatically can determine success or failure.
If I understand you correctly you would install your program (edit V3) like this
$ gcc -o edit_v3 edit_v3.c
$ ls -l edit_v3
-rwxrwxr-x 1 erik erik 7698 2012-04-15 12:22 edit_v3
$ sudo chown root.root edit_v3
$ sudo chmod 4755 edit_v3
$ ls -l edit_v3
-rwsr-xr-x 1 root root 7698 2012-04-15 12:22 edit_v3
But a user could then easily get root access
$ ls -l /bin/su
-rwsr-xr-x 1 root root 31116 2011-06-24 11:37 /bin/su
$ ./edit_v3 /bin/su
# id
uid=0(root) gid=0(root) grupper=0(root),116(pulse),117(pulse-access)
It is difficult to write secure setuid programs. Over the years a lot of security vulnerabilities have been found in such program.
Instead of writing your own setuid program, you could also use sudo. You would then have to edit the configuration file /etc/sudoers
One thing I’d do is
change
sprintf(command, "%s %s", command, argv[i]);
to use strcat
while this does work on a number of implementations, it’s not considered “safe”
refer https://stackoverflow.com/questions/1283354/is-sprintfbuffer-s-buffer-safe