Great discussion on the last bug. For those of you that took a look at it, it dealt with insecure use of cryptography resulting in exposure to dictionary attacks. Here's a new one:

class CDatabase
{
private:
  HANDLE m_hwndMutex;
public:
  void InitDBConnection(void);
  void UpdateDB(void);
};
 
int _tmain(int argc, _TCHAR* argv[])
{
  CDatabase obj1;
  obj1.InitDBConnectionEx();
  obj1.UpdateDB();
 
  return 0;
}
 
void CDatabase::InitDBConnection(void)
{
  SECURITY_DESCRIPTOR SecurityDescriptor;
  InitializeSecurityDescriptor(&SecurityDescriptor, SECURITY_DESCRIPTOR_REVISION);
 
  SetSecurityDescriptorDacl(&SecurityDescriptor, TRUE, NULL, FALSE);
 
  SECURITY_ATTRIBUTES secAttribMutex;
  secAttribMutex.nLength = sizeof(SECURITY_ATTRIBUTES);
  secAttribMutex.lpSecurityDescriptor = &SecurityDescriptor;
  secAttribMutex.bInheritHandle = TRUE;
 
  m_hwndMutex = ::CreateMutex(&secAttribMutex,FALSE,"GUARD_DB_ACCESS_MUTEX");
 
  return;
}
 
void CDatabase::UpdateDB(void)
{
  //Acquire the db mutex
  WaitForSingleObject(m_hwndMutex,INFINITE);
 
  //Update the database
  //
 
 
  //Release the db mutex
  ReleaseMutex(m_hwndMutex);
 
  return;
}

 

Solution

            Kernel objects are used for inter process synchronization purposes viz. guarding access to the database, signaling occurrence of an event etc. Insecure creation of kernel objects may allow a malicious attacker to cause denial of service attacks. In the above example, in the insecure approach, a malicious attacker can lock the mutex and deny the valid application access to the database. Care must be taken while creating kernel objects to make sure that they do not exist before you actually create them. If they already exist, then before continuing the ownership and the access control on the kernel object must be changed. In the event that the kernel object did not exist before care must be taken to enforce strong access control on the kernel object.

 

void CDatabase::InitDBConnection(void)
{
  m_hwndMutex = ::OpenMutex(MUTEX_ALL_ACCESS, TRUE, "GUARD_DB_ACCESS");
  if(NULL != m_hwndMutex)
  {
    //The mutex has already been created by some other process!
    //Either return an error OR change the ownership and the allowed access of the already existing mutex.
  }
 
  SECURITY_DESCRIPTOR SecurityDescriptor;
InitializeSecurityDescriptor(&SecurityDescriptor,SECURITY_DESCRIPTOR_REVISION);
 
  char strUserName[2048];
  ULONG uLen = 2048;
  ::GetUserName(strUserName, &uLen);
 
  PSID psidUser = ::LocalAlloc(LPTR, sizeof(SID));
  DWORD dwSidSize = sizeof(SID);
  char strReferencedDomainName[2048]="";
  DWORD dwDomainNameLen = sizeof(strReferencedDomainName);
  SID_NAME_USE sidAccType;
 
  BOOL bSuccess = ::LookupAccountName(NULL, strUserName, psidUser, &dwSidSize, strReferencedDomainName, &dwDomainNameLen, &sidAccType);
  if(FALSE == bSuccess)
  {
    DWORD dwErrorcode = ::GetLastError();
    if (ERROR_INSUFFICIENT_BUFFER == dwErrorcode)
    {
      psidUser = ::LocalAlloc(LPTR, dwSidSize);
      bSuccess = ::LookupAccountName(NULL, strUserName, psidUser, &dwSidSize, strReferencedDomainName, &dwDomainNameLen, &sidAccType);
    }
  }
 
  PACL pAcl = NULL;
  DWORD cbAcl = 0;
  cbAcl = sizeof(ACL) + sizeof(ACCESS_ALLOWED_ACE) - sizeof(DWORD) + GetLengthSid(psidUser);
  pAcl = (ACL*)LocalAlloc(LPTR, cbAcl);
  InitializeAcl(pAcl, cbAcl, ACL_REVISION);
 
  bSuccess = ::AddAccessAllowedAce(pAcl, ACL_REVISION_DS, GENERIC_ALL , psidUser);

  //Only allow access to required users
  SetSecurityDescriptorDacl(&SecurityDescriptor, TRUE, pAcl, FALSE);
 
  SECURITY_ATTRIBUTES secAttribMutex;
  secAttribMutex.nLength = sizeof(SECURITY_ATTRIBUTES);
  secAttribMutex.lpSecurityDescriptor = &SecurityDescriptor;
  secAttribMutex.bInheritHandle = TRUE;
 
 
  m_hwndMutex = ::CreateMutex(&secAttribMutex,FALSE,"GUARD_DB_ACCESS_MUTEX");
 
  return;
}