Don't have arrindex() return 0 on failure as 0 is a valid index. Instead return -1 and change callers to check this.

6 files changed, 60 insertions, 28 deletions

diff --git a/TODO b/TODO
index fa3b393..b414406 100644
--- a/TODO
+++ b/TODO
@@ -24,8 +24,6 @@ Can memory usage be reduced further?  (e.g. better channel struct management)
 
 Ability to load new certificate whilst running.
 
-arrindex() shouldn't return 0 on failure as 0 is a valid index.  Instead return -1 and change callers to check this.
-
 Make the "channels" configuration file entry an array.
 
 Crash when requesting 30 hour replay.
diff --git a/blabouncer.c b/blabouncer.c
index 9604ba3..982d504 100644
--- a/blabouncer.c
+++ b/blabouncer.c
@@ -764,6 +764,14 @@ void dochat(int *serversockfd, int *clientsockfd, struct settings *settings) {
       debugprint(DEBUG_FULL, "checking client socket %d out of %d.\n", i, fdmax);
       if (FD_ISSET(i, &rfds)) {
         debugprint(DEBUG_FULL, "fd %d is FD_ISSET and it is a...\n", i);
+
+        // Index of client fd in clients array for use later
+        int clientindex = arrindex(clients, i);
+        if (clientindex < 0 && i != *clientsockfd) {
+          debugprint(DEBUG_CRIT, "dochat(): error: arrindex() returned '%d', exiting!\n", clientindex);
+          exit(1);
+        }
+
         // if value of clientsockfd then must be a new connection, if greater must be an existing connection
         if (i == *clientsockfd) {
           debugprint(DEBUG_SOME, "...new connection!\n");
@@ -845,15 +853,15 @@ void dochat(int *serversockfd, int *clientsockfd, struct settings *settings) {
             debugprint(DEBUG_FULL, "bouncer-client: total client connections: %d\n", numclients(clients));
           }
         // If using client TLS and still pending SSL_accept() then re-try SSL_accept() (it can't be real client data yet)
-        } else if (clients[arrindex(clients, i)].pendingsslaccept) {
+        } else if (clients[clientindex].pendingsslaccept) {
           debugprint(DEBUG_FULL, "...previous connection pending SSL_accept!\n");
           // Try to SSL_accept(), not interested in return code here since openssl_accept() does the right thing.
-          openssl_accept(clients[arrindex(clients, i)].fd, clients, &ircdstate, settings, clientcodes);
+          openssl_accept(clients[clientindex].fd, clients, &ircdstate, settings, clientcodes);
         } else {
           debugprint(DEBUG_FULL, "...previous connection!\n");
 
           // handle data from a client
-          if ((clientnumbytes = sockread(clients[arrindex(clients, i)].ssl, clientbuf, MAXRCVSIZE - 1, settings->clienttls)) <= 0) {
+          if ((clientnumbytes = sockread(clients[clientindex].ssl, clientbuf, MAXRCVSIZE - 1, settings->clienttls)) <= 0) {
             // got error or connection closed by client
             if (clientnumbytes == 0) {
               // connection closed
diff --git a/functions.c b/functions.c
index 10e71a1..5dd7586 100644
--- a/functions.c
+++ b/functions.c
@@ -393,6 +393,7 @@ void updategreetings(char *greeting001, char *greeting002, char *greeting003, ch
 }
 
 // Return index of requested client FD within the clients array.
+// Returns 0 or more on success, or -1 on failure.
 // TODO - Use this wherever we are calculating the position (various places) instead of
 // duplicating code.
 int arrindex(struct client *clients, int clientfd) {
@@ -404,8 +405,8 @@ int arrindex(struct client *clients, int clientfd) {
   }
 
   // Something went wrong, we didn't find it
-  // TODO - Don't return 0 since 0 is a valid array index, return -1 and have callers check for that.
-  return 0;
+  debugprint(DEBUG_CRIT, "arrindex(): error: got to MAXCLIENTS (%d) but didn't find clientfd '%d' in clients struct.\n", MAXCLIENTS, clientfd);
+  return -1;
 }
 
 // Send whatever string to a specific client by providing the FD
@@ -577,11 +578,17 @@ int sendtoserver(SSL *server_ssl, char *strsrc, int str_len, int clientfd, struc
 // Also set its authentication and registration statuses to 0.
 // Also set the pending statuses to 0
 int disconnectclient(int fd, struct client *clients, struct ircdstate *ircdstate, struct settings *settings, struct clientcodes *clientcodes) {
-  debugprint(DEBUG_SOME, "disconnectclient(): disconnecting client %s with fd '%d'\n", clients[arrindex(clients, fd)].remoteip, fd);
+  // Index of client fd in clients array for use later
+  int clientindex = arrindex(clients, fd);
+  if (clientindex < 0) {
+    debugprint(DEBUG_CRIT, "disconnectclient(): error: arrindex() returned '%d', exiting!\n", clientindex);
+    exit(1);
+  }
+  debugprint(DEBUG_SOME, "disconnectclient(): disconnecting client %s with fd '%d'\n", clients[clientindex].remoteip, fd);
 
   // Alert other clients about the disconnection (don't send yet, we haven't removed from the clients array yet)
   char alertmsg[MAXDATASIZE];
-  if (!snprintf(alertmsg, MAXDATASIZE, "NOTICE %s :blabouncer: client %s has disconnected.", ircdstate->ircnick, clients[arrindex(clients, fd)].remoteip)) {
+  if (!snprintf(alertmsg, MAXDATASIZE, "NOTICE %s :blabouncer: client %s has disconnected.", ircdstate->ircnick, clients[clientindex].remoteip)) {
     fprintf(stderr, "Error while preparing authentication failure NOTICE!\n");
     debugprint(DEBUG_CRIT, "Error while preparing authentication failure NOTICE!\n");
     alertmsg[0] = '\0';
@@ -1112,10 +1119,17 @@ void tryautonick(struct ircdstate *ircdstate) {
 void cleanexit(SSL *server_ssl, struct client *clients, int sourcefd, struct ircdstate *ircdstate, struct settings *settings, char *quitmsg) {
   char outgoingmsg[MAXDATASIZE];
 
+  // Index of client fd in clients array for use later
+  int clientindex = arrindex(clients, sourcefd);
+  if (clientindex < 0) {
+    debugprint(DEBUG_CRIT, "cleanexit(): error: arrindex() returned '%d', exiting!\n", clientindex);
+    exit(1);
+  }
+
   // Tell clients and debug log
   if (sourcefd) {
-    snprintf(outgoingmsg, MAXDATASIZE, "NOTICE %s :Exiting on request from client %s, message '%s'.", ircdstate->ircnick, clients[arrindex(clients, sourcefd)].remoteip, quitmsg);
-    debugprint(DEBUG_CRIT, "Exiting on request from client %s with fd '%d', message '%s'.\n", clients[arrindex(clients, sourcefd)].remoteip, sourcefd, quitmsg);
+    snprintf(outgoingmsg, MAXDATASIZE, "NOTICE %s :Exiting on request from client %s, message '%s'.", ircdstate->ircnick, clients[clientindex].remoteip, quitmsg);
+    debugprint(DEBUG_CRIT, "Exiting on request from client %s with fd '%d', message '%s'.\n", clients[clientindex].remoteip, sourcefd, quitmsg);
   } else {
     snprintf(outgoingmsg, MAXDATASIZE, "NOTICE %s :Exiting on request (not from a client), message '%s'.", ircdstate->ircnick, quitmsg);
     debugprint(DEBUG_CRIT, "Exiting on request (not from a client), message '%s'.\n", quitmsg);
diff --git a/functions.h b/functions.h
index 643328f..c22d7ac 100644
--- a/functions.h
+++ b/functions.h
@@ -93,6 +93,7 @@ void updatenickuserhost(char *nickuserhost, char *nick);
 void updategreetings(char *greeting001, char *greeting002, char *greeting003, char *greeting004, char *greeting005a, char *greeting005b, char *greeting005c, char *newnickuserhost, char *oldnickuserhost, char *newnick, char *oldnick);
 
 // Return index of requested client FD within the clients array.
+// Returns 0 or more on success, or -1 on failure.
 // TODO - Use this wherever we are calculating the position (various places) instead of
 // duplicating code.
 int arrindex(struct client *clients, int clientfd);
diff --git a/message.c b/message.c
index 6ee959a..d0c5fed 100644
--- a/message.c
+++ b/message.c
@@ -823,10 +823,17 @@ int processservermessage(SSL *server_ssl, char *str, struct client *clients, int
 // Return 1 if we processed it, or 0 if we didn't.
 int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int sourcefd, struct ircdstate *ircdstate,
                          struct channel *channels, struct settings *settings, char tokens[MAXTOKENS][MAXDATASIZE], int counter, struct clientcodes *clientcodes) {
+  // Index of client fd in clients array for use later
+  int clientindex = arrindex(clients, sourcefd);
+  if (clientindex < 0) {
+    debugprint(DEBUG_CRIT, "processclientmessage(): error: arrindex() returned '%d', exiting!\n", clientindex);
+    exit(1);
+  }
+
   // PASS received?  User is trying to log in, check their password.
   if (strncasecmp(tokens[0], "PASS", strlen(tokens[0])) == 0) {
     if (checkpassword(tokens[1], settings)) {
-      debugprint(DEBUG_FULL, "Password accepted!  Setting client %s with fd %d to authenticated.\n", clients[arrindex(clients, sourcefd)].remoteip, sourcefd);
+      debugprint(DEBUG_FULL, "Password accepted!  Setting client %s with fd %d to authenticated.\n", clients[clientindex].remoteip, sourcefd);
       // Find the client in the clients array and set them as authenticated
       for (int i = 0; i < MAXCLIENTS; i++) {
         if (clients[i].fd == sourcefd) {
@@ -836,7 +843,7 @@ int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int
           // Alert other clients about the successful authentication
           char alertmsg[MAXDATASIZE];
           if (!snprintf(alertmsg, MAXDATASIZE, "NOTICE %s :blabouncer: new client %s has successfully authenticated.", ircdstate->ircnick,
-              clients[arrindex(clients, sourcefd)].remoteip)) {
+              clients[clientindex].remoteip)) {
             fprintf(stderr, "Error while preparing authentication success NOTICE!\n");
             debugprint(DEBUG_CRIT, "Error while preparing authentication success NOTICE!\n");
             alertmsg[0] = '\0';
@@ -849,7 +856,7 @@ int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int
       // Store the client's IP address for now, since we need to refer to it after disconnecting
       // them (thus clearing the array entry that the IP is read from)
       char remoteip[INET6_ADDRSTRLEN];
-      strncpy(remoteip, clients[arrindex(clients, sourcefd)].remoteip, INET6_ADDRSTRLEN);
+      strncpy(remoteip, clients[clientindex].remoteip, INET6_ADDRSTRLEN);
       debugprint(DEBUG_SOME, "Password rejected, disconnecting client %s with fd %d.\n", remoteip, sourcefd);
       disconnectclient(sourcefd, clients, ircdstate, settings, clientcodes);
       // Alert other clients about the failed authentication
@@ -877,7 +884,7 @@ int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int
     }
     // Get the real IRC server name from greeting001
     // They are now pending CAP negotiation
-    clients[arrindex(clients, sourcefd)].pendingcap = 1;
+    clients[clientindex].pendingcap = 1;
     char outgoingmsg[MAXDATASIZE];
     // If client is requesting CAP list, send it...
     if (strncasecmp(tokens[1], "LS", strlen(tokens[1])) == 0) {
@@ -904,13 +911,13 @@ int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int
       }
     // If client is finishing CAP negotiation then mark them as so
     } else if (strncasecmp(tokens[1], "END", strlen(tokens[1])) == 0) {
-      clients[arrindex(clients, sourcefd)].pendingcap = -1;
+      clients[clientindex].pendingcap = -1;
     }
   }
 
   // We're past PASS in the list of possible commands, so ignore
   // anything else the client says if they are not authenticated yet.
-  if (!clients[arrindex(clients, sourcefd)].authed) {
+  if (!clients[clientindex].authed) {
     debugprint(DEBUG_CRIT, "Ignoring client command '%s' from sourcefd '%d' as not authenticated yet.\n", tokens[0], sourcefd);
     return 1;
   }
@@ -918,12 +925,12 @@ int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int
   // USER received and not pending CAP negotiation during registration?
   // Or client has just finished negotiating CAP (pendingcap = -1)?
   // If so, assume this is a new client connecting and catch them on up on the state
-  if ((strncasecmp(tokens[0], "USER", strlen(tokens[0])) == 0 && clients[arrindex(clients, sourcefd)].pendingcap == 0) || clients[arrindex(clients, sourcefd)].pendingcap == -1) {
+  if ((strncasecmp(tokens[0], "USER", strlen(tokens[0])) == 0 && clients[clientindex].pendingcap == 0) || clients[clientindex].pendingcap == -1) {
     // Somewhere to store the several strings we will need to build and send
     char outgoingmsg[MAXDATASIZE]; // String to send to client
 
     // If registering then they must no longer be pending CAP negotiation
-    clients[arrindex(clients, sourcefd)].pendingcap = 0;
+    clients[clientindex].pendingcap = 0;
 
     // Tell the client to go away if we aren't registered with the real server yet as defined by the last greeting not being set yet
     if (!strlen(ircdstate->greeting004)) {
@@ -976,7 +983,7 @@ int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int
     int channelcount = getchannelcount(channels, ircdstate->maxchannelcount);
     // Set the client as pending RPL_NAMREPLYs for 'channelcount' channels
     debugprint(DEBUG_FULL, "Setting pendingnames to '%d' for client with fd '%d'.\n", channelcount, sourcefd);
-    clients[arrindex(clients, sourcefd)].pendingnames = channelcount;
+    clients[clientindex].pendingnames = channelcount;
 
     // Get client to join channels, and tell client about those channels
     for (int i = 0; i < ircdstate->maxchannelcount; i++) {
@@ -1045,7 +1052,7 @@ int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int
     }
 
     // Set the client as registered
-    clients[arrindex(clients, sourcefd)].registered = 1;
+    clients[clientindex].registered = 1;
 
     // Catch the client up with the default number of seconds of replay
     if (!doautoreplay(sourcefd, clients, settings, ircdstate, channels)) {
@@ -1061,7 +1068,7 @@ int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int
 
   // Pretty much ignore anything else the client says if it's not registered yet,
   // as the first thing we want to hear is either PASS or USER
-  if (!clients[arrindex(clients, sourcefd)].registered) {
+  if (!clients[clientindex].registered) {
     debugprint(DEBUG_SOME, "Ignoring client command '%s' from sourcefd '%d' as not registered yet.\n", tokens[0], sourcefd);
     return 1;
   }
@@ -1167,11 +1174,11 @@ int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int
     // Is it a ban MODE request (MODE #channel b)?
     if (counter >= 3 && strncmp(tokens[2], "b", strlen("b")) == 0) {
       debugprint(DEBUG_FULL, "Ban MODE request received, marking as pending.\n");
-      clients[arrindex(clients, sourcefd)].pendingban = 1;
+      clients[clientindex].pendingban = 1;
     } else if (counter == 2) {
       // Assume a normal channel mode request (MODE #channel) - TODO - What if it isn't!?
       debugprint(DEBUG_FULL, "Assuming channel MODE request received, marking as pending.\n");
-      clients[arrindex(clients, sourcefd)].pendingchannelmode = 1;
+      clients[clientindex].pendingchannelmode = 1;
     }
 
     // Either way, send it on the server
@@ -1182,7 +1189,7 @@ int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int
   // WHO requested, mark the client as waiting for the reply (so not all clients get it) and send it on the server
   if (strncasecmp(tokens[0], "WHO", strlen(tokens[0])) == 0) {
     debugprint(DEBUG_FULL, "Client WHO found and it is: %s with length %zd!  Marking as pending.\n", tokens[0], strlen(tokens[0]));
-    clients[arrindex(clients, sourcefd)].pendingwho = 1;
+    clients[clientindex].pendingwho = 1;
 
     // Either way, send it on the server
     sendtoserver(server_ssl, str, strlen(str), sourcefd, clients, settings);
@@ -1192,7 +1199,7 @@ int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int
   // LIST requested, mark the client as waiting for the reply (so not all clients get it) and send it on the server
   if (strncasecmp(tokens[0], "LIST", strlen(tokens[0])) == 0) {
     debugprint(DEBUG_FULL, "Client LIST found and it is: %s with length %zd!  Marking as pending.\n", tokens[0], strlen(tokens[0]));
-    clients[arrindex(clients, sourcefd)].pendinglist = 1;
+    clients[clientindex].pendinglist = 1;
 
     // Either way, send it on the server
     sendtoserver(server_ssl, str, strlen(str), sourcefd, clients, settings);
@@ -1202,7 +1209,7 @@ int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int
   // Client WHOIS received, send straight on to server and mark the client as pending the response
   if (strncasecmp(tokens[0], "WHOIS", strlen(tokens[0])) == 0) {
     debugprint(DEBUG_FULL, "Client WHOIS found and it is: %s with length %zd!  Sending to server and setting client as pending.\n", tokens[0], strlen(tokens[0]));
-    clients[arrindex(clients, sourcefd)].pendingwhois = 1;
+    clients[clientindex].pendingwhois = 1;
     sendtoserver(server_ssl, str, strlen(str), sourcefd, clients, settings);
     return 1;
   }
@@ -1210,7 +1217,7 @@ int processclientmessage(SSL *server_ssl, char *str, struct client *clients, int
   // Client WHOWAS received, send straight on to server and mark the client as pending the response
   if (strncasecmp(tokens[0], "WHOWAS", strlen(tokens[0])) == 0) {
     debugprint(DEBUG_FULL, "Client WHOWAS found and it is: %s with length %zd!  Sending to server and setting client as pending.\n", tokens[0], strlen(tokens[0]));
-    clients[arrindex(clients, sourcefd)].pendingwhowas = 1;
+    clients[clientindex].pendingwhowas = 1;
     sendtoserver(server_ssl, str, strlen(str), sourcefd, clients, settings);
     return 1;
   }
diff --git a/sockets.c b/sockets.c
index f201d79..c075232 100644
--- a/sockets.c
+++ b/sockets.c
@@ -277,6 +277,10 @@ int fd_toggle_blocking(int fd, int blocking) {
 int openssl_accept(int fd, struct client *clients, struct ircdstate *ircdstate, struct settings *settings, struct clientcodes *clientcodes) {
   // Get the index of the this client fd
   int clientindex = arrindex(clients, fd);
+  if (clientindex < 0) {
+    debugprint(DEBUG_CRIT, "openssl_accept(): error: arrindex() returned '%d', exiting!\n", clientindex);
+    exit(1);
+  }
 
   // Clear OpenSSL errors before proceeding so we can reliably get errors from SSL_accept
   ERR_clear_error();