Securing code

[screwloose]

How can I make the code below secure? Thanks. I am not a C programmer.

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

#define MAXSTRING 100
#define LEN 80
char s[LEN];
char cmd[MAXSTRING];

main(int argc,char **argv) {

setuid(0);
setgid(0);

if(argc != 2) {
printf("Syntax: %s [stop] [start]\n", argv[0]);
exit(1);
}

printf("The Apache Web Server will %s at your request.\n", argv[1]);
sprintf(cmd,"/usr/local/apache2/bin/apachectl %s\n", argv[1]);
system(cmd);
exit(0);
}

 

[xwb]

What do you mean by secure?

1) Does not crash easily
2) Cannot be copied easily

 

[Zech]

First, your codes have a buffer overflow problem. 'cmd' buffer has 100 bytes allocated to it and there is no proper check to ensure that argv[1] fits the allocated buffer. When the user enters argument (argv[1]) that exceeds the size of 'cmd' buffer minus the length of "/usr/local/apache2/bin/apachectl \n", buffer overflow will occur, possibly overwriting data that other programs are using as well. To prevent this problem, check the length of argv[1] to make sure that it fits the allocated buffer (don't forget to take into account "/usr/local/apache2/bin/apachectl \n") before assigning it to 'cmd'.

The next problem would be the calls to setuid() and setgid(). There were security problems with the old unix-based systems due to programmers abusing the setuid() (and its related commands) when writing their programs. I would be very cautious about using setuid() and try to avoid them whenever possible. At the very least, authenticate the users before granting them the priviledges (that's only the necessary priviledges).

I also notice that there is an unused buffer - char s[80]. You can delete it if you don't need it.

There are many other ways to improve the security further. You can restrict the 'cmd' to allow only certain commands to be executed (by comparing 'cmd' against a list of commands that you allow). You can also make the program to create its own log - to record who does what through your program (although typically unix system already logs almost everything).