Server that accepts HTML and PHP requests

Question

For the Harvard cs50x MOOC I have completed an assignment in which a C server framework that functions like a barebones apache server is provided, to be finished by the student.

The below code will launch a server on the host IP with the root server folder pointed to by whatever is pointed to in the launch arguments. Usage is ./server -p [port] /path/to/root

I am very curious about how I could improve the security of this, as I believe I have left myself open to stackoverflow attacks but am unsure how else to handle some of the strings otherwise. Are there any other security flaws that I am missing other than stackoverflow attacks? The areas where the request line is pulled, parsed, and broken apart are the most vulnerable, but as I am a beginner I am not clear on the details how or how I would make it more secure.

//
// server.c
//
// David J. Malan
//
// feature test macro requirements
#define _GNU_SOURCE
#define _XOPEN_SOURCE 700
#define _XOPEN_SOURCE_EXTENDED
// limits on an HTTP request's size, based on Apache's
// http://httpd.apache.org/docs/2.2/mod/core.html
#define LimitRequestFields 50
#define LimitRequestFieldSize 4094
#define LimitRequestLine 8190
// number of octets for buffered reads
#define OCTETS 512
// header files
#include <arpa/inet.h>
#include <errno.h>
#include <limits.h>
#include <math.h>
#include <signal.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <strings.h>
#include <unistd.h>
#include <ctype.h>
// types
typedef char octet;
// prototypes
bool connected(void);
bool error(unsigned short code);
void handler(int signal);
ssize_t load(void);
const char* lookup(const char* extension);
ssize_t parse(void);
void reset(void);
void start(short port, const char* path);
void stop(void);
// server's root
char* root = NULL;
// file descriptor for sockets
int cfd = -1, sfd = -1;
// buffer for request
octet* request = NULL;
// FILE pointer for files
FILE* file = NULL;
// buffer for response-body
octet* body = NULL;
int main(int argc, char* argv[])
{
 // a global variable defined in errno.h that's "set by system 
 // calls and some library functions [to a nonzero value]
 // in the event of an error to indicate what went wrong"
 errno = 0;
 // default to a random port
 int port = 0;
 // usage
 const char* usage = "Usage: server [-p port] /path/to/root";
 // parse command-line arguments
 int opt;
 while ((opt = getopt(argc, argv, "hp:")) != -1)
 {
 switch (opt)
 {
 // -h
 case 'h':
 printf("%s\n", usage);
 return 0;
 // -p port
 case 'p':
 port = atoi(optarg);
 break;
 }
 }
 // ensure port is a non-negative short and path to server's root is specified
 if (port < 0 || port > SHRT_MAX || argv[optind] == NULL || strlen(argv[optind]) == 0)
 {
 // announce usage
 printf("%s\n", usage);
 // return 2 just like bash's builtins
 return 2;
 }
 // start server
 start(port, argv[optind]);
 // listen for SIGINT (aka control-c)
 signal(SIGINT, handler);
 // accept connections one at a time
 while (true)
 {
 // reset server's state
 reset();
 // wait until client is connected
 if (connected())
 {
 // parse client's HTTP request
 ssize_t octets = parse();
 if (octets == -1)
 {
 continue;
 }
 // extract request's request-line
 // http://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html
 const char* haystack = request;
 char* needle = strstr(haystack, "\r\n");
 if (needle == NULL)
 {
 error(400);
 continue;
 }
 else if (needle - haystack + 2 > LimitRequestLine)
 {
 error(414);
 continue;
 } 
 char line[needle - haystack + 2 + 1];
 strncpy(line, haystack, needle - haystack + 2);
 line[needle - haystack + 2] = '0円';
 // log request-line
 printf("%s", line);
 //validate request line
 //break apart the request line
 char *method, *target, *version;
 const char *firstbuffer = line;
 const char *secondbuffer;
 while (*firstbuffer != ' ')
 {
 firstbuffer++;
 }
 method = strndup(line, firstbuffer - line);
 secondbuffer = ++firstbuffer;
 while(*firstbuffer != ' ')
 {
 firstbuffer++;
 }
 target = strndup(secondbuffer, firstbuffer-secondbuffer);
 secondbuffer = ++firstbuffer;
 while(*firstbuffer != '\r')
 {
 firstbuffer++;
 }
 version = strndup(secondbuffer, firstbuffer - secondbuffer);
 //request line is now broken apart
 //ensure method is GET
 if (strstr("GET", method) == NULL)
 {
 error(405);
 }
 //ensure request-target begins with /
 if (target[0] != '/')
 {
 error(501);
 }
 //search for " in target
 if(strchr(target, '\"') != NULL)
 {
 error(400);
 }
 //make sure the version is HTTP/1.1
 if(strcmp(version, "HTTP/1.1") != 0)
 {
 error(505);
 }
 //ensure there's a . in absolute-path 
 if(strchr(target, '.') == NULL)
 {
 error(501);
 }
 // extract query from request-target
 char* query = "0円";
 char* thirdbuffer = target;
 if (strstr(target, "p?") != NULL)
 {
 query = strstr(target, "p?");
 query++;
 thirdbuffer = strndup(target, query-target);
 target = thirdbuffer;
 query++;
 }
 // concatenate root and absolute-path
 char path[strlen(root) + strlen(target) + 2];
 snprintf(path, sizeof(path), "%s%s", root, target);
 // ensure path exists
 if(access(path, F_OK) != 0)
 {
 error(404);
 }
 // ensure path is readable
 if(access(path, R_OK) != 0)
 {
 error(403);
 }
 // extract path's extension
 char* extension = "0円";
 extension = strrchr(target, '.');
 extension++; 
 // dynamic content
 if (strcasecmp("php", extension) == 0)
 {
 // open pipe to PHP interpreter
 char* format = "QUERY_STRING=\"%s\" REDIRECT_STATUS=200 SCRIPT_FILENAME=\"%s\" php-cgi";
 char command[strlen(format) + (strlen(path) - 2) + (strlen(query) - 2) + 1];
 sprintf(command, format, query, path);
 file = popen(command, "r");
 if (file == NULL)
 {
 error(500);
 continue;
 }
 // load file
 ssize_t size = load();
 if (size == -1)
 {
 error(500);
 continue;
 }
 // subtract php-cgi's headers from body's size to get content's length
 haystack = body;
 needle = memmem(haystack, size, "\r\n\r\n", 4);
 if (needle == NULL)
 {
 error(500);
 continue;
 }
 size_t length = size - (needle - haystack + 4);
 // respond to client
 if (dprintf(cfd, "HTTP/1.1 200 OK\r\n") < 0)
 {
 continue;
 }
 if (dprintf(cfd, "Connection: close\r\n") < 0)
 {
 continue;
 }
 if (dprintf(cfd, "Content-Length: %i\r\n", length) < 0)
 {
 continue;
 }
 if (write(cfd, body, size) == -1)
 {
 continue;
 }
 }
 // static content
 else
 {
 // look up file's MIME type
 const char* type = lookup(extension);
 if (type == NULL)
 {
 error(501);
 continue;
 }
 // open file
 file = fopen(path, "r");
 if (file == NULL)
 {
 error(500);
 continue;
 }
 // load file
 ssize_t length = load();
 if (length == -1)
 {
 error(500);
 continue;
 }
 // respond to client
 if (dprintf(cfd, "HTTP/1.1 200 OK\r\n") < 0)
 {
 continue;
 }
 if (dprintf(cfd, "Connection: close\r\n") < 0)
 {
 continue;
 }
 if (dprintf(cfd, "Content-Length: %i\r\n", length) < 0)
 {
 continue;
 }
 if (dprintf(cfd, "Content-Type: %s\r\n", type) < 0)
 {
 continue;
 }
 if (dprintf(cfd, "\r\n") < 0)
 {
 continue;
 }
 if (write(cfd, body, length) == -1)
 {
 continue;
 }
 }
 // announce OK
 printf("033円[32m");
 printf("HTTP/1.1 200 OK");
 printf("033円[39m\n");
 }
 }
}
/**
 * Accepts a connection from a client, blocking (i.e., waiting) until one is heard.
 * Upon success, returns true; upon failure, returns false.
 */
