Check return value of SetProp() (#591)

* Check return value of SetProp

- If SetProp() is unsuccessful, we'll crash later when GetProp()
  returns null. Add a check, log the error and close the dialog.
  
  We could abort here, but closing the current dialog and possibly the
  corresponding connection, provides a chance for the user to fix the OOM
  condition which is the most likely cause of SetProp() failure.

- In pkcs11.c if SetProp() fails just do not use bold font for
  header instead of leaking the font resource.
  Also correct a bad fixup in commit 80697ecae6: hfontProp was not set!

  Github: Fixes OpenVPN/openvpn-gui#577
  
  Signed-off-by: Selva Nair <selva.nair@gmail.com>

as.c

@@ -230,7 +230,7 @@ CRDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam)
     switch (msg) {
         case WM_INITDIALOG:
             param = (auth_param_t*)lParam;
-            SetProp(hwndDlg, cfgProp, (HANDLE)param);
+            TRY_SETPROP(hwndDlg, cfgProp, (HANDLE)param);
 
             WCHAR *wstr = Widen(param->str);
             if (!wstr) {
@@ -280,7 +280,6 @@ CRDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam)
             return TRUE;
 
         case WM_NCDESTROY:
-            param = (auth_param_t*)GetProp(hwndDlg, cfgProp);
             RemoveProp(hwndDlg, cfgProp);
             break;
     }
@@ -614,7 +613,7 @@ ImportProfileFromURLDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lPa
     {
     case WM_INITDIALOG:
         type = (server_type_t) lParam;
-        SetProp(hwndDlg, cfgProp, (HANDLE)lParam);
+        TRY_SETPROP(hwndDlg, cfgProp, (HANDLE)lParam);
         SetStatusWinIcon(hwndDlg, ID_ICO_APP);
 
         if (type == server_generic)
@@ -700,6 +699,7 @@ ImportProfileFromURLDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lPa
         return TRUE;
 
     case WM_NCDESTROY:
+        RemoveProp(hwndDlg, cfgProp);
         break;
     }
 

openvpn.c

@@ -490,7 +490,7 @@ AutoCloseSetup(HWND hwnd, UINT btn, UINT timeout, UINT txtid, UINT txtres)
         SetTimer(hwnd, 1, 500, AutoCloseHandler); /* using timer id = 1 */
         if (txtid && txtres)
             SetDlgItemText(hwnd, txtid, LoadLocalizedString(txtres, timeout));
-        SetPropW(hwnd, L"AutoClose", (HANDLE) ac);
+        SetPropW(hwnd, L"AutoClose", (HANDLE) ac); /* failure not critical */
     }
 }
 
@@ -509,7 +509,7 @@ UserAuthDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam)
     case WM_INITDIALOG:
         /* Set connection for this dialog and show it */
         param = (auth_param_t *) lParam;
-        SetProp(hwndDlg, cfgProp, (HANDLE) param);
+        TRY_SETPROP(hwndDlg, cfgProp, (HANDLE) param);
         SetStatusWinIcon(hwndDlg, ID_ICO_APP);
 
         if (param->str)
@@ -699,7 +699,7 @@ GenericPassDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam)
     {
     case WM_INITDIALOG:
         param = (auth_param_t *) lParam;
-        SetProp(hwndDlg, cfgProp, (HANDLE) param);
+        TRY_SETPROP(hwndDlg, cfgProp, (HANDLE) param);
 
         WCHAR *wstr = Widen (param->str);
         if (!wstr)
@@ -893,7 +893,7 @@ PrivKeyPassDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam)
     case WM_INITDIALOG:
         /* Set connection for this dialog and show it */
         c = (connection_t *) lParam;
-        SetProp(hwndDlg, cfgProp, (HANDLE) c);
+        TRY_SETPROP(hwndDlg, cfgProp, (HANDLE) c);
         AppendTextToCaption (hwndDlg, c->config_name);
         if (RecallKeyPass(c->config_name, passphrase) && wcslen(passphrase)
             && c->failed_psw_attempts == 0)
