Can someone help me understand why my C webserver is throwing SIGABRT

Question

I'm new to C and trying to build a very simple webserver as a starter project but for some reason, after handling one request (and correctly returning the HTML file) I get SIGABRT at the handleClient function

It showed up when I introduced the Posix threads and I'm assuming it's something to do with multi-threading and incorrect memory handling. Can someone point me in the right direction or help me figure out what part of the code might be wrong?

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <unistd.h>
#include <fcntl.h>
#include <pthread.h>

void serveFile(int socket, const char* filename) {
    char buffer[1024];
    int fd;
    int readBytes;

    fd = open(filename, O_RDONLY);

    if (fd == -1) {
        perror("File open failed");
        return;
    }

    while ((readBytes = read(fd, buffer, sizeof(buffer))) > 0) {
        write(socket, buffer, readBytes);
    }

    close(fd);
}

void *handleClient(void *arg) {
    int socket = *(int*)arg;
    char buffer[1024];
    int readBytes;

    readBytes = read(socket, buffer, sizeof(buffer));

    buffer[readBytes] = '\0';
    printf("Received: %s\n", buffer);

    char *header = "HTTP/1.1 200 OK\nContent-Type: text/html\n\n";
    write(socket, header, strlen(header));

    serveFile(socket, "./helloworld.html");

    close(socket);

    free(arg);

    return NULL;
}

int main() {
    int server_fd, new_socket;
    struct sockaddr_in address;
    int opt = 1;
    int addrlen = sizeof(address);

    if ((server_fd = socket(AF_INET, SOCK_STREAM, 0)) == 0) {
        perror("socket failed");
        exit(EXIT_FAILURE);
    }

    address.sin_family = AF_INET;
    address.sin_addr.s_addr = INADDR_ANY;
    address.sin_port = htons(8080);

    if (bind(server_fd, (struct sockaddr *)&address, sizeof(address))<0) {
        perror("bind failed");
        exit(EXIT_FAILURE);
    }

    if (listen(server_fd, 3) < 0) {
        perror("listen");
        exit(EXIT_FAILURE);
    }

    while(1) {
        printf("Waiting for connections...\n");

        int *client_fd = malloc(sizeof(int));

        if ((*client_fd = accept(server_fd, (struct sockaddr *)&address, (socklen_t*)&addrlen))<0) {
            perror("accept");
            continue;
        }
        
        pthread_t thread_id;

        if(pthread_create(&thread_id, NULL, handleClient, (void *)client_fd)) {
            perror("Could not create thread");
            continue;
        }

        pthread_detach(thread_id);        
    }

    return 0;
}

edit: this is what the server receives from the client:

Received: GET / HTTP/1.1
Host: localhost:8080
Connection: keep-alive
Cache-Control: max-age=0
sec-ch-ua: "Google Chrome";v="123", "Not:A-Brand";v="8", "Chromium";v="123"
sec-ch-ua-mobile: ?0
sec-ch-ua-platform: "macOS"
Upgrade-Insecure-Requests: 1
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/123.0.0.0 Safari/537.36
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7
Sec-Fetch-Site: none
Sec-Fetch-Mode: navigate
Sec-Fetch-User: ?1
Sec-Fetch-Dest: document
Accept-Encoding: gzip, deflate, br, zstd
Accept-Language: en-US,en;q=0.9,sv-SE;q=0.8,sv;q=0.7
Cookie: access_token="token"

I'm running and compiling on macOS with an m3 chip. I'm using the latest version of Chrome to test and this is how the error message looks in the terminal

zsh: abort      ./http_server

Answer 1

There are several issues with your code that are readily apparent to me.

First, this is a potential buffer overflow:

char buffer[1024];
int readBytes;

readBytes = read(socket, buffer, sizeof(buffer));

buffer[readBytes] = '\0';

If you read a full 1024 bytes, you'll overflow buffer.

If you want to read the data as a nul-terminated string, you'll have to account for having to add it yourself:

char buffer[1024];
int readBytes;

readBytes = read(socket, buffer, sizeof(buffer) - 1);

buffer[readBytes] = '\0';

This cast to socklen_t could be flat-out wrong:

    if ((*client_fd = accept(server_fd, (struct sockaddr *)&address, (socklen_t*)&addrlen))<0) {
        ...

If addrlen is not a socklen_t, the accept() can overwrite memory if socklen_t is larger than int addrlen. Never cast away warnings.

There are likely other issues - those are the ones that I noted quickly.