Merge pull request #9589 from RussWhitehead/coverity640

HPCC-17042 Coverity: Fix numerous LDAP issue found by Coverity

Reviewed-By: Gavin Halliday <gavin.halliday@lexisnexis.com>
Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

system/security/LdapSecurity/aci.cpp

@@ -441,6 +441,9 @@ public:
         const char* bptr = acistr;
         const char* eptr = acistr;
 
+        m_isDeny = false;//set later in this ctor
+        m_permission = NewSecAccess_None;//set later in this ctor
+
         // <OID>
         while(*bptr == ' ')
             bptr++;
@@ -510,7 +513,6 @@ public:
         }
 
         //calculate the secperm
-        m_permission = 0;
         if(strchr(m_perms.str(), 'r') != NULL && strchr(m_perms.str(), 'w') != NULL && strchr(m_perms.str(), 'd') != NULL)
         {
             m_permission |= NewSecAccess_Full;
@@ -895,6 +897,8 @@ AciProcessor::AciProcessor(IPropertyTree* cfg)
 
     m_sidcache.setown(createPTree());
     m_cfg->getProp(".//@ldapAddress", m_server);
+    m_ldap_client = nullptr;
+    m_servertype = LDAPSERVER_UNKNOWN;
 }
 
 bool AciProcessor::getPermissions(ISecUser& user, IArrayOf<CSecurityDescriptor>& sdlist, IArrayOf<ISecResource>& resources)
@@ -972,7 +976,7 @@ void AciProcessor::cacheSid(const char* name, int len, const void* sidbuf)
 void AciProcessor::lookupSid(const char* act_name, MemoryBuffer& act_sid, ACT_TYPE acttype)
 {
     throw MakeStringException(-1, "You shouldn't need function lookupSid");
-
+/*preserve dead code for reference
     act_sid.clear();
 
     MemoryBuffer mb;
@@ -989,6 +993,7 @@ void AciProcessor::lookupSid(const char* act_name, MemoryBuffer& act_sid, ACT_TY
             cacheSid(act_name, act_sid.length(), (const void*)act_sid.toByteArray());
         }
     }
+*/
 }
 
 int AciProcessor::sdSegments(CSecurityDescriptor* sd)

system/security/LdapSecurity/ldapconnection.cpp

@@ -140,6 +140,7 @@ public:
     CHostManager()
     {
         m_populated = false;
+        m_curHostIdx = 0;
     }
 
     void populateHosts(const char* addrlist)
@@ -773,7 +774,7 @@ public:
             m_useSSL = (0 == stricmp(proto, "ldaps") ? true : false);
         }
 
-        int rc;
+        int rc = LDAP_SERVER_DOWN;//assume bad things
         StringBuffer hostbuf;
         for (int numHosts=0; numHosts < m_ldapconfig->getHostCount(); numHosts++)
         {
@@ -1400,6 +1401,7 @@ public:
         m_pp = NULL;
         //m_defaultFileScopePermission = -2;
         //m_defaultWorkunitScopePermission = -2;
+        m_domainPwdsNeverExpire = false;
     }
 
     virtual void init(IPermissionProcessor* pp)
@@ -1935,6 +1937,10 @@ public:
         if(infotype && stricmp(infotype, "sudoers") == 0)
         {
             CLdapSecUser* ldapuser = dynamic_cast<CLdapSecUser*>(&user);
+            if (ldapuser == nullptr)
+            {
+                throw MakeStringException(-1, "Unable to cast user %s to CLdapSecUser", username);
+            }
 
             TIMEVAL timeOut = {m_ldapconfig->getLdapTimeout(),0};
             Owned<ILdapConnection> lconn = m_connections->getConnection();
@@ -2185,12 +2191,13 @@ public:
         // Since we've got the SID for the user, cache it for later uses.
         MemoryBuffer mb;
         if(m_pp != NULL)
-            m_pp->getCachedSid(ldapuser->getName(), mb);
-        if(mb.length() == 0)
         {
-            m_pp->cacheSid(ldapuser->getName(), usersidbuf.length(), usersidbuf.toByteArray());
+            m_pp->getCachedSid(ldapuser->getName(), mb);
+            if(mb.length() == 0)
+            {
+                m_pp->cacheSid(ldapuser->getName(), usersidbuf.length(), usersidbuf.toByteArray());
+            }
         }
-
         return ldapuser;
     }
 
@@ -2736,6 +2743,10 @@ public:
                 throw MakeStringException(-1, "posixAccount isn't applicable to Active Directory");
 
             CLdapSecUser* ldapuser = dynamic_cast<CLdapSecUser*>(&user);
