Browse code

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>

Selva Nair authored on 2017/11/19 02:40:57
Showing 1 changed files
... ...
@@ -57,14 +57,18 @@ openvpn_sntprintf(LPTSTR str, size_t size, LPCTSTR format, ...)
57 57
 }
58 58
 
59 59
 static DWORD
60
-GetRegString(HKEY key, LPCTSTR value, LPTSTR data, DWORD size)
60
+GetRegString(HKEY key, LPCTSTR value, LPTSTR data, DWORD size, LPCTSTR default_value)
61 61
 {
62
-    DWORD type;
63
-    LONG status = RegQueryValueEx(key, value, NULL, &type, (LPBYTE) data, &size);
62
+    LONG status = RegGetValue(key, NULL, value, RRF_RT_REG_SZ|RRF_RT_REG_EXPAND_SZ,
63
+                              NULL, (LPBYTE) data, &size);
64 64
 
65
-    if (status == ERROR_SUCCESS && type != REG_SZ)
65
+    if (status == ERROR_FILE_NOT_FOUND && default_value)
66 66
     {
67
-        status = ERROR_DATATYPE_MISMATCH;
67
+        size_t len = size/sizeof(data[0]);
68
+        if (openvpn_sntprintf(data, len, default_value) > 0)
69
+        {
70
+            status = ERROR_SUCCESS;
71
+        }
68 72
     }
69 73
 
70 74
     if (status != ERROR_SUCCESS)
... ...
@@ -95,48 +99,48 @@ GetOpenvpnSettings(settings_t *s)
95 95
         return MsgToEventLog(M_SYSERR, TEXT("Could not open Registry key HKLM\\%s not found"), reg_path);
96 96
     }
97 97
 
98
-    error = GetRegString(key, TEXT("exe_path"), s->exe_path, sizeof(s->exe_path));
98
+    error = GetRegString(key, TEXT("exe_path"), s->exe_path, sizeof(s->exe_path), NULL);
99 99
     if (error != ERROR_SUCCESS)
100 100
     {
101 101
         goto out;
102 102
     }
103 103
 
104
-    error = GetRegString(key, TEXT("config_dir"), s->config_dir, sizeof(s->config_dir));
104
+    error = GetRegString(key, TEXT("config_dir"), s->config_dir, sizeof(s->config_dir), NULL);
105 105
     if (error != ERROR_SUCCESS)
106 106
     {
107 107
         goto out;
108 108
     }
109 109
 
110
-    error = GetRegString(key, TEXT("config_ext"), s->ext_string, sizeof(s->ext_string));
110
+    error = GetRegString(key, TEXT("config_ext"), s->ext_string, sizeof(s->ext_string), NULL);
111 111
     if (error != ERROR_SUCCESS)
112 112
     {
113 113
         goto out;
114 114
     }
115 115
 
116
-    error = GetRegString(key, TEXT("log_dir"), s->log_dir, sizeof(s->log_dir));
116
+    error = GetRegString(key, TEXT("log_dir"), s->log_dir, sizeof(s->log_dir), NULL);
117 117
     if (error != ERROR_SUCCESS)
118 118
     {
119 119
         goto out;
120 120
     }
121 121
 
122
-    error = GetRegString(key, TEXT("priority"), priority, sizeof(priority));
122
+    error = GetRegString(key, TEXT("priority"), priority, sizeof(priority), NULL);
123 123
     if (error != ERROR_SUCCESS)
124 124
     {
125 125
         goto out;
126 126
     }
127 127
 
128
-    error = GetRegString(key, TEXT("log_append"), append, sizeof(append));
128
+    error = GetRegString(key, TEXT("log_append"), append, sizeof(append), NULL);
129 129
     if (error != ERROR_SUCCESS)
130 130
     {
131 131
         goto out;
132 132
     }
133 133
 
134 134
     /* read if present, else use default */
135
-    error = GetRegString(key, TEXT("ovpn_admin_group"), s->ovpn_admin_group, sizeof(s->ovpn_admin_group));
135
+    error = GetRegString(key, TEXT("ovpn_admin_group"), s->ovpn_admin_group,
136
+                         sizeof(s->ovpn_admin_group), OVPN_ADMIN_GROUP);
136 137
     if (error != ERROR_SUCCESS)
137 138
     {
138
-        openvpn_sntprintf(s->ovpn_admin_group, _countof(s->ovpn_admin_group), OVPN_ADMIN_GROUP);
139
-        error = 0; /* this error is not fatal */
139
+        goto out;
140 140
     }
141 141
     /* set process priority */
142 142
     if (!_tcsicmp(priority, TEXT("IDLE_PRIORITY_CLASS")))