plaisthos has uploaded this change for review. ( http://gerrit.openvpn.net/c/openvpn/+/1373?usp=email )
Change subject: Fix construction of invalid pointer in tls_pre_decrypt ...................................................................... Fix construction of invalid pointer in tls_pre_decrypt In tls_pre_decrypt we construct a pointer ks with an invalid i if i is TM_SIZE doing a out-of-bounds access in multi->session. This is a something that exists at least since 2.3.0 (I didn't go further back but probalby exists in earlier version as well as the commits date back to SVN beta21 branch). So we construct the pointer but do not do anything with it if it is inval id as we check i *after* we construct the pointer `ks`. I suspect that the compiler optimises the bug away in any higher optimisation level. Assuming there is no optimisation, let's check what is possible. Since we never use the value `ks` if it is invalid, we do not have worry if it ends up invalid or not. The only thing that we have to worry about is whether `session + offsetof(struct tls_session, key[KS_PRIMARY])` is pointing to memory that is valid to read to construct the `ks` pointer. This is outside the tls_multi struct, so this is not guaranteed to be allocated memory but at the same time it is also only few bytes (or few tens/houndred) after the struct, so it will with an extremely high probably be in a memory region that will not cause a segfault. Every time this condition is hit and we construct the invalid pointer, the log message "TLS Error: Unroutable control packet received" is printed at `verb 1` or higher. And this is a quite common log message, which serves as indication as well that a crash is not something that typically happens but either the optimisation fixes or the memory region of the invalid access is valid to read from. Change-Id: Ided1ac7c804487055b175d8766535bead257b7d5 Signed-off-by: Arne Schwabe <[email protected]> --- M src/openvpn/ssl.c 1 file changed, 2 insertions(+), 3 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/73/1373/1 diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 398c9ae..e21ac78 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -3729,9 +3729,6 @@ } else { - struct tls_session *session = &multi->session[i]; - struct key_state *ks = &session->key[KS_PRIMARY]; - /* * Packet must belong to an existing session. */ @@ -3742,6 +3739,8 @@ goto error; } + struct tls_session *session = &multi->session[i]; + struct key_state *ks = &session->key[KS_PRIMARY]; /* * Verify remote IP address */ -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1373?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ided1ac7c804487055b175d8766535bead257b7d5 Gerrit-Change-Number: 1373 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos <[email protected]> Gerrit-CC: openvpn-devel <[email protected]>
_______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
