Skip to content

Commit

Permalink
kex.c : additional bounds checks in diffie_hellman_sha1/256 (libssh2#361
Browse files Browse the repository at this point in the history 
)

Files : kex.c, misc.c, misc.h

Notes :
Fixed possible out of bounds memory access when reading malformed data in diffie_hellman_sha1() and diffie_hellman_sha256().

Added _libssh2_copy_string() to misc.c to return an allocated and filled char buffer from a string_buf offset. Removed no longer needed s var in kmdhgGPshakex_state_t.
  • Loading branch information
@willco007
willco007 authored May 1, 2019
1 parent dd74f24 commit 16f2d2b
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 51 deletions.
100 changes: 50 additions & 50 deletions src/kex.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,9 @@ static int diffie_hellman_sha1(LIBSSH2_SESSION *session,

if(exchange_state->state == libssh2_NB_state_sent1) {
/* Wait for KEX reply */
struct string_buf buf;
size_t host_key_len;

rc = _libssh2_packet_require(session, packet_type_reply,
&exchange_state->s_packet,
&exchange_state->s_packet_len, 0, NULL,
Expand All @@ -261,31 +264,22 @@ static int diffie_hellman_sha1(LIBSSH2_SESSION *session,
goto clean_exit;
}

exchange_state->s = exchange_state->s_packet + 1;

session->server_hostkey_len = _libssh2_ntohu32(exchange_state->s);
exchange_state->s += 4;

if(session->server_hostkey_len > exchange_state->s_packet_len - 5) {
ret = _libssh2_error(session, LIBSSH2_ERROR_OUT_OF_BOUNDARY,
"Host key length out of bounds");
goto clean_exit;
}
buf.data = exchange_state->s_packet;
buf.len = exchange_state->s_packet_len;
buf.dataptr = buf.data;
buf.dataptr++; /* advance past type */

if(session->server_hostkey)
LIBSSH2_FREE(session, session->server_hostkey);

session->server_hostkey =
LIBSSH2_ALLOC(session, session->server_hostkey_len);
if(!session->server_hostkey) {
if(_libssh2_copy_string(session, &buf, &(session->server_hostkey),
&host_key_len)) {
ret = _libssh2_error(session, LIBSSH2_ERROR_ALLOC,
"Unable to allocate memory for a copy "
"of the host key");
"Could not copy host key");
goto clean_exit;
}
memcpy(session->server_hostkey, exchange_state->s,
session->server_hostkey_len);
exchange_state->s += session->server_hostkey_len;

session->server_hostkey_len = (uint32_t)host_key_len;

#if LIBSSH2_MD5
{
Expand Down Expand Up @@ -383,16 +377,22 @@ static int diffie_hellman_sha1(LIBSSH2_SESSION *session,
goto clean_exit;
}

exchange_state->f_value_len = _libssh2_ntohu32(exchange_state->s);
exchange_state->s += 4;
exchange_state->f_value = exchange_state->s;
exchange_state->s += exchange_state->f_value_len;
if(_libssh2_get_string(&buf, &(exchange_state->f_value),
&(exchange_state->f_value_len))) {
ret = _libssh2_error(session, LIBSSH2_ERROR_HOSTKEY_INIT,
"Unable to get f value");
goto clean_exit;
}

_libssh2_bn_from_bin(exchange_state->f, exchange_state->f_value_len,
exchange_state->f_value);

exchange_state->h_sig_len = _libssh2_ntohu32(exchange_state->s);
exchange_state->s += 4;
exchange_state->h_sig = exchange_state->s;
if(_libssh2_get_string(&buf, &(exchange_state->h_sig),
&(exchange_state->h_sig_len))) {
ret = _libssh2_error(session, LIBSSH2_ERROR_HOSTKEY_INIT,
"Unable to get h sig");
goto clean_exit;
}

/* Compute the shared secret */
libssh2_dh_secret(&exchange_state->x, exchange_state->k,
Expand Down Expand Up @@ -932,6 +932,9 @@ static int diffie_hellman_sha256(LIBSSH2_SESSION *session,

if(exchange_state->state == libssh2_NB_state_sent1) {
/* Wait for KEX reply */
struct string_buf buf;
size_t host_key_len;

rc = _libssh2_packet_require(session, packet_type_reply,
&exchange_state->s_packet,
&exchange_state->s_packet_len, 0, NULL,
Expand All @@ -952,31 +955,22 @@ static int diffie_hellman_sha256(LIBSSH2_SESSION *session,
goto clean_exit;
}

exchange_state->s = exchange_state->s_packet + 1;

session->server_hostkey_len = _libssh2_ntohu32(exchange_state->s);
exchange_state->s += 4;

if(session->server_hostkey_len > exchange_state->s_packet_len - 5) {
ret = _libssh2_error(session, LIBSSH2_ERROR_OUT_OF_BOUNDARY,
"Host key length out of bounds");
goto clean_exit;
}
buf.data = exchange_state->s_packet;
buf.len = exchange_state->s_packet_len;
buf.dataptr = buf.data;
buf.dataptr++; /* advance past type */

if(session->server_hostkey)
LIBSSH2_FREE(session, session->server_hostkey);

session->server_hostkey =
LIBSSH2_ALLOC(session, session->server_hostkey_len);
if(!session->server_hostkey) {
if(_libssh2_copy_string(session, &buf, &(session->server_hostkey),
&host_key_len)) {
ret = _libssh2_error(session, LIBSSH2_ERROR_ALLOC,
"Unable to allocate memory for a copy "
"of the host key");
"Could not copy host key");
goto clean_exit;
}
memcpy(session->server_hostkey, exchange_state->s,
session->server_hostkey_len);
exchange_state->s += session->server_hostkey_len;

session->server_hostkey_len = (uint32_t)host_key_len;

#if LIBSSH2_MD5
{
Expand Down Expand Up @@ -1073,16 +1067,22 @@ static int diffie_hellman_sha256(LIBSSH2_SESSION *session,
goto clean_exit;
}

exchange_state->f_value_len = _libssh2_ntohu32(exchange_state->s);
exchange_state->s += 4;
exchange_state->f_value = exchange_state->s;
exchange_state->s += exchange_state->f_value_len;
if(_libssh2_get_string(&buf, &(exchange_state->f_value),
&(exchange_state->f_value_len))) {
ret = _libssh2_error(session, LIBSSH2_ERROR_HOSTKEY_INIT,
"Unable to get f value");
goto clean_exit;
}

_libssh2_bn_from_bin(exchange_state->f, exchange_state->f_value_len,
exchange_state->f_value);

exchange_state->h_sig_len = _libssh2_ntohu32(exchange_state->s);
exchange_state->s += 4;
exchange_state->h_sig = exchange_state->s;
if(_libssh2_get_string(&buf, &(exchange_state->h_sig),
&(exchange_state->h_sig_len))) {
ret = _libssh2_error(session, LIBSSH2_ERROR_HOSTKEY_INIT,
"Unable to get h sig");
goto clean_exit;
}

/* Compute the shared secret */
libssh2_dh_secret(&exchange_state->x, exchange_state->k,
Expand Down
1 change: 0 additions & 1 deletion src/libssh2_priv.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,6 @@ typedef struct kmdhgGPshakex_state_t
_libssh2_bn *e;
_libssh2_bn *f;
_libssh2_bn *k;
unsigned char *s;
unsigned char *f_value;
unsigned char *k_value;
unsigned char *h_sig;
Expand Down
24 changes: 24 additions & 0 deletions src/misc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,30 @@ int _libssh2_get_string(struct string_buf *buf, unsigned char **outbuf,
return 0;
}

int _libssh2_copy_string(LIBSSH2_SESSION *session, struct string_buf *buf,
unsigned char **outbuf, size_t *outlen)
{
size_t str_len;
unsigned char *str;

if(_libssh2_get_string(buf, &str, &str_len)) {
return -1;
}

*outbuf = LIBSSH2_ALLOC(session, str_len);
if(*outbuf) {
memcpy(*outbuf, str, str_len);
}
else {
return -1;
}

if(outlen)
*outlen = str_len;

return 0;
}

int _libssh2_get_bignum_bytes(struct string_buf *buf, unsigned char **outbuf,
size_t *outlen)
{
Expand Down
2 changes: 2 additions & 0 deletions src/misc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ int _libssh2_get_u64(struct string_buf *buf, libssh2_uint64_t *out);
int _libssh2_match_string(struct string_buf *buf, const char *match);
int _libssh2_get_string(struct string_buf *buf, unsigned char **outbuf,
size_t *outlen);
int _libssh2_copy_string(LIBSSH2_SESSION* session, struct string_buf *buf,
unsigned char **outbuf, size_t *outlen);
int _libssh2_get_bignum_bytes(struct string_buf *buf, unsigned char **outbuf,
size_t *outlen);
int _libssh2_check_length(struct string_buf *buf, size_t requested_len);
Expand Down

0 comments on commit 16f2d2b

Please sign in to comment.