4
\$\begingroup\$

I recently made this simple server in C for Linux systems and was wanting to get another set of eyes on it for a review of the design. I am new to socket programming and used a textbook from school to guide my way.

It is for GET requests from browsers and serves from a public folder (or other destination of choice). It seems to work with all the files I have thrown at it so far. If you test it out, keep in mind that it only works with files that have their type extensions. For example, "index.html" would have to be requested as "index.html" in the browser. I was wanting information about how sessions and POST requests also work on professional servers

Net.h

#ifndef NET_H_INCLUDED
#define NET_H_INCLUDED


#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <ctype.h>
#include <errno.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/sendfile.h>
#include <netinet/in.h>
#include <arpa/inet.h>


/**
 *  common functions for TCP IP4 server
 *
********************************************************************/


typedef struct sockaddr_in SA_in;


typedef struct sockaddr SA;


// configure server address
int configure_server_IP4(struct sockaddr_in *, uint16_t, int);


int sanitary(const char *);


int compareFileType(const char *, const char *);


int sendPage(const char *, int);


typedef struct {
    int fd_client;
    char *file_name;
} GetRequest;


GetRequest * prepareGetRequest(int, const char *, const char *);


void * handleGetRequest(void *);


#endif // NET_H_INCLUDED

Net.c

#include "Net.h"


const char *HEADER          =   "HTTP/1.1 200 OK\r\n"
                                "Content-Type: text/html\r\n"
                                "\r\n";


const char *FILE_NOT_FOUND  =   "<!DOCTYPE html>\r\n"
                                "<html>\r\n"
                                "<head>\r\n"
                                "   <title>404 Error</title>\r\n"
                                "</head>\r\n"
                                "<body>\r\n"
                                "   <h1>404 Error: File Not Found</h1>\r\n"
                                "</body>\r\n"
                                "</html>\r\n";


/**
 *  common functions for TCP IP4 server
 *
********************************************************************/


// configure server address
int configure_server_IP4(SA_in *addr_server_p,
                         uint16_t PORT, int back_log) {

    int fd_server, status;

    // clear memory for address
    memset(addr_server_p, 0, sizeof(*addr_server_p));

    // set up server address
    addr_server_p->sin_family = AF_INET;            // internet
    addr_server_p->sin_port = htons(PORT);          // host to network short
    addr_server_p->sin_addr.s_addr = INADDR_ANY;    // 0

    // get socket from OS
    if((fd_server = socket(AF_INET, SOCK_STREAM, 0)) < 0) {
        perror("server socket failure\n");
        exit(1);
    } else
        printf("server socket: %i\n", fd_server);

    // bind server socket to server address
    status = bind(
        fd_server,
        (SA *) addr_server_p,
        sizeof(SA)
    ); 

    if(status < 0) {
        perror("socket binding failure\n");
        exit(1);
    } else
        printf("socket binding success\n");

    // listen for connection from clients
    status = listen(fd_server, back_log);

    if(status < 0) {
        perror("failure to start listening\n");
        exit(1);
    } else
        printf("listening on port: %i\n", PORT);

    // return file descriptor for client
    return fd_server;
}


int sanitary(const char *file_name) {
    size_t end = strlen(file_name) - 1;

    for(int i = 0; i < end; ++i)
        if(file_name[i] == '.' && file_name[i + 1] == '.')
            return 0;

    return 1;
}


int compareFileType(const char *file_name, const char *file_type) {

    int l_name, l_type, i, j;

    l_name = strlen(file_name);
    l_type = strlen(file_type);

    printf("file name: %s\n", file_name);

    if(l_type > l_name)
        return 0;

    for(i = l_name - 1, j = l_type - 1; j >= 0; --i, --j)
        if(file_name[i] != file_type[j])
            return 0;

    return 1;
}


int sendPage(const char *file_name, int fd_client) {

    int fd_storage;

    size_t file_size;

    struct stat st;

    // account for unsanitary file names and files not found
    if(!sanitary(file_name) || stat(file_name, &st) < 0) {

        // write HEADER to client
        write(fd_client, HEADER, strlen(HEADER));

        // write FILE_NOT_FOUND to client
        write(fd_client, FILE_NOT_FOUND, strlen(FILE_NOT_FOUND));

        return 0;
    }

    // open file
    if((fd_storage = open(file_name, O_RDONLY)) < 0) {
        perror("unable to open file\n");
        return 0;
    }

    printf("file opened\n");

    // account for html file header
    if(compareFileType(file_name, "html"))
        write(fd_client, HEADER, strlen(HEADER));

    // send file to client
    sendfile(fd_client, fd_storage, NULL, st.st_size + 1);

    // close file
    close(fd_storage);

    printf("file closed\n");

    return 1;
}


GetRequest * prepareGetRequest(int fd_client,
                               const char *location, const char *buffer) {

    char c,
         *file_name;

    size_t left,
           right,
           length_location,
           length_file_name,
           length_combined;

    GetRequest *getR;

    length_location = strlen(location);

    for(left = 0; buffer[left] != '\0'; ++left)
        if(buffer[left] == '/')
            break;

    for(right = left; buffer[right] != '\0'; ++right) {
        c = buffer[right];

        if(!isalnum(c) && c != '.' && c != '-' && c != '/')
            break;
    }

    length_file_name = right - left;

    length_combined = length_location + length_file_name;

    file_name = malloc((length_combined + 1) * sizeof(char));

    if(file_name == NULL) {
        perror("memory allocation error\n");
        return NULL;
    }

    // put location in front of file name
    strncpy(file_name, location, length_location);

    // put buffer contents in file name 
    for(size_t i = length_location; i < length_combined && left < right; ++i, ++left)
        file_name[i] = buffer[left];

    file_name[length_combined] = '\0';

    if((getR = malloc(sizeof(GetRequest))) == NULL) {
        free(file_name);
        return NULL;
    }

    getR->fd_client = fd_client;
    getR->file_name = file_name;

    return getR;
}


