Always clear username/password from memory on error

This issue was found by Quarkslab during the OSTIF-founded security audit
(issue 5.4), we are with their analysis:

"There’s a special case where the client username and password are not
erased when the server is launched without an external script or
authentication plugin. While being invalid, this configuration does not
raise any error. If the client transmits its credentials and the session
is not established (for instance if the certificates chain has not been
verified), these credentials are not erased from memory by the server.

The likelihood of an occurrence of this issue in real life is
exceptionally low since an attacker needs elevated privileges on the
server to exploit this kind of information leak. The severity of this
issue is rated as very low."

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1494354764-19354-1-git-send-email-steffan.karger@fox-it.com>
URL: http://www.mail-archive.com/search?l=mid&q=1494354764-19354-1-git-send-email-steffan.karger@fox-it.com
Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 2b60198e08a9d7e8de9beeb65a587ee34107efe8)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 630b77f9272bd29a5354d4d1a69308a9715dc8f1..d2f4730bfb44e1c2d82e2bad8794c5c889ea7798 100644 (file)
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2492,7 +2492,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_sessio
 
     struct gc_arena gc = gc_new();
     char *options;
-    struct user_pass *up;
+    struct user_pass *up = NULL;
 
     /* allocate temporary objects */
     ALLOC_ARRAY_CLEAR_GC(options, char, TLS_OPTIONS_LEN, &gc);
@@ -2654,6 +2654,10 @@ key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_sessio
 
 error:
     secure_memzero(ks->key_src, sizeof(*ks->key_src));
+    if (up)
+    {
+        secure_memzero(up, sizeof(*up));
+    }
     buf_clear(buf);
     gc_free(&gc);
     return false;