Changeset 6146

Timestamp:

04/13/2008 11:11:50 AM (15 months ago)

Author:

emostar

Message:

Prevent a DoS that crashed licq by using too many descriptors.
Fixes #1623

Location:

trunk/licq

Files:

• include/licq_socket.h (modified) (1 diff)
• src/icqd-chat.cpp (modified) (2 diffs)
• src/icqd-threads.cpp (modified) (2 diffs)
• src/socket.cpp (modified) (1 diff)

trunk/licq/include/licq_socket.h

@@ -251 +251 @@
   fd_set SocketSet()   {  return m_sSockets.SocketSet(); }
   int LargestSocket()  {  return m_sSockets.Largest(); }
+  unsigned short Num() {  return m_sSockets.Num(); }
 
 protected:

trunk/licq/src/icqd-chat.cpp

@@ -24 +24 @@
 #include "gettext.h"
 
+#define MAX_CONNECTS  256
 #define DEBUG_THREADS(x)
 
@@ -2384 +2385 @@
         else if (nCurrentSocket == chatman->chatServer.Descriptor())
         {
-          CChatUser *u = new CChatUser;
-          u->m_pClient = new CChatClient;
-
-          chatman->chatServer.RecvConnection(u->sock);
-          chatman->sockman.AddSocket(&u->sock);
-          chatman->sockman.DropSocket(&u->sock);
-
-          u->state = CHAT_STATE_HANDSHAKE;
-          chatman->chatUsers.push_back(u);
-          gLog.Info(tr("%sChat: Received connection.\n"), L_TCPxSTR);
+          if (chatman->sockman.Num() >= MAX_CONNECTS)
+          {
+            // Too many sockets, drop this one
+            gLog.Warn(tr("%sToo many connected clients, rejecting new connection.\n"), L_WARNxSTR);
+          }
+          else
+          {
+            CChatUser *u = new CChatUser;
+            u->m_pClient = new CChatClient;
+
+            chatman->chatServer.RecvConnection(u->sock);
+            chatman->sockman.AddSocket(&u->sock);
+            chatman->sockman.DropSocket(&u->sock);
+
+            u->state = CHAT_STATE_HANDSHAKE;
+            chatman->chatUsers.push_back(u);
+            gLog.Info(tr("%sChat: Received connection.\n"), L_TCPxSTR);
+          }
         }
 

trunk/licq/src/icqd-threads.cpp

@@ -24 +24 @@
 #include "gettext.h"
 
+#define MAX_CONNECTS  256
 #define DEBUG_THREADS(x)
 //#define DEBUG_THREADS(x) gLog.Info(x)
@@ -781 +782 @@
               tcp->RecvConnection(*newSocket);
               gSocketManager.DropSocket(tcp);
-              gSocketManager.AddSocket(newSocket);
-              gSocketManager.DropSocket(newSocket);
+
+              // Make sure we can handle another socket before accepting it
+              if (gSocketManager.Num() > MAX_CONNECTS)
+              {
+                // Too many sockets, drop this one
+                char remoteIp[32];
+                gLog.Warn(tr("%sToo many connected sockets, rejecting connection from %s.\n"),
+                    L_WARNxSTR, newSocket->RemoteIpStr(remoteIp));
+                delete newSocket;
+              }
+              else
+              {
+                gSocketManager.AddSocket(newSocket);
+                gSocketManager.DropSocket(newSocket);
+              }
             }
           }

trunk/licq/src/socket.cpp

@@ -818 +818 @@
   socklen_t sizeofSockaddr = sizeof(struct sockaddr_in);
 
-  newSocket.m_nDescriptor = accept(m_nDescriptor, (struct sockaddr *)&newSocket.m_sRemoteAddr, &sizeofSockaddr);
-  newSocket.SetLocalAddress();
+  // Make sure we stay under FD_SETSIZE
+  // See:
+  // * http://www.securityfocus.com/archive/1/490711
+  // * http://securityvulns.com/docs7669.html
+  // for more details
+  // This probably has no affect, since we are using multiple threads, but keep it here 
+  // to be used as a sanity check.
+  int newDesc = accept(m_nDescriptor, (struct sockaddr *)&newSocket.m_sRemoteAddr, &sizeofSockaddr);
+  if (newDesc < FD_SETSIZE)
+  {
+    newSocket.m_nDescriptor = newDesc;
+    newSocket.SetLocalAddress();
+  }
+  else
+  {
+    gLog.Error(tr("%sCannot accept new connection, too many descriptors in use.\n"), L_ERRORxSTR);
+    close(newDesc);
+
+    // TODO throw an exception, or do something to tell the caller it failed
+  }
 }