bool connected(void)
{
 struct sockaddr_in cli_addr;
 memset(&cli_addr, 0, sizeof(cli_addr));
 socklen_t cli_len = sizeof(cli_addr);
 cfd = accept(sfd, (struct sockaddr*) &cli_addr, &cli_len);
 if (cfd == -1)
 {
 return false;
 }
 return true;
}
/**
 * Handles client errors (4xx) and server errors (5xx).
 */
bool error(unsigned short code)
{
 // ensure client's socket is open
 if (cfd == -1)
 {
 return false;
 }
 // ensure code is within range
 if (code < 400 || code > 599)
 {
 return false;
 }
 // determine Status-Line's phrase
 // http://www.w3.org/Protocols/rfc2616/rfc2616-sec6.html#sec6.1
 const char* phrase = NULL;
 switch (code)
 {
 case 400: phrase = "Bad Request"; break;
 case 403: phrase = "Forbidden"; break;
 case 404: phrase = "Not Found"; break;
 case 405: phrase = "Method Not Allowed"; break;
 case 413: phrase = "Request Entity Too Large"; break;
 case 414: phrase = "Request-URI Too Long"; break;
 case 418: phrase = "I'm a teapot"; break;
 case 500: phrase = "Internal Server Error"; break;
 case 501: phrase = "Not Implemented"; break;
 case 505: phrase = "HTTP Version Not Supported"; break;
 }
 if (phrase == NULL)
 {
 return false;
 }
 // template
 char* template = "<html><head><title>%i %s</title></head><body><h1>%i %s</h1></body></html>";
 char content[strlen(template) + 2 * ((int) log10(code) + 1 - 2) + 2 * (strlen(phrase) - 2) + 1];
 int length = sprintf(content, template, code, phrase, code, phrase);
 // respond with Status-Line
 if (dprintf(cfd, "HTTP/1.1 %i %s\r\n", code, phrase) < 0)
 {
 return false;
 }
 // respond with Connection header
 if (dprintf(cfd, "Connection: close\r\n") < 0)
 {
 return false;
 }
 // respond with Content-Length header
 if (dprintf(cfd, "Content-Length: %i\r\n", length) < 0)
 {
 return false;
 }
 // respond with Content-Type header
 if (dprintf(cfd, "Content-Type: text/html\r\n") < 0)
 {
 return false;
 }
 // respond with CRLF
 if (dprintf(cfd, "\r\n") < 0)
 {
 return false;
 }
 // respond with message-body
 if (write(cfd, content, length) == -1)
 {
 return false;
 }
 // announce Response-Line
 printf("033円[31m");
 printf("HTTP/1.1 %i %s", code, phrase);
 printf("033円[39m\n");
 return true;
}
/**
 * Loads file into message-body.
 */
