From cadc76a8b450f4e2181009c8faa2c4dace9bcc2c Mon Sep 17 00:00:00 2001 From: Aris Adamantiadis Date: Wed, 15 Apr 2015 16:08:37 +0200 Subject: [PATCH 1/2] CVE-2015-3146: Fix state validation in packet handlers The state validation in the packet handlers for SSH_MSG_NEWKEYS and SSH_MSG_KEXDH_REPLY had a bug which did not raise an error. The issue has been found and reported by Mariusz Ziule. Signed-off-by: Aris Adamantiadis Reviewed-by: Andreas Schneider --- src/client.c | 4 ++-- src/server.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client.c b/src/client.c index 0e50497..6919e7a 100644 --- a/src/client.c +++ b/src/client.c @@ -186,7 +186,7 @@ SSH_PACKET_CALLBACK(ssh_packet_dh_reply){ (void)type; (void)user; ssh_log(session,SSH_LOG_PROTOCOL,"Received SSH_KEXDH_REPLY"); - if(session->session_state!= SSH_SESSION_STATE_DH && + if(session->session_state!= SSH_SESSION_STATE_DH || session->dh_handshake_state != DH_STATE_INIT_SENT){ ssh_set_error(session,SSH_FATAL,"ssh_packet_dh_reply called in wrong state : %d:%d", session->session_state,session->dh_handshake_state); @@ -246,7 +246,7 @@ SSH_PACKET_CALLBACK(ssh_packet_newkeys){ (void)user; (void)type; ssh_log(session, SSH_LOG_PROTOCOL, "Received SSH_MSG_NEWKEYS"); - if(session->session_state!= SSH_SESSION_STATE_DH && + if (session->session_state != SSH_SESSION_STATE_DH || session->dh_handshake_state != DH_STATE_NEWKEYS_SENT){ ssh_set_error(session,SSH_FATAL,"ssh_packet_newkeys called in wrong state : %d:%d", session->session_state,session->dh_handshake_state); diff --git a/src/server.c b/src/server.c index 9a611c1..c07dd8a 100644 --- a/src/server.c +++ b/src/server.c @@ -133,6 +133,7 @@ SSH_PACKET_CALLBACK(ssh_packet_kexdh_init){ ssh_log(session,SSH_LOG_PACKET,"Received SSH_MSG_KEXDH_INIT"); if(session->dh_handshake_state != DH_STATE_INIT){ ssh_log(session,SSH_LOG_RARE,"Invalid state for SSH_MSG_KEXDH_INIT"); + session->session_state=SSH_SESSION_STATE_ERROR; goto error; } e = buffer_get_ssh_string(packet); -- 2.3.5 From ac683699201a3233b3659baa5f22c96ddab83cd4 Mon Sep 17 00:00:00 2001 From: Aris Adamantiadis Date: Wed, 15 Apr 2015 16:25:29 +0200 Subject: [PATCH 2/2] buffers: Fix a possible null pointer dereference This is an addition to CVE-2015-3146 to fix the null pointer dereference. The patch is not required to fix the CVE but prevents issues in future. Signed-off-by: Aris Adamantiadis Reviewed-by: Andreas Schneider (cherry picked from commit 59b316623ee723a5b6d4c980d0617bbaff4094c6) --- src/buffer.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/buffer.c b/src/buffer.c index aef7e44..9808399 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -188,6 +188,9 @@ int buffer_reinit(struct ssh_buffer_struct *buffer) { int buffer_add_data(struct ssh_buffer_struct *buffer, const void *data, uint32_t len) { buffer_verify(buffer); + if (data == NULL){ + return -1; + } if (buffer->used + len < len) return -1; @@ -220,6 +223,9 @@ int buffer_add_ssh_string(struct ssh_buffer_struct *buffer, struct ssh_string_struct *string) { uint32_t len = 0; + if (string == NULL){ + return -1; + } len = ssh_string_len(string); if (buffer_add_data(buffer, string, len + sizeof(uint32_t)) < 0) { return -1; -- 2.3.5