Merge pull request #33 from selvanair/bugfix

Some small bug fixes
OpenVPN · Apr 12, 2016 · 72818bb · 72818bb
2 parents 5adc5be + bd2d80c
commit 72818bb
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 11 deletions.
diff --git a/misc.c b/misc.c
@@ -34,34 +34,51 @@
 #include "manage.h"
 #include "main.h"
 #include "misc.h"
+#include "main.h"
 
 /*
  * Helper function to do base64 conversion through CryptoAPI
+ * Returns TRUE on success, FALSE on error. Caller must free *output.
  */
-static void
+static BOOL
 Base64Encode(const char *input, int input_len, char **output)
 {
     DWORD output_len;
+
+    if (input_len == 0)
+    {
+        /* set output to empty string  -- matches the behavior in openvpn */
+        *output = calloc (1, sizeof(char));
+        return TRUE;
+    }
     if (!CryptBinaryToStringA((const BYTE *) input, (DWORD) input_len,
         CRYPT_STRING_BASE64, NULL, &output_len) || output_len == 0)
     {
+#ifdef DEBUG
+        PrintDebug (L"Error in CryptBinaryToStringA: input = '%.*S'", input_len, input);
+#endif
         *output = NULL;
-        return;
+        return FALSE;
     }
     *output = (char *)malloc(output_len);
     if (!CryptBinaryToStringA((const BYTE *) input, (DWORD) input_len,
         CRYPT_STRING_BASE64, *output, &output_len))
     {
+#ifdef DEBUG
+        PrintDebug (L"Error in CryptBinaryToStringA: input = '%.*S'", input_len, input);
+#endif
         free(*output);
         *output = NULL;
-        return;
+        return FALSE;
     }
     /* Trim trailing "\r\n" manually.
        Actually they can be stripped by adding CRYPT_STRING_NOCRLF to dwFlags,
        but Windows XP/2003 does not support this flag. */
     if(output_len > 1 && (*output)[output_len - 1] == '\x0A'
         && (*output)[output_len - 2] == '\x0D')
         (*output)[output_len - 2] = 0;
+
+    return TRUE;
 }
 
 /*
@@ -167,6 +184,9 @@ ManagementCommandFromInputBase64(connection_t *c, LPCSTR fmt, HWND hDlg,int id,
     LPSTR input, input2, input_b64, input2_b64, cmd;
     int input_len, input2_len, cmd_len, pos;
 
+    input_b64 = NULL;
+    input2_b64 = NULL;
+
     GetDlgItemTextUtf8(hDlg, id, &input, &input_len);
     GetDlgItemTextUtf8(hDlg, id2, &input2, &input2_len);
 
@@ -200,22 +220,29 @@ ManagementCommandFromInputBase64(connection_t *c, LPCSTR fmt, HWND hDlg,int id,
         }
     }
 
-    Base64Encode(input, input_len, &input_b64);
-    Base64Encode(input2, input2_len, &input2_b64);
+    if (!Base64Encode(input, input_len, &input_b64))
+        goto out;
+    if (!Base64Encode(input2, input2_len, &input2_b64))
+        goto out;
 
-    cmd_len = input_len * 2 + input2_len * 2 + strlen(fmt);
+    cmd_len = strlen(input_b64) + strlen(input2_b64) + strlen(fmt);
     cmd = malloc(cmd_len);
     if (cmd)
     {
         snprintf(cmd, cmd_len, fmt, input_b64, input2_b64);
         retval = ManagementCommand(c, cmd, NULL, regular);
         free(cmd);
     }
-    free(input_b64);
-    free(input2_b64);
 
 out:
     /* Clear buffers with potentially secret content */
+    if (input_b64)
+        memset(input_b64, 0, strlen(input_b64));
+    if (input2_b64)
+        memset(input2_b64, 0, strlen(input2_b64));
+    free(input_b64);
+    free(input2_b64);
+
     if (input_len)
     {
         memset(input, 'x', input_len);
@@ -317,7 +344,8 @@ BOOL IsUserAdmin(VOID)
                                   &AdministratorsGroup);
     if(b)
     {
-        CheckTokenMembership(NULL, AdministratorsGroup, &b);
+        if (!CheckTokenMembership(NULL, AdministratorsGroup, &b))
+            b = FALSE;
         FreeSid(AdministratorsGroup);
     }
 

diff --git a/openvpn.c b/openvpn.c
@@ -641,7 +641,7 @@ ThreadOpenVPNStatus(void *p)
                 c->state = suspending;
                 EnableWindow(GetDlgItem(c->hwndStatus, ID_DISCONNECT), FALSE);
                 EnableWindow(GetDlgItem(c->hwndStatus, ID_RESTART), FALSE);
-                SetMenuStatus(&o.conn[config], disconnecting);
+                SetMenuStatus(c, disconnecting);
                 SetDlgItemText(c->hwndStatus, ID_TXT_STATUS, LoadLocalizedString(IDS_NFO_STATE_WAIT_TERM));
                 SetEvent(c->exit_event);
                 break;

diff --git a/registry.c b/registry.c
@@ -247,6 +247,11 @@ int GetRegKey(const TCHAR name[], TCHAR *data, const TCHAR default_data[], DWORD
       RegCloseKey(openvpn_key_write);
 
     }
+  else
+    {
+      size /= sizeof(*data);
+      data[size - 1] = L'\0'; /* REG_SZ strings are not guaranteed to be null-terminated */
+    }
 
   RegCloseKey(openvpn_key);
 
@@ -270,7 +275,10 @@ LONG GetRegistryValue(HKEY regkey, const TCHAR *name, TCHAR *data, DWORD len)
   if (status != ERROR_SUCCESS || type != REG_SZ)
     return(0);
 
-  return(data_len / sizeof(*data));
+  data_len /= sizeof(*data);
+  data[data_len - 1] = L'\0'; /* REG_SZ strings are not guaranteed to be null-terminated */
+
+  return(data_len);
 
 }