void * handleGetRequest(void *generic) {
    GetRequest *getR;

    int fd_client;

    char *file_name;

    getR = (GetRequest *) generic;

    fd_client = getR->fd_client;
    file_name = getR->file_name;

    sendPage(file_name, fd_client);

    printf("closing fd_client: %i\n\n", fd_client);
    close(fd_client);

    // free memory for file name
    free(getR), free(file_name);

    return NULL;
}

server.c

#include "Net.h"

#include <pthread.h>


int main() {

    uint16_t PORT = 3000;

    int back_log = 20;

    // request buffer
    const size_t SIZE = 1024;
    char request_buffer[SIZE + 1];

    char *location = "public";

    // socket file descriptors
    int fd_server,
        fd_client;

    GetRequest *getR;

    // ipv4 socket addresses
    SA_in addr_server,
          addr_client;

    socklen_t socket_length = sizeof(SA_in);

    // pthread id
    pthread_t thread_id;

    // configure server and begin waiting for connections
    fd_server = configure_server_IP4(&addr_server, PORT, back_log);

    /**
     *  acceptance loop for clients
     *
     ***************************************************************************/

    while(1) {

        // clear memory for address
        memset(&addr_client, 0, sizeof(addr_client));

        if((fd_client = accept(fd_server, (SA *) &addr_client, &socket_length)) < 0) {
            perror("client not accepted\n");
            continue;
        }

        printf("client accepted...\n\n");

        // recieve request from client
        memset(request_buffer, '\0', (SIZE + 1) * sizeof(char));
        read(fd_client, request_buffer, SIZE);
        printf("%s\n", request_buffer);

        // prepare GET request
        if((getR = prepareGetRequest(fd_client, location, request_buffer)) == NULL)
            continue;

        // single thread
        // handleGetRequest(getR);

        // if possible, handle GET request in seperate thread
        if(pthread_create(&thread_id, NULL, handleGetRequest, getR) != 0)
            handleGetRequest(getR);
        else if(pthread_detach(thread_id) != 0)
            pthread_join(thread_id, NULL);
    }

    exit(0);
}
\$\endgroup\$

1 Answer 1

Reset to default
2
\$\begingroup\$

Declare variables as close to where they're used as possible. As an example, you can move the pthread_t thread_id declaration inside this while loop. This increases code readability since the reader doesn't have to look around for declarations and other uses of a variable.

Comment non-trivial code. However, too many comments can be distracting rather than helpful.

// request buffer
const size_t SIZE = 1024;
char request_buffer[SIZE + 1];

char *location = "public";

// socket file descriptors
int fd_server,
    fd_client;

GetRequest *getR;

// ipv4 socket addresses
SA_in addr_server,
      addr_client;

socklen_t socket_length = sizeof(SA_in);

// pthread id
pthread_t thread_id;

// configure server and begin waiting for connections
fd_server = configure_server_IP4(&addr_server, PORT, back_log);

IMO, almost all of these comments are superfluous. The only exception might the one alluding to that configure_server_IP4 doesn't actually just configure but also binds and starts listening on the socket. That probably means that you should rename the function to be more descriptive of what it does, perhaps server_config_and_listen or similar.

Another function that isn't quite clear what it does at first glance is sanitary. I think you should at least add a comment describing why ".." is dangerous.

Also note that there is a potential buffer overrun and DoS exploit in this function. What happens if strlen(file_name) is 0? end will underflow and become SIZE_MAX. It doesn't seem like this path is reachable in the current form, but always be defensive and prepare for the worst! You can reimplement sanitary using strstr instead, which is probably both faster and safer.

int sanitary(const char *file_name) {
    return strstr(file_name, "..") == NULL;
}

In fact, parsing the file_name may not be the best way to handle "sanitizing" the file names. There may be some other way for an adversary to manipulate the path to escape your public directory. Checking for ".." seems to be an instance of the XY problem: what we want is our server to only access files below a certain directory, but what we're telling it is to look for ".." in the path. Let's make use of realpath instead!

realpath() expands all symbolic links and resolves references to /./, /../ and extra '/' characters in the null-terminated string named by path to produce a canonicalized absolute pathname. The resulting pathname is stored as a null-terminated string, up to a maximum of PATH_MAX bytes, in the buffer pointed to by resolved_path. The resulting path will have no symbolic link, /./ or /../ components.

/** Initialize this somewhere **/
char public_dir[PATH_MAX];

if (!realpath("public", public_dir)) {
    perror("invalid public path");
    exit(1);
}

/* Check if a path is inside the public directory.
 * Return  0 if file path is safe.
 * Return  1 if path is not safe.
 * Return -1 on error.
 */
int check_path_safe(const char *file_path, const char *public_dir) {
    char resolved_file_path[PATH_MAX];

    if (!realpath(file_path, resolved_file_path)) {
        return -1;
    }

    // If resolved_file_path starts with public_dir, it's safe
    if (strstr(resolved_file_path, public_dir) == resolved_file_path) {
        return 0;
    } else {
        return 1;
    }
}

Similarly, you can make use of standard library functions in other places. compareFileType can be refactored to use strrchr.

Use a consistent code style. The code above has mixed snake_case and camelCase. Choose one (whichever) and roll with it!

If you're interested in looking at another simple HTTP server in C I would recommend browsing through tinyhttpd. It's only 524 lines and handles both GET and POST via CGI.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.