Security: Stack based Buffer Overflows

[hacksysteam]

WebSockify Bug Report - Project Srishti

Note: This is an old bug found by source code audit.

While looking at websockify.c in websockify/other, I noticed an obvious Stack based Buffer Overflow bug.

Let's first see how settings_t structure has been defined:

typedef struct {
    int verbose;
    char listen_host[256];
    int listen_port;
    void (*handler)(ws_ctx_t*);
    int handler_id;
    char *cert;
    char *key;
    int ssl_only;
    int daemon;
    int run_once;
} settings_t;

We can see that size of listen_host is 256 bytes as sizeof(char) is 1 byte. I found two occurrence of the Stack based Buffer Overflow afterwards I did not audit further as I was asked to stop the audit.

Now, let's see the occurrence where the Overflow occurs.

found = strstr(argv[optind], ":");
if (found) {
    memcpy(settings.listen_host, argv[optind], found - argv[optind]);
    settings.listen_port = strtol(found + 1, NULL, 10);
} else {
    settings.listen_host[0] = '\0';
    settings.listen_port = strtol(argv[optind], NULL, 10);
}
optind++;
if (settings.listen_port == 0) {
    usage("Could not parse listen_port\n");
}

found = strstr(argv[optind], ":");
if (found) {
    memcpy(target_host, argv[optind], found - argv[optind]);
    target_port = strtol(found + 1, NULL, 10);
} else {
    usage("Target argument must be host:port\n");
}
if (target_port == 0) {
    usage("Could not parse target port\n");
}

Let's understand the below given piece of code.

found = strstr(argv[optind], ":");
if (found) {
    memcpy(settings.listen_host, argv[optind], found - argv[optind]);
    settings.listen_port = strtol(found + 1, NULL, 10);
}

We know that strstr() returns the pointer to the found substring. According to this piece of code, when : is found in a string, found variable holds the pointer to the start of the substring.

Developer has ignored the fact that, found - argv[optind] will vary depending the how long the argument is provided. Hence, overflows the listen_host[256] buffer.

This allows us to take control of the Instruction Pointer when void (*handler)(ws_ctx_t*) is overflowed in the same structure and ws_ctx handler is called.

As a proof of concept, I have developed a dummy program that show Stack base Buffer Overflow happening.

websockifyboftest.c

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

int main(int argc, char *argv[])
{
    char *found;
    char listen_host[12];

    found = strstr(argv[1], ":");
    
    printf("argv[1]: %p\n", argv[1]);
    printf("found: %p\n", found);
    printf("The substring is: %s\n", found);
    printf("found-argv[1]: %d\n", found-argv[1]);
    
    if (found) {
        printf("Storing: listen_host");
        memcpy(listen_host, argv[1], found-argv[1]);
    }
    
    printf("listen_host: %s\n", listen_host);

    return(0);
}

Output

ashfaq@hacksys:~$ ./websockifyboftest AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA:A

argv[1]: 0x7ffd199af357
found: 0x7ffd199af387
The substring is: :A
found-argv[1]: 48
Storing: listen_hostlisten_host: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
Segmentation fault (core dumped)

Recommendation

The size of copy should be exactly same as the size of the buffer available.

memcpy(settings.listen_host, argv[optind], sizeof(settings.listen_host));

Credits

Ashfaq Ansari - Project Srishti