+            if (ldapuser == nullptr)
+            {
+                throw MakeStringException(-1, "Unable to cast user %s to CLdapSecUser", username);
+            }
 
             char* oc_values[] = {"posixAccount", NULL};
             LDAPMod oc_attr = {
@@ -2825,9 +2836,6 @@ public:
             }
             else
             {
-
-                CLdapSecUser* ldapuser = dynamic_cast<CLdapSecUser*>(&user);
-
                 char* oc_values[] = {"posixAccount", NULL};
                 LDAPMod oc_attr = {
                     LDAP_MOD_DELETE,
@@ -2889,6 +2897,10 @@ public:
         else if(stricmp(type, "sudoersadd") == 0)
         {
             CLdapSecUser* ldapuser = dynamic_cast<CLdapSecUser*>(&user);
+            if (ldapuser == nullptr)
+            {
+                throw MakeStringException(-1, "Unable to cast user %s to CLdapSecUser", username);
+            }
 
             char *cn_values[] = {(char*)username, NULL };
             LDAPMod cn_attr =
@@ -2991,6 +3003,10 @@ public:
         else if(stricmp(type, "sudoersupdate") == 0)
         {
             CLdapSecUser* ldapuser = dynamic_cast<CLdapSecUser*>(&user);
+            if (ldapuser == nullptr)
+            {
+                throw MakeStringException(-1, "Unable to cast user %s to CLdapSecUser", username);
+            }
 
             char* sudoHost = (char*)ldapuser->getSudoHost();
             char* sudoCommand = (char*)ldapuser->getSudoCommand();
@@ -5356,7 +5372,7 @@ private:
             prevptr = nextptr + 2;
             nextptr = strstr(prevptr, "::");
         }
-        if(prevptr != NULL && *prevptr != '\0')
+        if(*prevptr != '\0')
             ldapname.append(prevptr);
     }
 

system/security/LdapSecurity/ldapsecurity.cpp

@@ -28,6 +28,10 @@ CLdapSecUser::CLdapSecUser(const char *name, const char *pw) :
     m_pw(pw), m_authenticateStatus(AS_UNKNOWN)
 {
     setName(name);
+    setUserID(0);
+    setPosixenabled(false);
+    setSudoersEnabled(false);
+    setInSudoers(false);
 }
 
 CLdapSecUser::~CLdapSecUser()

system/security/LdapSecurity/permissions.cpp

@@ -58,6 +58,7 @@ PermissionProcessor::PermissionProcessor(IPropertyTree* config)
 
     m_sidcache.setown(createPTree());
     m_cfg->getProp(".//@ldapAddress", m_server);
+    m_ldap_client = nullptr;
 }
 
 SecAccessFlags PermissionProcessor::ldap2sec(unsigned ldapperm)
@@ -406,12 +407,6 @@ bool GetAclInformation(PACL pAcl, void* pAclInformation, DWORD nAclInformationLe
         return false;
     }
 
-    if(pAcl == NULL)
-    {
-        DBGLOG("GetAclInformation: PACL is NULL");
-        return false;
-    }
-
     PACL_SIZE_INFORMATION pinfo = (PACL_SIZE_INFORMATION)pAclInformation;
     pinfo->AceCount = pAcl->AceCount;
 

system/security/LdapSecurity/permissions.hpp

@@ -34,7 +34,6 @@ private:
     StringAttr   m_name;
     StringAttr   m_relativeBasedn;
     MemoryBuffer m_descriptor;
-    unsigned     m_permissions;
     StringAttr   m_dn;
     StringAttr   m_objectClass;
 

system/security/shared/caching.hpp

@@ -130,6 +130,9 @@ public:
 
 // main cache that stores all user-specific caches (defined by CResPermissionsCache above)
 //
+
+#define DEFAULT_CACHE_TIMEOUT_SECONDS 10
+
 class CPermissionsCache : public CInterface
 {
 public:
@@ -141,6 +144,7 @@ public:
         m_lastManagedFileScopesRefresh = 0;
         m_defaultPermission = SecAccess_Unknown;
         m_secMgrClass.set(_secMgrClass);
+        m_transactionalCacheTimeout = DEFAULT_CACHE_TIMEOUT_SECONDS;
     }
 
     virtual ~CPermissionsCache();
@@ -167,7 +171,6 @@ public:
     virtual void add (ISecUser& sec_user);
     virtual void removeFromUserCache(ISecUser& sec_user);
 
-#define DEFAULT_CACHE_TIMEOUT_SECONDS 10
     void  setCacheTimeout(int timeoutSeconds)
     {
         m_cacheTimeout = timeoutSeconds;