On 09-05-17 19:47, David Sommerseth wrote: > On 09/05/17 12:50, Steffan Karger wrote: >> This issue was found by Quarkslab during the OSTIF-founded security audit >> (issue 5.4), we agree 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.kar...@fox-it.com> >> --- >> src/openvpn/ssl.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c >> index 1033e58..abf5786 100644 >> --- a/src/openvpn/ssl.c >> +++ b/src/openvpn/ssl.c >> @@ -2661,6 +2661,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi >> *multi, struct tls_sessio >> >> error: >> secure_memzero(ks->key_src, sizeof(*ks->key_src)); >> + secure_memzero(up, sizeof(*up)); >> buf_clear(buf); >> gc_free(&gc); >> return false; >> > > Feature-ACK, but code-NAK. > > In key_method_2_read() there are a few places where `goto error;` can be > used before `up` is allocated. This will make secure_memzero() > segfault, as it does not handle NULL pointers. Or even worse (if the > compiler let this pointer point at a random address), which leads to an > unpredictable behaviour. > > That said, I think we should fix secure_memzero() to just return if the > input pointer is NULL. And even though most compilers do initialize > variables, I think it's good to be defensive here and initialize `up` too. >
Oof, good catch. I'll send a v2 in a minute. -Steffan
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