ssize_t load(void)
{
 // ensure file is open
 if (file == NULL)
 {
 return -1;
 }
 // ensure body isn't already loaded
 if (body != NULL)
 {
 return -1;
 }
 // buffer for octets
 octet buffer[OCTETS];
 // read file
 ssize_t size = 0;
 while (true)
 {
 // try to read a buffer's worth of octets
 ssize_t octets = fread(buffer, sizeof(octet), OCTETS, file);
 // check for error
 if (ferror(file) != 0)
 {
 if (body != NULL)
 {
 free(body);
 body = NULL;
 }
 return -1;
 }
 // if octets were read, append to body
 if (octets > 0)
 {
 body = realloc(body, size + octets);
 if (body == NULL)
 {
 return -1;
 }
 memcpy(body + size, buffer, octets);
 size += octets;
 }
 // check for EOF
 if (feof(file) != 0)
 {
 break;
 }
 }
 return size;
}
/**
 * Handles signals.
 */
void handler(int signal)
{
 // control-c
 if (signal == SIGINT)
 {
 // ensure this isn't considered an error
 // (as might otherwise happen after a recent 404)
 errno = 0;
 // announce stop
 printf("033円[33m");
 printf("Stopping server\n");
 printf("033円[39m");
 // stop server
 stop();
 }
}
/**
 * Returns MIME type for supported extensions, else NULL.
 */
const char* lookup(const char* extension)
{
 char* extensionlowered = malloc(sizeof(extension) + 1);
 strcpy(extensionlowered, extension);
 extensionlowered[strlen(extensionlowered) + 1] = '0円';
 //ensure all cases work 
 for(int i =0, length = strlen(extension); i < length; i++)
 {
 extensionlowered[i] = tolower(extension[i]);
 }
 if(strcmp(extensionlowered, "css") == 0)
 {
 return "text/css";
 }
 else if(strcmp(extensionlowered, "html") == 0)
 {
 return "text/html";
 }
 else if(strcmp(extensionlowered, "gif") == 0)
 {
 return "image/gif";
 }
 else if(strcmp(extensionlowered, "ico") == 0)
 {
 return "image/x-icon";
 }
 else if(strcmp(extensionlowered, "jpg") == 0)
 {
 return "image/jpeg";
 }
 else if(strcmp(extensionlowered, "js") == 0)
 {
 return "text/javascript";
 }
 else if(strcmp(extensionlowered, "png") == 0)
 {
 return "image/png";
 }
 else
 {
 return NULL;
 } 
}
/**
 * Parses an HTTP request.
 */
ssize_t parse(void)
{
 // ensure client's socket is open
 if (cfd == -1)
 {
 return -1;
 }
 // ensure request isn't already parsed
 if (request != NULL)
 {
 return -1;
 }
 // buffer for octets
 octet buffer[OCTETS];
 // parse request
 ssize_t length = 0;
 while (true)
 {
 // read from socket
 ssize_t octets = read(cfd, buffer, sizeof(octet) * OCTETS);
 if (octets == -1)
 {
 error(500);
 return -1;
 }
 // if octets have been read, remember new length
 if (octets > 0)
 {
 request = realloc(request, length + octets);
 if (request == NULL)
 {
 return -1;
 }
 memcpy(request + length, buffer, octets);
 length += octets;
 }
 // else if nothing's been read, socket's been closed
 else
 {
 return -1;
 }
 // search for CRLF CRLF
 int offset = (length - octets < 3) ? length - octets : 3;
 char* haystack = request + length - octets - offset;
 char* needle = memmem(haystack, request + length - haystack, "\r\n\r\n", 4);
 if (needle != NULL)
 {
 // trim to one CRLF and null-terminate
 length = needle - request + 2 + 1;
 request = realloc(request, length);
 if (request == NULL)
 {
 return -1;
 }
 request[length - 1] = '0円';
 break;
 }
 // if buffer's full and we still haven't found CRLF CRLF,
 // then request is too large
 if (length - 1 >= LimitRequestLine + LimitRequestFields * LimitRequestFieldSize)
 {
 error(413);
 return -1;
 }
 }
 return length;
}
/**
 * Resets server's state, deallocating any resources.
 */
