From 0a9b973f18ddb17be83d1b2d737cf4382809d425 Mon Sep 17 00:00:00 2001 From: Luke Bratch Date: Sat, 18 May 2019 18:13:51 +0100 Subject: Fix another buffer overrun and numerous memory leaks. --- blabouncer.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/blabouncer.c b/blabouncer.c index b996f25..812f91e 100644 --- a/blabouncer.c +++ b/blabouncer.c @@ -173,7 +173,6 @@ int sendtoclient(int fd, char *strsrc, struct client *clients, struct settings * // Also set the pending statuses to 0 int disconnectclient(int fd, struct client *clients) { printf("disconnectclient(): disconnecting client fd '%d'\n", fd); - close(fd); // bye! // Remove the client from the clients array for (int i = 0; i < MAXCLIENTS; i++) { if (clients[i].fd == fd) { @@ -185,6 +184,10 @@ int disconnectclient(int fd, struct client *clients) { clients[i].pendingban = 0; clients[i].pendingwho = 0; clients[i].pendinglist = 0; + // Finish up with OpenSSL + SSL_free(clients[i].ssl); + // Close the socket + close(fd); return 1; } } @@ -573,6 +576,8 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // Copy to a temporary string so we still have the original in case it's not processed char *strcopy = strdup(str); + // Keep track of initial pointer for free()ing later + char *strcopyPtr = strcopy; char *token; @@ -602,6 +607,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli sendtoserver(server_ssl, outgoingmsg, strlen(outgoingmsg), 0, clients, settings); // We processed something so return true + free(strcopyPtr); return 1; } @@ -625,18 +631,21 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // Null the end of the new string ircdstrings->nickuserhost[strlen(tokens[counter - 1]) + 1] = '\0'; // +1 for the inserted colon printf("nickuserhost '%s' stored.\n", ircdstrings->nickuserhost); + free(strcopyPtr); return 1; } else if (strncmp(tokens[1], "002", strlen(tokens[1])) == 0) { printf("Found greeting 002 (%s), storing in ircdstrings struct.\n", str); strncpy(ircdstrings->greeting002, str, strlen(str)); // Null the end of the string ircdstrings->greeting002[strlen(str)] = '\0'; + free(strcopyPtr); return 1; } else if (strncmp(tokens[1], "003", strlen(tokens[1])) == 0) { printf("Found greeting 003 (%s), storing in ircdstrings struct.\n", str); strncpy(ircdstrings->greeting003, str, strlen(str)); // Null the end of the string ircdstrings->greeting003[strlen(str)] = '\0'; + free(strcopyPtr); return 1; } else if (strncmp(tokens[1], "004", strlen(tokens[1])) == 0) { printf("Found greeting 004 (%s), storing in ircdstrings struct.\n", str); @@ -647,6 +656,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli strncpy(ircdstrings->ircdname, tokens[3], strlen(tokens[3])); // Null the end of the string ircdstrings->ircdname[strlen(tokens[3])] = '\0'; + free(strcopyPtr); return 1; } @@ -676,6 +686,8 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // And then send to all clients sendtoallclients(clients, str, sourcefd, settings); + free(prefixcopy); + free(strcopyPtr); return 1; } @@ -698,6 +710,8 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // And then send to all clients sendtoallclients(clients, str, sourcefd, settings); + free(prefixcopy); + free(strcopyPtr); return 1; } @@ -716,6 +730,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli char *topiccopy = strdup(str); extractfinalparameter(topiccopy); setchanneltopic(channels, tokens[3], topiccopy); + free(topiccopy); // Server 333 (RPL_TOPICWHOTIME) set the channel topic setter and the time it was set } else if (strncmp(tokens[1], "333", strlen(tokens[1])) == 0) { printf("Server 333 (RPL_TOPICWHOTIME) found, extracting who and when, and storing in channel struct.\n"); @@ -765,6 +780,8 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // And then finally relay to all clients sendtoallclients(clients, str, sourcefd, settings); + free(topiccopy); + free(strcopyPtr); return 1; } @@ -784,6 +801,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli logprivmsg(str, settings->ircnick, settings->basedir); } + free(strcopyPtr); return 1; } @@ -811,6 +829,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // Update our nick strcpy(ircdstrings->ircnick, prefixcopy); printf("Updated ircnick to '%s'.\n", ircdstrings->ircnick); + free(nickuserhostcpy); } // Update all nicks in channels @@ -821,6 +840,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // Relay to all clients sendtoallclients(clients, str, sourcefd, settings); + free(strcopyPtr); return 1; } @@ -834,6 +854,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // Relay to all current clients sendtoallclients(clients, str, sourcefd, settings); + free(strcopyPtr); return 1; } @@ -850,8 +871,9 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli } } - return 1; - } + free(strcopyPtr); + return 1; + } // Server 368 (RPL_ENDOFBANLIST) received? Send to any clients who requested a ban MODE query. - TODO - Identify and handle start/middle of ban responses. if (strncmp(tokens[1], "368", strlen(tokens[1])) == 0) { @@ -866,6 +888,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli } } + free(strcopyPtr); return 1; } @@ -884,6 +907,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli } } + free(strcopyPtr); return 1; } @@ -902,6 +926,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli } } + free(strcopyPtr); return 1; } @@ -928,6 +953,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli disconnectclient(sourcefd, clients); } + free(strcopyPtr); return 1; } @@ -1047,6 +1073,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // Catch the client up with the default number of seconds of replay doreplay(sourcefd, settings->replayseconds, clients, settings, ircdstrings); + free(strcopyPtr); return 1; } @@ -1054,6 +1081,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // as the first thing we want to hear is either PASS or USER if (!clients[arrindex(clients, sourcefd)].registered) { printf("Ignoring client command '%s' from sourcefd '%d' as not registered yet.\n", tokens[0], sourcefd); + free(strcopyPtr); return 1; } @@ -1069,12 +1097,14 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli sendtoclient(sourcefd, outgoingmsg, clients, settings); // We processed something so return true + free(strcopyPtr); return 1; } // TODO - Ignoring CAP for now so as not to confuse other clients, but we should probably query the server then relay the response to the client if (strncmp(tokens[0], "CAP", strlen(tokens[0])) == 0) { printf("Client CAP found and it is: %s with length %zd! Ignoring completely for now - TODO - do something useful!\n", tokens[0], strlen(tokens[0])); + free(strcopyPtr); return 1; } @@ -1082,6 +1112,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli if (strncmp(tokens[0], "NICK", strlen(tokens[0])) == 0) { printf("Client NICK found and it is: %s with length %zd! Sending to server...\n", tokens[0], strlen(tokens[0])); sendtoserver(server_ssl, str, strlen(str), sourcefd, clients, settings); + free(strcopyPtr); return 1; } @@ -1111,6 +1142,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli logprivmsg(outgoingmsg, settings->ircnick, settings->basedir); } + free(strcopyPtr); return 1; } @@ -1118,6 +1150,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli if (strncmp(tokens[0], "JOIN", strlen(tokens[0])) == 0) { printf("Client JOIN found and it is: %s with length %zd! Sending to server...\n", tokens[0], strlen(tokens[0])); sendtoserver(server_ssl, str, strlen(str), sourcefd, clients, settings); + free(strcopyPtr); return 1; } @@ -1125,8 +1158,8 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // A client has QUIT, so disconnect (close) them and don't do anything else for now - TODO: Let another clients know with a NOTICE or something if (strncmp(tokens[0], "QUIT", strlen(tokens[0])) == 0) { printf("Client QUIT found from fd %d and it is: %s with length %zd! Disconnecting that fd.\n", sourcefd, tokens[0], strlen(tokens[0])); - close(sourcefd); disconnectclient(sourcefd, clients); + free(strcopyPtr); return 1; } @@ -1134,6 +1167,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli if (strncmp(tokens[0], "PART", strlen(tokens[0])) == 0) { printf("Client PART found and it is: %s with length %zd! Sending to server...\n", tokens[0], strlen(tokens[0])); sendtoserver(server_ssl, str, strlen(str), sourcefd, clients, settings); + free(strcopyPtr); return 1; } @@ -1141,6 +1175,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli if (strncmp(tokens[0], "TOPIC", strlen(tokens[0])) == 0) { printf("Client TOPIC found and it is: %s with length %zd! Sending to server...\n", tokens[0], strlen(tokens[0])); sendtoserver(server_ssl, str, strlen(str), sourcefd, clients, settings); + free(strcopyPtr); return 1; } @@ -1160,7 +1195,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // Either way, send it on the server sendtoserver(server_ssl, str, strlen(str), sourcefd, clients, settings); - free(strcopy); + free(strcopyPtr); return 1; } @@ -1171,6 +1206,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // Either way, send it on the server sendtoserver(server_ssl, str, strlen(str), sourcefd, clients, settings); + free(strcopyPtr); return 1; } @@ -1181,6 +1217,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli // Either way, send it on the server sendtoserver(server_ssl, str, strlen(str), sourcefd, clients, settings); + free(strcopyPtr); return 1; } @@ -1197,10 +1234,12 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli printf("Invalid number of replay seconds provided by REPLAY. Telling client.\n"); snprintf(outgoingmsg, MAXDATASIZE, "NOTICE %s :Invalid number of seconds of replay requested by REPLAY command.", ircdstrings->ircnick); sendtoclient(sourcefd, outgoingmsg, clients, settings); + free(strcopyPtr); return 1; } doreplay(sourcefd, replayseconds, clients, settings, ircdstrings); + free(strcopyPtr); return 1; // Unrecognised BLABOUNCER command received, send some help instructions } else { @@ -1209,6 +1248,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli sendtoclient(sourcefd, outgoingmsg, clients, settings); snprintf(outgoingmsg, MAXDATASIZE, "NOTICE %s :\"BLABOUNCER REPLAY [seconds]\" (Where [seconds] is the number of seconds of replay log to replay.)", ircdstrings->ircnick); sendtoclient(sourcefd, outgoingmsg, clients, settings); + free(strcopyPtr); return 1; } } @@ -1224,6 +1264,7 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli printf("Done with processircmessage()\n"); // If we got here then we didn't process anything + free(strcopyPtr); return 0; } @@ -1245,6 +1286,8 @@ int processircmessage(SSL *server_ssl, char *str, int source, struct client *cli int processrawstring(SSL *server_ssl, char *str, int source, struct client *clients, int sourcefd, struct ircdstrings *ircdstrings, struct channel *channels, struct settings *settings) { // Copy to a temporary string so we still have the original in case it's not processed char *strcopy = strdup(str); + // Keep track of initial pointer for free()ing later + char *strcopyPtr = strcopy; printf("processrawstring(): Source type %d sent: \"%s\". Processing it...\n", source, strcopy); @@ -1265,7 +1308,7 @@ int processrawstring(SSL *server_ssl, char *str, int source, struct client *clie messagecount++; } - free(strcopy); + free(strcopyPtr); // If there is a previous possibly truncated message still in the holding area, the prepend that to the first message of this new lot // (Only if source was the server since we always strip \r\n from client messages when recving - TODO - Should we be doing that? @@ -1277,6 +1320,7 @@ int processrawstring(SSL *server_ssl, char *str, int source, struct client *clie messages[0][strlen(ircdstrings->currentmsg) + strlen(strtmp)] = '\0'; // Make sure it's null terminated printf("...into new string '%s' and clearing currentmsg holding area.\n", messages[0]); ircdstrings->currentmsg[0] = '\0'; + free(strtmp); } // If the final characters of the raw string weren't \r\n then assume the final token is a truncated message @@ -1432,9 +1476,12 @@ void dochat(int *serversockfd, int *clientsockfd, struct settings *settings) { // Struct of channels we're in struct channel *channels; channels = malloc(sizeof(struct channel) * MAXCHANNELS); - // Set initial channel names to empty strings + // Set initial channel names and their nicks to empty strings for (int i = 0; i < MAXCHANNELS; i++) { channels[i].name[0] = '\0'; + for (int j = 0; j < MAXCHANUSERS; j++) { + channels[i].nicks[j][0] = '\0'; + } } // =============================================> -- cgit v1.2.3