Validate cynara sockets 97/252597/3
authorKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Mon, 25 Jan 2021 08:09:05 +0000 (09:09 +0100)
committerKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Wed, 3 Feb 2021 11:17:05 +0000 (12:17 +0100)
Socket descriptors received from cynara are not validated which may lead
to:
- m_socketDescriptionVector buffer overflow/UB
- reuse of already opened descriptors for cynara
- growing m_socketDescriptionVector
- closure of descriptors used by other parts of code
- more than one cynara socket opened at the same time

Change-Id: I5c6cd521fbde2a461f24e175571b74885d163b50

src/manager/main/socket-manager.cpp
src/manager/main/socket-manager.h

index 4c7a91f..ceb3834 100644 (file)
@@ -166,7 +166,6 @@ SocketManager::CreateDefaultReadSocketDescription(int sock, bool timeout)
        auto &desc = m_socketDescriptionVector[sock];
        desc.setListen(false);
        desc.setOpen(true);
-       desc.setCynara(false);
        desc.interfaceID = 0;
        desc.service = NULL;
 
@@ -189,6 +188,7 @@ SocketManager::CreateDefaultReadSocketDescription(int sock, bool timeout)
 }
 
 SocketManager::SocketManager() :
+       m_cynaraSocket(-1),
        m_maxDesc(0),
        m_working(false),
        m_counter(0)
@@ -332,7 +332,7 @@ void SocketManager::ReadyForRead(int sock)
                return;
        }
 
-       if (m_socketDescriptionVector[sock].isCynara()) {
+       if (sock == m_cynaraSocket) {
                m_cynara->ProcessSocket();
                return;
        }
@@ -375,7 +375,7 @@ void SocketManager::ReadyForRead(int sock)
 
 void SocketManager::ReadyForWrite(int sock)
 {
-       if (m_socketDescriptionVector[sock].isCynara()) {
+       if (sock == m_cynaraSocket) {
                m_cynara->ProcessSocket();
                return;
        }
@@ -779,6 +779,8 @@ void SocketManager::CloseSocket(int sock)
                return;
        }
 
+       assert(sock != m_cynaraSocket);
+
        GenericSocketService::CloseEvent event;
        event.connectionID.sock = sock;
        event.connectionID.counter = desc.counter;
@@ -806,22 +808,47 @@ void SocketManager::CloseSocket(int sock)
 void SocketManager::CynaraSocket(int oldFd, int newFd, bool isRW)
 {
        if (newFd != oldFd) {
-               if (newFd >= 0) {
-                       auto &desc = CreateDefaultReadSocketDescription(newFd, false);
-                       desc.service = nullptr;
-                       desc.setCynara(true);
-               }
-
                if (oldFd >= 0) {
+                       if (oldFd != m_cynaraSocket) {
+                               LogError("Invalid old cynara socket " << oldFd);
+                               return;
+                       }
+
                        auto &old = m_socketDescriptionVector[oldFd];
+
+                       assert(old.isOpen());
+
                        old.setOpen(false);
-                       old.setCynara(false);
+                       m_cynaraSocket = -1;
                        FD_CLR(oldFd, &m_writeSet);
                        FD_CLR(oldFd, &m_readSet);
                }
+
+               if (newFd >= 0) {
+                       if (m_cynaraSocket != -1) {
+                               LogError("Another cynara socket is already opened");
+                               return;
+                       }
+
+                       if (newFd < static_cast<int>(m_socketDescriptionVector.size()) &&
+                               m_socketDescriptionVector[newFd].isOpen())
+                       {
+                               LogError("New cynara socket " << newFd << " is already opened");
+                               return;
+                       }
+
+                       auto &desc = CreateDefaultReadSocketDescription(newFd, false);
+                       desc.service = nullptr;
+                       m_cynaraSocket = newFd;
+               }
        }
 
        if (newFd >= 0) {
+               if (m_cynaraSocket != newFd) {
+                       LogError("Invalid new cynara socket " << newFd);
+                       return;
+               }
+
                FD_SET(newFd, &m_readSet);
 
                if (isRW)
index 5319d24..0f8c4b9 100644 (file)
@@ -92,11 +92,6 @@ protected:
                        return m_flags & LISTEN;
                }
 
-               bool isCynara()
-               {
-                       return m_flags & CYNARA;
-               }
-
                bool isTimeout()
                {
                        return m_flags & TIMEOUT;
@@ -112,11 +107,6 @@ protected:
                        isSet ? m_flags |= LISTEN : m_flags &= ~LISTEN;
                }
 
-               void setCynara(bool isSet)
-               {
-                       isSet ? m_flags |= CYNARA : m_flags &= ~CYNARA;
-               }
-
                void setTimeout(bool isSet)
                {
                        isSet ? m_flags |= TIMEOUT : m_flags &= ~TIMEOUT;
@@ -141,8 +131,7 @@ protected:
        private:
                static const char LISTEN  = 1 << 0;
                static const char OPEN    = 1 << 1;
-               static const char CYNARA  = 1 << 2;
-               static const char TIMEOUT = 1 << 3;
+               static const char TIMEOUT = 1 << 2;
                int m_flags;
        };
 
@@ -175,6 +164,7 @@ protected:
        // support for generic event Queue
 
        SocketDescriptionVector m_socketDescriptionVector;
+       int m_cynaraSocket;
        fd_set m_readSet;
        fd_set m_writeSet;
        int m_maxDesc;