Issue 1283 - wrong usage of memset, fix sizeof, make proper initializing

src/plugin/PluginClass.cpp

 #include "PluginStdAfx.h"
 
 #include "PluginClass.h"
 #include "PluginSettings.h"
 #include "PluginSystem.h"
 #ifdef SUPPORT_FILTER
 #include "PluginFilter.h"
 #endif
 #include "PluginMimeFilterClient.h"
 #include "PluginClient.h"
 #include "PluginClientFactory.h"
 #include "PluginMutex.h"
 #include "sddl.h"
 #include "PluginUtil.h"
 #include "PluginUserSettings.h"
 #include "../shared/Utils.h"
 #include "../shared/Dictionary.h"
 #include <thread>
+#include <array>
 
 #ifdef DEBUG_HIDE_EL
 DWORD profileTime = 0;
 #endif
 
 typedef HANDLE (WINAPI *OPENTHEMEDATA)(HWND, LPCWSTR);
 typedef HRESULT (WINAPI *DRAWTHEMEBACKGROUND)(HANDLE, HDC, INT, INT, LPRECT, LPRECT);
 typedef HRESULT (WINAPI *CLOSETHEMEDATA)(HANDLE);
 
 HICON CPluginClass::s_hIcons[ICON_MAX] = { NULL, NULL, NULL };