@@ -1964,7 +1964,14 @@ StatusDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam)
         SetStatusWinIcon(hwndDlg, ID_ICO_CONNECTING);
 
         /* Set connection for this dialog */
-        SetProp(hwndDlg, cfgProp, (HANDLE) c);
+        if (!SetPropW(hwndDlg, cfgProp, (HANDLE) c))
+        {
+            MsgToEventLog(EVENTLOG_ERROR_TYPE, L"%hs:%d SetProp failed (error = 0x%08x)",
+                          __func__, __LINE__, GetLastError());
+            DisconnectDaemon(c);
+            DestroyWindow(hwndDlg);
+            break;
+        }
 
         /* Create log window */
         HWND hLogWnd = CreateWindowEx(0, RICHEDIT_CLASS, NULL,
@@ -2058,9 +2065,7 @@ StatusDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam)
     case WM_SHOWWINDOW:
         if (wParam == TRUE)
         {
-            c = (connection_t *) GetProp(hwndDlg, cfgProp);
-            if (c->hwndStatus)
-                SetFocus(GetDlgItem(c->hwndStatus, ID_EDT_LOG));
+            SetFocus(GetDlgItem(hwndDlg, ID_EDT_LOG));
         }
         return FALSE;
 

openvpn.h

@@ -25,6 +25,14 @@
 
 #include "options.h"
 
+#define TRY_SETPROP(hwnd, name, p)                                                   \
+            do { if (SetPropW(hwnd, name, p)) break;                                 \
+                 MsgToEventLog(EVENTLOG_ERROR_TYPE, L"%hs:%d GetProp returned null", \
+                               __func__, __LINE__);                                  \
+                 EndDialog(hwnd, IDABORT);                                           \
+                 return false;                                                       \
+               } while(0)
+
 BOOL StartOpenVPN(connection_t *);
 void StopOpenVPN(connection_t *);
 void DetachOpenVPN(connection_t *);

pkcs11.c

@@ -37,7 +37,7 @@
 #include <shlwapi.h>
 
 extern options_t o;
-static const wchar_t *hfontProp;
+static const wchar_t *hfontProp = L"header_font";
 
 /* state of list array */
 #define STATE_GET_COUNT 1
@@ -414,10 +414,13 @@ pkcs11_listview_init(HWND parent)
         lf.lfWeight = FW_BOLD;
 
         HFONT hfb = CreateFontIndirect(&lf);
-        if (hfb)
+        if (hfb && SetPropW(parent, hfontProp, (HANDLE)hfb))
         {
             SendMessage(ListView_GetHeader(lv), WM_SETFONT, (WPARAM)hfb, 1);
-            SetProp(parent, hfontProp, (HANDLE)hfb);
+        }
+        else if (hfb)
+        {
+            DeleteObject(hfb);
         }
     }
 
@@ -569,7 +572,7 @@ QueryPkcs11DialogProc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam)
     {
         case WM_INITDIALOG:
             c = (connection_t *) lParam;
-            SetProp(hwndDlg, cfgProp, (HANDLE)lParam);
+            TRY_SETPROP(hwndDlg, cfgProp, (HANDLE)lParam);
             SetStatusWinIcon(hwndDlg, ID_ICO_APP);
 
             /* init the listview and schedule a call to listview_fill */

proxy.c

@@ -341,7 +341,7 @@ ProxyAuthDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam)
     case WM_INITDIALOG:
         /* Set connection for this dialog and show it */
         c = (connection_t *) lParam;
-        SetProp(hwndDlg, cfgProp, (HANDLE) c);
+        TRY_SETPROP(hwndDlg, cfgProp, (HANDLE) c);
         if (c->state == resuming)
             ForceForegroundWindow(hwndDlg);
         else