Ensure strings read from registry are null-terminated

- Strings stored in registry are not guaranteed to be null-terminated.
  So, use RegGetValue() instead of RegQueryValueEx() as the former
  adds null termination to the returned string if missing.
  (Needs Windows Vista+)

- While at it also add a default value parameter to GetRegString()
  to process optional registry values (such as ovpn_admin_group)
  without causing an otherwise confusing error logged to the
  eventlog[*].

[*] see Trac: #892

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1511026858-23281-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15893.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit b1263b06db40f21a8fd20e0efd0c12e37ce89a2c)

diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
index 2311583cb024319a158db964feac3c7cbf886032..200a47b5690491aa47e56e8e67214a9e3b2ce906 100644 (file)
--- a/src/openvpnserv/common.c
+++ b/src/openvpnserv/common.c
@@ -57,14 +57,18 @@ openvpn_sntprintf(LPTSTR str, size_t size, LPCTSTR format, ...)
 }
 
 static DWORD
-GetRegString(HKEY key, LPCTSTR value, LPTSTR data, DWORD size)
+GetRegString(HKEY key, LPCTSTR value, LPTSTR data, DWORD size, LPCTSTR default_value)
 {
-    DWORD type;
-    LONG status = RegQueryValueEx(key, value, NULL, &type, (LPBYTE) data, &size);
+    LONG status = RegGetValue(key, NULL, value, RRF_RT_REG_SZ|RRF_RT_REG_EXPAND_SZ,
+                              NULL, (LPBYTE) data, &size);
 
-    if (status == ERROR_SUCCESS && type != REG_SZ)
+    if (status == ERROR_FILE_NOT_FOUND && default_value)
     {
-        status = ERROR_DATATYPE_MISMATCH;
+        size_t len = size/sizeof(data[0]);
+        if (openvpn_sntprintf(data, len, default_value) > 0)
+        {
+            status = ERROR_SUCCESS;
+        }
     }
 
     if (status != ERROR_SUCCESS)
@@ -95,48 +99,48 @@ GetOpenvpnSettings(settings_t *s)
         return MsgToEventLog(M_SYSERR, TEXT("Could not open Registry key HKLM\\%s not found"), reg_path);
     }
 
-    error = GetRegString(key, TEXT("exe_path"), s->exe_path, sizeof(s->exe_path));
+    error = GetRegString(key, TEXT("exe_path"), s->exe_path, sizeof(s->exe_path), NULL);
     if (error != ERROR_SUCCESS)
     {
         goto out;
     }
 
-    error = GetRegString(key, TEXT("config_dir"), s->config_dir, sizeof(s->config_dir));
+    error = GetRegString(key, TEXT("config_dir"), s->config_dir, sizeof(s->config_dir), NULL);
     if (error != ERROR_SUCCESS)
     {
         goto out;
     }
 
-    error = GetRegString(key, TEXT("config_ext"), s->ext_string, sizeof(s->ext_string));
+    error = GetRegString(key, TEXT("config_ext"), s->ext_string, sizeof(s->ext_string), NULL);
     if (error != ERROR_SUCCESS)
     {
         goto out;
     }
 
-    error = GetRegString(key, TEXT("log_dir"), s->log_dir, sizeof(s->log_dir));
+    error = GetRegString(key, TEXT("log_dir"), s->log_dir, sizeof(s->log_dir), NULL);
     if (error != ERROR_SUCCESS)
     {
         goto out;
     }
 
-    error = GetRegString(key, TEXT("priority"), priority, sizeof(priority));
+    error = GetRegString(key, TEXT("priority"), priority, sizeof(priority), NULL);
     if (error != ERROR_SUCCESS)
     {
         goto out;
     }
 
-    error = GetRegString(key, TEXT("log_append"), append, sizeof(append));
+    error = GetRegString(key, TEXT("log_append"), append, sizeof(append), NULL);
     if (error != ERROR_SUCCESS)
     {
         goto out;
     }
 
     /* read if present, else use default */
-    error = GetRegString(key, TEXT("ovpn_admin_group"), s->ovpn_admin_group, sizeof(s->ovpn_admin_group));
+    error = GetRegString(key, TEXT("ovpn_admin_group"), s->ovpn_admin_group,
+                         sizeof(s->ovpn_admin_group), OVPN_ADMIN_GROUP);
     if (error != ERROR_SUCCESS)
     {
-        openvpn_sntprintf(s->ovpn_admin_group, _countof(s->ovpn_admin_group), OVPN_ADMIN_GROUP);
-        error = 0; /* this error is not fatal */
+        goto out;
     }
     /* set process priority */
     if (!_tcsicmp(priority, TEXT("IDLE_PRIORITY_CLASS")))