@@ skipping 384 matching lines @@
 
   return IObjectWithSiteImpl<CPluginClass>::SetSite(unknownSite);
 }
 
 bool CPluginClass::IsStatusBarEnabled()
 {
   DEBUG_GENERAL("IsStatusBarEnabled start");
   HKEY pHkey;
   HKEY pHkeySub;
   RegOpenCurrentUser(KEY_QUERY_VALUE, &pHkey);
   DWORD trueth = 1;

[Oleksandr, 2014/10/06 20:19:32]

That typo really jumps at me with this change. Mind changing it to "truth" in
this patch?

-  DWORD truethSize = sizeof(DWORD);
+  DWORD truethSize = sizeof(trueth);
   RegOpenKey(pHkey, L"Software\\Microsoft\\Internet Explorer\\Main", &pHkeySub);
   LONG res = RegQueryValueEx(pHkeySub, L"StatusBarWeb", NULL, NULL, (BYTE*)&trueth, &truethSize);
   RegCloseKey(pHkey);
   if (res != ERROR_SUCCESS)
   {
     res = RegOpenKey(pHkey, L"Software\\Microsoft\\Internet Explorer\\MINIE", &pHkeySub);
     if (res == ERROR_SUCCESS)
     {
       LONG res = RegQueryValueEx(pHkeySub, L"ShowStatusBar", NULL, NULL, (BYTE*)&trueth, &truethSize);
       if (res == ERROR_SUCCESS)
@@ skipping 43 matching lines @@
         }
         // Ask if a user wants to enable a status bar automatically
         LRESULT res = MessageBox((HWND)m_hTabWnd,
             dictionary->Lookup("status-bar", "question").c_str(),
             dictionary->Lookup("status-bar", "title").c_str(),
             MB_YESNO);
         if (res == IDYES)
         {
           DWORD trueth = 1;
           regRes = RegOpenKey(pHkey, L"Software\\Microsoft\\Internet Explorer\\MINIE", &pHkeySub);
-          regRes = RegSetValueEx(pHkeySub, L"ShowStatusBar", 0, REG_DWORD, (BYTE*)&trueth, sizeof(DWORD));
+          regRes = RegSetValueEx(pHkeySub, L"ShowStatusBar", 0, REG_DWORD, (BYTE*)&trueth, sizeof(trueth));
           regRes = RegCloseKey(pHkeySub);
           regRes = RegOpenKey(pHkey, L"Software\\Microsoft\\Internet Explorer\\Main", &pHkeySub);
-          regRes = RegSetValueEx(pHkeySub, L"StatusBarWeb", 0, REG_DWORD, (BYTE*)&trueth, sizeof(DWORD));
+          regRes = RegSetValueEx(pHkeySub, L"StatusBarWeb", 0, REG_DWORD, (BYTE*)&trueth, sizeof(trueth));
           regRes = RegCloseKey(pHkeySub);
           hr = browser->put_StatusBar(TRUE);
           if (FAILED(hr))
           {
             DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_PUT_STATUSBAR, "Class::Enable statusbar");
           }
           CreateStatusBarPane();
 
           // We need to restart the tab now, to enable the status bar properly
           VARIANT vFlags;
@@ skipping 304 matching lines @@
       }
     }
   }
   s_criticalSectionLocal.Unlock();
 
   // Register pane class
   if (!GetAtomPaneClass())
   {
     WNDCLASSEX wcex;
 
-    wcex.cbSize = sizeof(WNDCLASSEX);
+    wcex.cbSize = sizeof(wcex);
     wcex.style = 0;
     wcex.lpfnWndProc = (WNDPROC)PaneWindowProc;
     wcex.cbClsExtra = 0;
     wcex.cbWndExtra = 0;
     wcex.hInstance = _Module.m_hInst;
     wcex.hIcon = NULL;
     wcex.hCursor = NULL;
     wcex.hbrBackground = NULL;
     wcex.lpszMenuName = NULL;
     wcex.lpszClassName = STATUSBAR_PANE_NAME;
@@ skipping 55 matching lines @@
   s_criticalSectionLocal.Unlock();
   return true;
 }
 
 bool CPluginClass::CreateStatusBarPane()
 {
   CriticalSection::Lock lock(m_csStatusBar);
 
   CPluginClient* client = CPluginClient::GetInstance();
 
-  wchar_t szClassName[MAX_PATH];
+  std::array<wchar_t, MAX_PATH> className;
   // Get browser window and url
   HWND hBrowserWnd = GetBrowserHWND();
   if (!hBrowserWnd)
   {
     DEBUG_ERROR_LOG(0, PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_NO_STATUSBAR_BROWSER, "Class::CreateStatusBarPane - No status bar")
     return false;
   }
 
   // Looking for a TabWindowClass window in IE7
   // the last one should be parent for statusbar
   HWND hWndStatusBar = NULL;
 
   HWND hTabWnd = ::GetWindow(hBrowserWnd, GW_CHILD);
   UINT amoundOfNewTabs = 0;
   HWND uniqueNewTab = NULL;
   while (hTabWnd)
   {
-    memset(szClassName, 0, MAX_PATH);

[Eric, 2014/09/26 18:07:23]

The title of the review says "wrong usage of memset". Is this what you meant?
It's not wrong so much as unnecessary.

-    GetClassName(hTabWnd, szClassName, MAX_PATH);
+    className.fill(L'\0');

[Eric, 2014/09/26 18:07:23]

Are these calls to 'fill' really necessary?

The only case I can think of is when GetClassName writes nothing to the buffer,
but in that case, it would be better to handle the return value as an error and
not ignore it. Or if we don't want to do that, just writing an initial
terminating null would be sufficient. But it's probably better to handle the
error correctly.

+    GetClassName(hTabWnd, className.data(), className.size());

[Eric, 2014/09/26 18:07:23]

As long as we're making a modification, we might as well change to
'GetClassNameW', which we've been slowly doing elsewhere.

[Oleksandr, 2014/10/06 20:19:32]

+1
On 2014/09/26 18:07:23, Eric wrote:

[sergei, 2014/10/08 15:06:52]

Despite it's not a real performance trouble, I've fixed it.

 
-    if (wcscmp(szClassName, L"TabWindowClass") == 0 || wcscmp(szClassName,L"Frame Tab") == 0)
+    if (wcscmp(className.data(), L"TabWindowClass") == 0 || wcscmp(className.data(), L"Frame Tab") == 0)
     {
       // IE8 support
       HWND hTabWnd2 = hTabWnd;
-      if (wcscmp(szClassName,L"Frame Tab") == 0)
+      if (wcscmp(className.data(), L"Frame Tab") == 0)
       {
         hTabWnd2 = ::FindWindowEx(hTabWnd2, NULL, L"TabWindowClass", NULL);
       }
 
       if (hTabWnd2)
       {
         DWORD nProcessId;
         ::GetWindowThreadProcessId(hTabWnd2, &nProcessId);
         if (::GetCurrentProcessId() == nProcessId)
         {
@@ skipping 26 matching lines @@
         }
       }
     }
 
     hTabWnd = ::GetWindow(hTabWnd, GW_HWNDNEXT);
   }
 
   HWND hWnd = ::GetWindow(hBrowserWnd, GW_CHILD);
   while (hWnd)
   {
-    memset(szClassName, 0, MAX_PATH);
-    ::GetClassName(hWnd, szClassName, MAX_PATH);
+    className.fill(L'\0');
+    ::GetClassName(hWnd, className.data(), className.size());
 
-    if (wcscmp(szClassName,L"msctls_statusbar32") == 0)
+    if (wcscmp(className.data(), L"msctls_statusbar32") == 0)
     {
       hWndStatusBar = hWnd;
       break;
     }
 
     hWnd = ::GetWindow(hWnd, GW_HWNDNEXT);
   }
 
   if (!hWndStatusBar)
   {
@@ skipping 176 matching lines @@
   HINSTANCE hInstance = _AtlBaseModule.GetModuleInstance();
 
   HMENU hMenu = ::LoadMenu(hInstance, MAKEINTRESOURCE(IDR_MENU1));
 
   HMENU hMenuTrackPopup = GetSubMenu(hMenu, 0);
 
   SetMenuBar(hMenuTrackPopup, url);
 
   return hMenuTrackPopup;
 }
-BOOL CreateLowProcess(WCHAR* wszProcessName, WCHAR* cmdLine)
-{
-
-  BOOL                  fRet;
-  HANDLE                hToken        = NULL;
-  HANDLE                hNewToken     = NULL;
-  PSID                  pIntegritySid = NULL;
-  TOKEN_MANDATORY_LABEL TIL           = {0};
-  PROCESS_INFORMATION   ProcInfo      = {0};
-  STARTUPINFO           StartupInfo   = {0};
-
-
-
-  // Low integrity SID
-  WCHAR wszIntegritySid[20] = L"S-1-16-4096";
-
-
-  fRet = OpenProcessToken(GetCurrentProcess(),
-    TOKEN_DUPLICATE |
-    TOKEN_ADJUST_DEFAULT |
-    TOKEN_QUERY |
-    TOKEN_ASSIGN_PRIMARY,
-    &hToken);
-
-  if (!fRet)
-  {
-    goto CleanExit;
-  }
-
-  fRet = DuplicateTokenEx(hToken,
-    0,
-    NULL,
-    SecurityImpersonation,
-    TokenPrimary,
-    &hNewToken);
-
-  if (!fRet)
-  {
-    goto CleanExit;
-  }
-
-  fRet = ConvertStringSidToSid(wszIntegritySid, &pIntegritySid);
-
-  if (!fRet)
-  {
-    goto CleanExit;
-  }
-
-
-  TIL.Label.Attributes = SE_GROUP_INTEGRITY;
-  TIL.Label.Sid        = pIntegritySid;
-
-
-  //
-  // Set the process integrity level
-  //
-
-  fRet = SetTokenInformation(hNewToken,
-    TokenIntegrityLevel,
-    &TIL,
-    sizeof(TOKEN_MANDATORY_LABEL) + GetLengthSid(pIntegritySid));
-
-  if (!fRet)
-  {
-    goto CleanExit;
-  }
-
-  //
-  // Create the new process at Low integrity
-  //
-
-  fRet  = CreateProcessAsUser(hNewToken,
-    wszProcessName,
-    cmdLine,
-    NULL,
-    NULL,
-    FALSE,
-    0,
-    NULL,
-    NULL,
-    &StartupInfo,
-    &ProcInfo);
-
-
-CleanExit:
-
-  if (ProcInfo.hProcess != NULL)
-  {
-    CloseHandle(ProcInfo.hProcess);
-  }
-
-  if (ProcInfo.hThread != NULL)
-  {
-    CloseHandle(ProcInfo.hThread);
-  }
-
-  LocalFree(pIntegritySid);
-
-  if (hNewToken != NULL)
-  {
-    CloseHandle(hNewToken);
-  }
-
-  if (hToken != NULL)
-  {
-    CloseHandle(hToken);
-  }
-
-  return fRet;
-}
 
 void CPluginClass::DisplayPluginMenu(HMENU hMenu, int nToolbarCmdID, POINT pt, UINT nMenuFlags)
 {
   CPluginClient* client = CPluginClient::GetInstance();
 
   // Create menu parent window
   HWND hMenuWnd = ::CreateWindowEx(
     NULL,
     MAKEINTATOM(GetAtomPaneClass()),
     L"",
@@ skipping 111 matching lines @@
   {
 #ifdef SUPPORT_WHITELIST
     s_menuDomains.clear();
 #endif
   }
   s_criticalSectionLocal.Unlock();
 
 
   Dictionary* dictionary = Dictionary::GetInstance();
 
-  MENUITEMINFOW fmii;
-  memset(&fmii, 0, sizeof(MENUITEMINFO));
-  fmii.cbSize = sizeof(MENUITEMINFO);
+  MENUITEMINFOW fmii = {};
+  fmii.cbSize = sizeof(fmii);
 
-  MENUITEMINFOW miiSep;
-  memset(&miiSep, 0, sizeof(MENUITEMINFO));
-  miiSep.cbSize = sizeof(MENUITEMINFO);
+  MENUITEMINFOW miiSep = {};
+  miiSep.cbSize = sizeof(miiSep);
   miiSep.fMask = MIIM_TYPE | MIIM_FTYPE;
   miiSep.fType = MFT_SEPARATOR;
 
   CPluginClient* client = CPluginClient::GetInstance();
 
   CPluginSettings* settings = CPluginSettings::GetInstance();
 
 #ifdef SUPPORT_WHITELIST
   {
     ctext = dictionary->Lookup("menu", "menu-disable-on-site");
@@ skipping 82 matching lines @@
   DWORD nProcessId;
   ::GetWindowThreadProcessId(hWndToolBar, &nProcessId);
 
   if (hWndToolBar && ::GetCurrentProcessId() == nProcessId)
   {
     ::ScreenToClient(hWndToolBar, &pt);
     int nButton = (int)::SendMessage(hWndToolBar, TB_HITTEST, 0, (LPARAM)&pt);
 
     if (nButton > 0)
     {
-      TBBUTTON pTBBtn;
-      memset(&pTBBtn, 0, sizeof(TBBUTTON));
+      TBBUTTON pTBBtn = {};
 
       if (SendMessage(hWndToolBar, TB_GETBUTTON, nButton, (LPARAM)&pTBBtn))
       {
         RECT rcButton;
         nIDCommand = pTBBtn.idCommand;
 
         if (SendMessage(hWndToolBar, TB_GETRECT, nIDCommand, (LPARAM)&rcButton))
         {
           pt.x = rcButton.left;
           pt.y = rcButton.bottom;
@@ skipping 408 matching lines @@
   return icon;
 }
 
 ATOM CPluginClass::GetAtomPaneClass()
 {
   return s_atomPaneClass;
 }
 
 HWND CPluginClass::GetTabHWND() const
 {
-  wchar_t szClassName[MAX_PATH];
+  std::array<wchar_t, MAX_PATH> className;
   // Get browser window and url
   HWND hBrowserWnd = GetBrowserHWND();
   if (!hBrowserWnd)
   {
     DEBUG_ERROR_LOG(0, PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_NO_STATUSBAR_BROWSER, "Class::GetTabWindow - No tab window")
       s_criticalSectionWindow.Unlock();
 
     return false;
   }
 
   // Looking for a TabWindowClass window in IE7
 
   HWND hTabWnd = ::GetWindow(hBrowserWnd, GW_CHILD);
   while (hTabWnd)
   {
-    memset(szClassName, 0, MAX_PATH);
-    GetClassName(hTabWnd, szClassName, MAX_PATH);
+    className.fill(L'\0');
+    GetClassName(hTabWnd, className.data(), className.size());
 
-    if (wcscmp(szClassName, L"TabWindowClass") == 0 || wcscmp(szClassName, L"Frame Tab") == 0)
+    if (wcscmp(className.data(), L"TabWindowClass") == 0 || wcscmp(className.data(), L"Frame Tab") == 0)
     {
       // IE8 support
       HWND hTabWnd2 = hTabWnd;
-      if (wcscmp(szClassName, L"Frame Tab") == 0)
+      if (wcscmp(className.data(), L"Frame Tab") == 0)
       {
         hTabWnd2 = ::FindWindowEx(hTabWnd2, NULL, L"TabWindowClass", NULL);
       }
 
       if (hTabWnd2)
       {
         DWORD nProcessId;
         ::GetWindowThreadProcessId(hTabWnd2, &nProcessId);
         if (::GetCurrentProcessId() == nProcessId)
         {
@@ skipping 23 matching lines @@
         }
       }
     }
 
     hTabWnd = ::GetWindow(hTabWnd, GW_HWNDNEXT);
   }
 
   return hTabWnd;
 
 }