Thread: C help
View Single Post
Old 05-10-07, 03:01 PM   #6
evilghost
Registered User
 
Join Date: Jul 2005
Posts: 3,606
Default Re: C help

wnd, if you have the time to review the code again I'd certainly appreciate. I don't trust www touching a suid binary so I'm trying to create as much security as possible and you've been a huge help.

I read that using lstat() versus access() causes greater overhead and I should use access() in place with R_OK to test if the file is ready for reading (exists). I don't need to check uid/gid since they can be different values and I'm not too concerned about owner since they are dropped in by authenticated users (me or my wife) over SSH.

I also added a input length check to ensure the path passed doesn't exceed 255 characters, a number I determined to be a reasonable input length.

I also modified the code to take into account your suggestions.

Code:
/* evilghost
 * Thanks wnd for code snippets & help.
 * May 10, 2007
 * SETUID binary to fix permissions with uploaded images if they are invalid.
 * Permissions on compiled binary must be 4770 with uid/gid root:www-data
 */

#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <errno.h>
#include <time.h>

#define wwwdata_GID 33
#define shared_GID 5140
#define BASEPATH "/www/dlf/dlf-albums/"

int main(int argc, char **argv){

        FILE *fout;
        time_t curtime;
        struct tm *loctime;
        char t[80];
        t[79] = '\0';

        //Check initial input.
        if (argc != 2 || argv[1] == NULL || strncmp(argv[1], BASEPATH, strlen(BASEPATH)) != 0 || strstr(argv[1], "/..") != NULL) {
                fprintf(stderr,"Failed:  Invalid file path passed.\n");
                return EXIT_FAILURE;
        }

        //Check for possible abnormal behavior (excessively long parameter)
        if (strlen(argv[1]) > 255){
                fprintf(stderr,"Failed:  Extraordinarily long file path passed, ignoring.\n");
                return EXIT_FAILURE;
        }

        //Check if file exists and adjust permissions
        if (access(argv[1],R_OK) == 0){
                // Get the current time & convert to local.
                curtime = time (NULL);
                loctime = localtime (&curtime);
                strftime(t, 79, "%a %b %H:%M:%S %Y", loctime);

                //Write audit trail.
                fout = fopen ("/www/dlf/fixperms.log", "a+");
                if (fout != NULL){
                        fprintf(fout, "%s\t%s\n", t, argv[1]);
                        fclose(fout);
                }else{
                        fprintf(stderr,"Failed:  Error while opening audit log for writing\n");
                        return EXIT_FAILURE;
                }

                //Adjust file permissions
                chown(argv[1], wwwdata_GID, shared_GID);
                chmod(argv[1], 0770);

                fprintf(stderr,"Success\n");
                return EXIT_SUCCESS;
        }else{
                fprintf(stderr,"Failed:  File path specified does not exist, no action performed.\n");
                return EXIT_FAILURE;
        }
}
evilghost is offline   Reply With Quote