void reset(void)
{
 // free response's body
 if (body != NULL)
 {
 free(body);
 body = NULL;
 }
 // close file
 if (file != NULL)
 {
 fclose(file);
 file = NULL;
 }
 // free request
 if (request != NULL)
 {
 free(request);
 request = NULL;
 }
 // close client's socket
 if (cfd != -1)
 {
 close(cfd);
 cfd = -1;
 }
}
/**
 * Starts server.
 */
void start(short port, const char* path)
{
 // path to server's root
 root = realpath(path, NULL);
 if (root == NULL)
 {
 stop();
 }
 // ensure root exists
 if (access(root, F_OK) == -1)
 {
 stop();
 }
 // ensure root is executable
 if (access(root, X_OK) == -1)
 {
 stop();
 }
 // announce root
 printf("033円[33m");
 printf("Using %s for server's root", root);
 printf("033円[39m\n");
 // create a socket
 sfd = socket(AF_INET, SOCK_STREAM, 0);
 if (sfd == -1)
 {
 stop();
 }
 // allow reuse of address (to avoid "Address already in use")
 int optval = 1;
 setsockopt(sfd, SOL_SOCKET, SO_REUSEADDR, &optval, sizeof(optval));
 // assign name to socket
 struct sockaddr_in serv_addr;
 memset(&serv_addr, 0, sizeof(serv_addr));
 serv_addr.sin_family = AF_INET;
 serv_addr.sin_port = htons(port);
 serv_addr.sin_addr.s_addr = htonl(INADDR_ANY);
 if (bind(sfd, (struct sockaddr*) &serv_addr, sizeof(serv_addr)) == -1)
 {
 stop();
 }
 // listen for connections
 if (listen(sfd, SOMAXCONN) == -1)
 {
 stop();
 }
 // announce port in use
 struct sockaddr_in addr;
 socklen_t addrlen = sizeof(addr);
 if (getsockname(sfd, (struct sockaddr*) &addr, &addrlen) == -1)
 {
 stop();
 }
 printf("033円[33m");
 printf("Listening on port %i", ntohs(addr.sin_port));
 printf("033円[39m\n");
}
/**
 * Stop server, deallocating any resources.
 */
void stop(void)
{
 // preserve errno across this function's library calls
 int errsv = errno;
 // reset server's state
 reset();
 // free root, which was allocated by realpath
 if (root != NULL)
 {
 free(root);
 }
 // close server socket
 if (sfd != -1)
 {
 close(sfd);
 }
 // terminate process
 if (errsv == 0)
 {
 // success
 exit(0);
 }
 else
 {
 // announce error
 printf("033円[33m");
 printf("%s", strerror(errsv));
 printf("033円[39m\n");
 // failure
 exit(1);
 }
}

Answer 1

This is a massive code. I only scrub the surface. In no particular order:

• Is there a reason for not supporting HTTP/1.0?

• 500 family of error codes indicate internal server errors. Returning them for malformed request (e.g. target[0] != '/' or strchr(target, '.') == NULL) is inappropriate.

• access() is the most useless function: fopen() failure returns just the same information (via errno). Besides, a gap between testing permissions and using the resource is inherent, leading to unexpected race conditions.

  It might be OK to check permission at the startup, but not while serving the request.

• I don't think you may rely on fclose() to close popened stream. Use pclose() to make sure that php didn't crash midway.

• I strongly advise against globals. It is very hard to follow what is modified when.

• errno treatment is a bit strange. Normal pattern is

   if (some_call() == -1) {
   // check errno and act accordingly
   ...
   }
  

  There is absolutely no reason to set it to 0.

• Since you refuse to serve requests larger than LimitRequestLine, there is no need to have request dynamically allocated. parse shall simply fail on the long requests. BTW, parse guarantees that there is a CRLF in the request. Serve loop doesn't need to repeat the search.

• read not returning a positive value needs more accurate treatment. 0 is returned when the client shuts down it writing end, which is perfectly legal, and doesn't mean that client disconnected.

• stop is called from the signal handler, and therefore cannot call signal-unsafe functions like free and printf.