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