]> jfr.im git - irc/gunnarbeutner/shroudbnc.git/commitdiff
Fix a potential crash with invalid DNS responses.
authorGunnar Beutner <redacted>
Thu, 6 Mar 2014 22:30:54 +0000 (23:30 +0100)
committerGunnar Beutner <redacted>
Fri, 7 Mar 2014 12:10:33 +0000 (13:10 +0100)
src/Connection.cpp
src/Connection.h
src/IRCConnection.cpp
src/utility.cpp
src/utility.h

index e6da594f9578e6a8675784841e142c544768f4b5..8a875d18525ef69f2ada572fef142d3f19ba848b 100644 (file)
@@ -783,16 +783,13 @@ void CConnection::AsyncConnect(void) {
                        return;
                }
 
-               m_Socket = SocketAndConnectResolved(Remote, Bind);
+               int ErrorCode;
+               m_Socket = SocketAndConnectResolved(Remote, Bind, &ErrorCode);
 
                free(m_HostAddr);
                m_HostAddr = NULL;
 
                if (m_Socket == INVALID_SOCKET) {
-                       int ErrorCode;
-
-                       ErrorCode = errno;
-
 #ifndef _WIN32
                        if (ErrorCode == 0) {
                                ErrorCode = -1;
@@ -816,43 +813,33 @@ void CConnection::AsyncConnect(void) {
  * @param Response the response
  */
 void CConnection::AsyncDnsFinished(hostent *Response) {
-       if (Response == NULL) {
+       if (Response == NULL || Response->h_addr_list[0] == NULL) {
                // we cannot destroy the object here as there might still be the other
                // dns query (bind ip) in the queue which would get destroyed in the
                // destructor; this causes a crash in the StartMainLoop() function
                m_LatchedDestruction = true;
-       } else {
-               int Size;
 
-               m_Family = Response->h_addrtype;
-
-               if (Response->h_addrtype == AF_INET) {
-                       Size = sizeof(in_addr);
-               } else if (Response->h_addrtype == AF_INET6) {
-                       Size = sizeof(in6_addr);
-               } else {
-                       m_LatchedDestruction = true;
-
-                       return;
-               }
-
-               m_HostAddr = (in_addr *)malloc(Size);
+               return;
+        }
 
-               if (AllocFailed(m_HostAddr)) {
-                       m_LatchedDestruction = true;
+       int Size = INADDR_LEN(Response->h_addrtype);
 
-                       return;
-               }
+       m_HostAddr = malloc(Size);
 
-               memcpy(m_HostAddr, Response->h_addr_list[0], Size);
+       if (AllocFailed(m_HostAddr)) {
+               m_LatchedDestruction = true;
+               return;
+       }
 
-                if (m_BindIpCache != NULL) {
-                       m_BindDnsQuery = new CDnsQuery(this, USE_DNSEVENTPROXY(CConnection, AsyncBindIpDnsFinished));
-                       m_BindDnsQuery->GetHostByName(m_BindIpCache, Response->h_addrtype);
-               }
+       m_Family = Response->h_addrtype;
+       memcpy(m_HostAddr, Response->h_addr_list[0], Size);
 
-               AsyncConnect();
+        if (m_BindIpCache != NULL) {
+               m_BindDnsQuery = new CDnsQuery(this, USE_DNSEVENTPROXY(CConnection, AsyncBindIpDnsFinished));
+               m_BindDnsQuery->GetHostByName(m_BindIpCache, Response->h_addrtype);
        }
+
+       AsyncConnect();
 }
 
 /**
@@ -864,23 +851,13 @@ void CConnection::AsyncDnsFinished(hostent *Response) {
  */
 
 void CConnection::AsyncBindIpDnsFinished(hostent *Response) {
-       if (Response != NULL) {
-               int Size;
-
-               if (Response->h_addrtype == AF_INET) {
-                       Size = sizeof(in_addr);
-               } else if (Response->h_addrtype == AF_INET6) {
-                       Size = sizeof(in6_addr);
-               } else {
-                       Size = 0;
-               }
+       if (Response != NULL && Response->h_addr_list[0] != NULL) {
+               int Size = INADDR_LEN(Response->h_addrtype);
 
-               if (Size != 0) {
-                       m_BindAddr = (in_addr *)malloc(Size);
+               m_BindAddr = malloc(Size);
 
-                       if (!AllocFailed(m_BindAddr)) {
-                               memcpy(m_BindAddr, Response->h_addr_list[0], Size);
-                       }
+               if (!AllocFailed(m_BindAddr)) {
+                       memcpy(m_BindAddr, Response->h_addr_list[0], Size);
                }
        }
 
index ca00ce8b4b1ea0b1f9d77456e01bfe6e4781573a..a95f0853ecd4251a3e25b0bdb05c246a1d174928 100644 (file)
@@ -67,6 +67,8 @@ protected:
        CFIFOBuffer *m_SendQ; /**< send queue */
        CFIFOBuffer *m_RecvQ; /**< receive queue */
 
+       bool m_LatchedDestruction; /**< should the connection object be destroyed? */
+
 public:
        virtual void AsyncDnsFinished(hostent *Response);
        virtual void AsyncBindIpDnsFinished(hostent *Response);
@@ -77,7 +79,6 @@ private:
        unsigned int m_PortCache; /**< the port or -1 if the cache is invalided */
        char *m_BindIpCache; /**< the bind address */
 
-       bool m_LatchedDestruction; /**< should the connection object be destroyed? */
        CTrafficStats *m_Traffic; /**< the traffic statistics for this connection */
 
        void *m_BindAddr; /**< the bind address (an in_addr or in_addr6) */
index 2c053e2c0ed21cca205d8e7aefa940d0c79d7468..8273940d7fd9980fcbc5ae913a21dbe7e897a46b 100644 (file)
@@ -1522,8 +1522,12 @@ void CIRCConnection::AsyncDnsFinished(hostent *Response) {
  * @param Response the reponse from the DNS server
  */
 void CIRCConnection::AsyncBindIpDnsFinished(hostent *Response) {
-       if (Response == NULL && GetOwner() != NULL) {
-               g_Bouncer->LogUser(GetOwner(), "DNS request (vhost) for user %s failed.", GetOwner()->GetUsername());
+       if ((Response == NULL || Response->h_addr_list[0] == NULL) && GetOwner() != NULL) {
+               g_Bouncer->LogUser(GetOwner(), "DNS request (vhost) for user %s failed. Cancelling connection attempt.", GetOwner()->GetUsername());
+
+               m_LatchedDestruction = true;
+
+               return;
        }
 
        CConnection::AsyncBindIpDnsFinished(Response);
index c18415b9332552498bdc0dc3e06c7c84d4df9980..e3f9d18dd38c92f8c5f11e3aa027067cfbd5621f 100644 (file)
@@ -426,12 +426,18 @@ SOCKET SocketAndConnect(const char *Host, unsigned int Port, const char *BindIp)
  * @param Host the host's ip address
  * @param BindIp the ip address which should be used for binding the socket
  */
-SOCKET SocketAndConnectResolved(const sockaddr *Host, const sockaddr *BindIp) {
+SOCKET SocketAndConnectResolved(const sockaddr *Host, const sockaddr *BindIp, int *error) {
        unsigned long lTrue = 1;
        int Code, Size;
 
        SOCKET Socket = socket(Host->sa_family, SOCK_STREAM, IPPROTO_TCP);
 
+#ifdef _WIN32
+       *error = WSAGetLastError();
+#else
+       *error = errno;
+#endif
+
        if (Socket == INVALID_SOCKET) {
                return INVALID_SOCKET;
        }
@@ -439,7 +445,17 @@ SOCKET SocketAndConnectResolved(const sockaddr *Host, const sockaddr *BindIp) {
        ioctlsocket(Socket, FIONBIO, &lTrue);
 
        if (BindIp != NULL) {
-               bind(Socket, (sockaddr *)BindIp, SOCKADDR_LEN(BindIp->sa_family));
+               if (bind(Socket, (sockaddr *)BindIp, SOCKADDR_LEN(BindIp->sa_family)) != 0) {
+#ifdef _WIN32
+                       *error = WSAGetLastError();
+#else
+                       *error = errno;
+#endif
+
+                       closesocket(Socket);
+
+                       return INVALID_SOCKET;
+               }
        }
 
        Size = SOCKADDR_LEN(Host->sa_family);
@@ -447,15 +463,23 @@ SOCKET SocketAndConnectResolved(const sockaddr *Host, const sockaddr *BindIp) {
        Code = connect(Socket, Host, Size);
 
 #ifdef _WIN32
-       if (Code != 0 && WSAGetLastError() != WSAEWOULDBLOCK) {
+       *error = WSAGetLastError();
 #else
-       if (Code != 0 && errno != EINPROGRESS) {
+       *error = errno;
+#endif
+
+#ifdef _WIN32
+       if (Code != 0 && *error != WSAEWOULDBLOCK) {
+#else
+       if (Code != 0 && *error != EINPROGRESS) {
 #endif
                closesocket(Socket);
 
                return INVALID_SOCKET;
        }
 
+       *error = 0;
+
        return Socket;
 }
 
index bb964e272e4b81796bde6b1789a70fef22a6667d..abaa34de05703a64fcd09b2b692160f0c4298a22 100644 (file)
@@ -66,7 +66,7 @@ const char *ArgGet2(const tokendata_t& Tokens, unsigned int Arg);
 unsigned int ArgCount2(const tokendata_t& Tokens);
 
 SOCKET SocketAndConnect(const char *Host, unsigned int Port, const char *BindIp = NULL);
-SOCKET SocketAndConnectResolved(const sockaddr *Host, const sockaddr *BindIp);
+SOCKET SocketAndConnectResolved(const sockaddr *Host, const sockaddr *BindIp, int *error);
 
 SOCKET CreateListener(unsigned int Port, const char *BindIp = NULL, int Family = AF_INET);