"George Garner (online)" <ggarner_onl...@gmgsystemsinc.com> writes: [...] > 3. Where is the p_len/group_order parameter validated? In > kex_method_diffie_hellman_group_exchange_sha256_key_exchange it is > converted from network byte order and accepted at face value. What > happens if a malicious packet is received with a bogus value for > p_len?
Maybe I miss something, but it looks like this defect (blindly trust various 32-bit length that was sent remote side and don't verify if it fits buffer) is *everywhere* in libssh2. I've sent some patches for kex.c via gh pull request, but quickly discovered it is much worse. Very WIP (and incomplete) patch for *other* files is attached; unfortunately, in most cases, I have no idea how such errors should be handled within libssh2, don't know libssh2 code base well enough, so I give up at this. Note that in early connection setup "malicious server" is not required, "malicious MITM" can insert broken packets as well. In general, please re-review all `grep ntoh -r src/`, in many cases surrounding code looks problematic in one way or other.
--- Changelog: v2: fixed obvious errors Note: This is still NOT COMPLETE work, all XXX comment must be reviewed and acted upon. src/agent.c | 32 ++++++++-------- src/channel.c | 10 ++++- src/hostkey.c | 19 +++++++-- src/kex.c | 43 +++++++++++---------- src/packet.c | 45 +++++++++++++++++----- src/publickey.c | 117 +++++++++++++++++++++++++++++++++++++++++++------------- src/session.c | 2 + src/sftp.c | 42 ++++++++++++++++---- src/sftp.h | 1 + src/userauth.c | 32 ++++++++++++++++ 10 files changed, 260 insertions(+), 83 deletions(-) diff --git a/src/agent.c b/src/agent.c index c2ba422..255b63d 100644 --- a/src/agent.c +++ b/src/agent.c @@ -449,12 +449,12 @@ agent_sign(LIBSSH2_SESSION *session, unsigned char **sig, size_t *sig_len, goto error; } method_len = _libssh2_ntohu32(s); - s += 4; - len -= method_len; - if (len < 0) { + if (method_len < 0 || len < method_len) { rc = LIBSSH2_ERROR_AGENT_PROTOCOL; goto error; } + s += 4; + len -= method_len; s += method_len; /* Read the signature */ @@ -464,12 +464,12 @@ agent_sign(LIBSSH2_SESSION *session, unsigned char **sig, size_t *sig_len, goto error; } *sig_len = _libssh2_ntohu32(s); - s += 4; - len -= *sig_len; - if (len < 0) { + if ((size_t)len < *sig_len) { rc = LIBSSH2_ERROR_AGENT_PROTOCOL; goto error; } + len -= *sig_len; + s += 4; *sig = LIBSSH2_ALLOC(session, *sig_len); if (!*sig) { @@ -558,15 +558,15 @@ agent_list_identities(LIBSSH2_AGENT *agent) goto error; } identity->external.blob_len = _libssh2_ntohu32(s); - s += 4; - - /* Read the blob */ - len -= identity->external.blob_len; - if (len < 0) { + if ((size_t)len < identity->external.blob_len) { rc = LIBSSH2_ERROR_AGENT_PROTOCOL; LIBSSH2_FREE(agent->session, identity); goto error; } + s += 4; + + /* Read the blob */ + len -= identity->external.blob_len; identity->external.blob = LIBSSH2_ALLOC(agent->session, identity->external.blob_len); @@ -587,16 +587,16 @@ agent_list_identities(LIBSSH2_AGENT *agent) goto error; } comment_len = _libssh2_ntohu32(s); - s += 4; - - /* Read the comment */ - len -= comment_len; - if (len < 0) { + if (comment_len < 0 || len < comment_len) { rc = LIBSSH2_ERROR_AGENT_PROTOCOL; LIBSSH2_FREE(agent->session, identity->external.blob); LIBSSH2_FREE(agent->session, identity); goto error; } + s += 4; + + /* Read the comment */ + len -= comment_len; identity->external.comment = LIBSSH2_ALLOC(agent->session, comment_len + 1); diff --git a/src/channel.c b/src/channel.c index 32d914d..38572be 100644 --- a/src/channel.c +++ b/src/channel.c @@ -225,6 +225,7 @@ _libssh2_channel_open(LIBSSH2_SESSION * session, const char *channel_type, } if (session->open_state == libssh2_NB_state_sent) { + unsigned char *end; rc = _libssh2_packet_requirev(session, reply_codes, &session->open_data, &session->open_data_len, 1, @@ -238,7 +239,11 @@ _libssh2_channel_open(LIBSSH2_SESSION * session, const char *channel_type, goto channel_error; } + end = session->open_data + session->open_data_len; + if (session->open_data[0] == SSH_MSG_CHANNEL_OPEN_CONFIRMATION) { + if (13+4 > (end - session->open_data)) + goto channel_error; session->open_channel->remote.id = _libssh2_ntohu32(session->open_data + 5); session->open_channel->local.window_size = @@ -265,7 +270,8 @@ _libssh2_channel_open(LIBSSH2_SESSION * session, const char *channel_type, return session->open_channel; } - if (session->open_data[0] == SSH_MSG_CHANNEL_OPEN_FAILURE) { + if (session->open_data[0] == SSH_MSG_CHANNEL_OPEN_FAILURE && + 4 <= (end - (session->open_data + 5))) { unsigned int reason_code = _libssh2_ntohu32(session->open_data + 5); switch (reason_code) { case SSH_OPEN_ADMINISTRATIVELY_PROHIBITED: @@ -1399,6 +1405,7 @@ _libssh2_channel_flush(LIBSSH2_CHANNEL *channel, int streamid) if (((packet_type == SSH_MSG_CHANNEL_DATA) || (packet_type == SSH_MSG_CHANNEL_EXTENDED_DATA)) + && packet->data_len >= 5 + (packet_type == SSH_MSG_CHANNEL_EXTENDED_DATA ? 4 : 0) && (_libssh2_ntohu32(packet->data + 1) == channel->local.id)) { /* It's our channel at least */ long packet_stream_id = @@ -1418,6 +1425,7 @@ _libssh2_channel_flush(LIBSSH2_CHANNEL *channel, int streamid) bytes_to_flush, packet_stream_id, channel->local.id, channel->remote.id); + /* XXX assert(packet->data_len >= 13); XXX */ /* It's one of the streams we wanted to flush */ channel->flush_refund_bytes += packet->data_len - 13; channel->flush_flush_bytes += bytes_to_flush; diff --git a/src/hostkey.c b/src/hostkey.c index 2a0a8f9..7b780e2 100644 --- a/src/hostkey.c +++ b/src/hostkey.c @@ -66,31 +66,42 @@ hostkey_method_ssh_rsa_init(LIBSSH2_SESSION * session, libssh2_rsa_ctx *rsactx; const unsigned char *s, *e, *n; unsigned long len, e_len, n_len; + const unsigned char *end = hostkey_data + hostkey_data_len; int ret; - (void) hostkey_data_len; - if (*abstract) { hostkey_method_ssh_rsa_dtor(session, abstract); *abstract = NULL; } s = hostkey_data; + if (4 > end - s) + return -1; len = _libssh2_ntohu32(s); s += 4; + if (len > (size_t)(end - s)) + return -1; if (len != 7 || strncmp((char *) s, "ssh-rsa", 7) != 0) { return -1; } - s += 7; + s += len; + if (4 > end - s) + return -1; e_len = _libssh2_ntohu32(s); s += 4; + if (e_len > (size_t)(end - s)) + return -1; e = s; s += e_len; + if (4 > end - s) + return -1; n_len = _libssh2_ntohu32(s); s += 4; + if (n_len > (size_t)(end - s)) + return -1; n = s; ret = _libssh2_rsa_new(&rsactx, e, e_len, n, n_len, NULL, 0, @@ -181,6 +192,8 @@ hostkey_method_ssh_rsa_sig_verify(LIBSSH2_SESSION * session, (void) session; /* Skip past keyname_len(4) + keyname(7){"ssh-rsa"} + signature_len(4) */ + if (15 > sig_len) + return -1; sig += 15; sig_len -= 15; return _libssh2_rsa_sha1_verify(rsactx, sig, sig_len, m, m_len); diff --git a/src/kex.c b/src/kex.c index 40dbeab..2381d52 100644 --- a/src/kex.c +++ b/src/kex.c @@ -2463,21 +2463,20 @@ static int kex_agree_comp(LIBSSH2_SESSION *session, * within the given packet. */ static int kex_string_pair(unsigned char **sp, /* parsing position */ - unsigned char *data, /* start pointer to packet */ - size_t data_len, /* size of total packet */ + unsigned char *end, /* end of packet */ size_t *lenp, /* length of the string */ unsigned char **strp) /* pointer to string start */ { unsigned char *s = *sp; - *lenp = _libssh2_ntohu32(s); - /* the length of the string must fit within the current pointer and the - end of the packet */ - if (*lenp > (data_len - (s - data) -4)) + if (4 > end - s) return 1; - *strp = s + 4; - s += 4 + *lenp; - + *lenp = _libssh2_ntohu32(s); + s += 4; + if (*lenp > (size_t)(end - s)) + return 1; + *strp = s; + s += *lenp; *sp = s; return 0; } @@ -2493,6 +2492,10 @@ static int kex_agree_methods(LIBSSH2_SESSION * session, unsigned char *data, size_t kex_len, hostkey_len, crypt_cs_len, crypt_sc_len, comp_cs_len; size_t comp_sc_len, mac_cs_len, mac_sc_len; unsigned char *s = data; + unsigned char *end = data + data_len; + + if (1 + 16 > end - s) + return -1; /* Skip packet_type, we know it already */ s++; @@ -2501,21 +2504,24 @@ static int kex_agree_methods(LIBSSH2_SESSION * session, unsigned char *data, s += 16; /* Locate each string */ - if(kex_string_pair(&s, data, data_len, &kex_len, &kex)) + if(kex_string_pair(&s, end, &kex_len, &kex)) + return -1; + if(kex_string_pair(&s, end, &hostkey_len, &hostkey)) return -1; - if(kex_string_pair(&s, data, data_len, &hostkey_len, &hostkey)) + if(kex_string_pair(&s, end, &crypt_cs_len, &crypt_cs)) return -1; - if(kex_string_pair(&s, data, data_len, &crypt_cs_len, &crypt_cs)) + if(kex_string_pair(&s, end, &crypt_sc_len, &crypt_sc)) return -1; - if(kex_string_pair(&s, data, data_len, &crypt_sc_len, &crypt_sc)) + if(kex_string_pair(&s, end, &mac_cs_len, &mac_cs)) return -1; - if(kex_string_pair(&s, data, data_len, &mac_cs_len, &mac_cs)) + if(kex_string_pair(&s, end, &mac_sc_len, &mac_sc)) return -1; - if(kex_string_pair(&s, data, data_len, &mac_sc_len, &mac_sc)) + if(kex_string_pair(&s, end, &comp_cs_len, &comp_cs)) return -1; - if(kex_string_pair(&s, data, data_len, &comp_cs_len, &comp_cs)) + if(kex_string_pair(&s, end, &comp_sc_len, &comp_sc)) return -1; - if(kex_string_pair(&s, data, data_len, &comp_sc_len, &comp_sc)) + + if (1 > end - s) return -1; /* If the server sent an optimistic packet, assume that it guessed wrong. @@ -2524,9 +2530,6 @@ static int kex_agree_methods(LIBSSH2_SESSION * session, unsigned char *data, session->burn_optimistic_kexinit = *(s++); /* Next uint32 in packet is all zeros (reserved) */ - if (data_len < (unsigned) (s - data)) - return -1; /* short packet */ - if (kex_agree_kex_hostkey(session, kex, kex_len, hostkey, hostkey_len)) { return -1; } diff --git a/src/packet.c b/src/packet.c index 5f1feb8..3659daa 100644 --- a/src/packet.c +++ b/src/packet.c @@ -85,10 +85,12 @@ packet_queue_listener(LIBSSH2_SESSION * session, unsigned char *data, char failure_code = SSH_OPEN_ADMINISTRATIVELY_PROHIBITED; int rc; - (void) datalen; - if (listen_state->state == libssh2_NB_state_idle) { unsigned char *s = data + (sizeof("forwarded-tcpip") - 1) + 5; + unsigned char *end = data + datalen; + if (4*4 > (end - s)) { + return 0; /* XXX ??? XXX */ + } listen_state->sender_channel = _libssh2_ntohu32(s); s += 4; @@ -99,15 +101,27 @@ packet_queue_listener(LIBSSH2_SESSION * session, unsigned char *data, listen_state->host_len = _libssh2_ntohu32(s); s += 4; + if (listen_state->host_len > (size_t)(end - s)) { + return 0; /* XXX ??? XXX */ + } listen_state->host = s; s += listen_state->host_len; + if (4*2 > (end - s)) { + return 0; /* XXX ??? XXX */ + } listen_state->port = _libssh2_ntohu32(s); s += 4; listen_state->shost_len = _libssh2_ntohu32(s); s += 4; + if (listen_state->shost_len > (size_t)(end - s)) { + return 0; /* XXX ??? XXX */ + } listen_state->shost = s; s += listen_state->shost_len; + if (4 > (end - s)) { + return 0; /* XXX ??? XXX */ + } listen_state->sport = _libssh2_ntohu32(s); _libssh2_debug(session, LIBSSH2_TRACE_CONN, @@ -271,10 +285,12 @@ packet_x11_open(LIBSSH2_SESSION * session, unsigned char *data, LIBSSH2_CHANNEL *channel = x11open_state->channel; int rc; - (void) datalen; - if (x11open_state->state == libssh2_NB_state_idle) { unsigned char *s = data + (sizeof("x11") - 1) + 5; + unsigned char *end = data + datalen; + if (4*4 > (end - s)) { + return 0; /* XXX ??? XXX */ + } x11open_state->sender_channel = _libssh2_ntohu32(s); s += 4; x11open_state->initial_window_size = _libssh2_ntohu32(s); @@ -283,8 +299,14 @@ packet_x11_open(LIBSSH2_SESSION * session, unsigned char *data, s += 4; x11open_state->shost_len = _libssh2_ntohu32(s); s += 4; + if (x11open_state->shost_len > (size_t)(end - s)) { + return 0; /* XXX ??? XXX */ + } x11open_state->shost = s; s += x11open_state->shost_len; + if (4 > (end - s)) { + return 0; /* XXX ??? XXX */ + } x11open_state->sport = _libssh2_ntohu32(s); _libssh2_debug(session, LIBSSH2_TRACE_CONN, @@ -807,22 +829,26 @@ _libssh2_packet_add(LIBSSH2_SESSION * session, unsigned char *data, else if (len == sizeof("exit-signal") - 1 && !memcmp("exit-signal", data + 9, sizeof("exit-signal") - 1)) { + unsigned char *end = data + datalen; + unsigned char *s = data + 9 + sizeof("exit-signal"); /* command terminated due to signal */ if(datalen >= 20) channelp = _libssh2_channel_locate(session, channel); - if (channelp) { + if (channelp && end - s >= 4) { /* set signal name (without SIG prefix) */ - uint32_t namelen = - _libssh2_ntohu32(data + 9 + sizeof("exit-signal")); + uint32_t namelen = _libssh2_ntohu32(s); + s += 4; + if (namelen > (size_t)(end - s)) + /* XXX ??? XXX */; + else { channelp->exit_signal = LIBSSH2_ALLOC(session, namelen + 1); if (!channelp->exit_signal) rc = _libssh2_error(session, LIBSSH2_ERROR_ALLOC, "memory for signal name"); else { - memcpy(channelp->exit_signal, - data + 13 + sizeof("exit_signal"), namelen); + memcpy(channelp->exit_signal, s, namelen); channelp->exit_signal[namelen] = '\0'; /* TODO: save error message and language tag */ _libssh2_debug(session, LIBSSH2_TRACE_CONN, @@ -832,6 +858,7 @@ _libssh2_packet_add(LIBSSH2_SESSION * session, unsigned char *data, channelp->local.id, channelp->remote.id); } + } } } diff --git a/src/publickey.c b/src/publickey.c index bfee0a8..d19efb7 100644 --- a/src/publickey.c +++ b/src/publickey.c @@ -247,6 +247,7 @@ publickey_response_success(LIBSSH2_PUBLICKEY * pkey) switch (response) { case LIBSSH2_PUBLICKEY_RESPONSE_STATUS: /* Error, or processing complete */ + if (data_len >= 4) { unsigned long status = _libssh2_ntohu32(s); @@ -258,6 +259,7 @@ publickey_response_success(LIBSSH2_PUBLICKEY * pkey) publickey_status_error(pkey, session, status); return -1; } + /* fallthru */ default: LIBSSH2_FREE(session, data); if (response < 0) { @@ -403,6 +405,7 @@ static LIBSSH2_PUBLICKEY *publickey_init(LIBSSH2_SESSION *session) if (session->pkeyInit_state == libssh2_NB_state_sent3) { while (1) { unsigned char *s; + unsigned char *end; rc = publickey_packet_receive(session->pkeyInit_pkey, &session->pkeyInit_data, &session->pkeyInit_data_len); @@ -419,6 +422,7 @@ static LIBSSH2_PUBLICKEY *publickey_init(LIBSSH2_SESSION *session) } s = session->pkeyInit_data; + end = session->pkeyInit_data + session->pkeyInit_data_len; if ((response = publickey_response_id(&s, session->pkeyInit_data_len)) < 0) { _libssh2_error(session, LIBSSH2_ERROR_PUBLICKEY_PROTOCOL, @@ -432,19 +436,33 @@ static LIBSSH2_PUBLICKEY *publickey_init(LIBSSH2_SESSION *session) { unsigned long status, descr_len, lang_len; - status = _libssh2_ntohu32(s); - s += 4; - descr_len = _libssh2_ntohu32(s); - s += 4; - /* description starts here */ - s += descr_len; - lang_len = _libssh2_ntohu32(s); - s += 4; - /* lang starts here */ - s += lang_len; - - if (s > - session->pkeyInit_data + session->pkeyInit_data_len) { + if (4*2 > end - s) + s = NULL; + else { + status = _libssh2_ntohu32(s); + s += 4; + descr_len = _libssh2_ntohu32(s); + s += 4; + /* description starts here */ + if (descr_len > (size_t)(end - s)) + s = NULL; + else { + s += descr_len; + if (4 > end - s) + s = NULL; + else { + lang_len = _libssh2_ntohu32(s); + s += 4; + /* lang starts here */ + if (lang_len > (size_t)(end - s)) + s = NULL; + else + s += lang_len; + } + } + } + + if (s == NULL) { _libssh2_error(session, LIBSSH2_ERROR_PUBLICKEY_PROTOCOL, "Malformed publickey subsystem packet"); @@ -810,6 +828,7 @@ libssh2_publickey_list_fetch(LIBSSH2_PUBLICKEY * pkey, unsigned long *num_keys, } while (1) { + unsigned char *end; rc = publickey_packet_receive(pkey, &pkey->listFetch_data, &pkey->listFetch_data_len); if (rc == LIBSSH2_ERROR_EAGAIN) { @@ -822,6 +841,7 @@ libssh2_publickey_list_fetch(LIBSSH2_PUBLICKEY * pkey, unsigned long *num_keys, } pkey->listFetch_s = pkey->listFetch_data; + end = pkey->listFetch_data + pkey->listFetch_data_len; if ((response = publickey_response_id(&pkey->listFetch_s, pkey->listFetch_data_len)) < 0) { @@ -836,19 +856,34 @@ libssh2_publickey_list_fetch(LIBSSH2_PUBLICKEY * pkey, unsigned long *num_keys, { unsigned long status, descr_len, lang_len; - status = _libssh2_ntohu32(pkey->listFetch_s); - pkey->listFetch_s += 4; - descr_len = _libssh2_ntohu32(pkey->listFetch_s); - pkey->listFetch_s += 4; - /* description starts at pkey->listFetch_s */ - pkey->listFetch_s += descr_len; - lang_len = _libssh2_ntohu32(pkey->listFetch_s); - pkey->listFetch_s += 4; - /* lang starts at pkey->listFetch_s */ - pkey->listFetch_s += lang_len; - - if (pkey->listFetch_s > - pkey->listFetch_data + pkey->listFetch_data_len) { + if (4*2 > end - pkey->listFetch_s) + pkey->listFetch_s = NULL; + else { + status = _libssh2_ntohu32(pkey->listFetch_s); + pkey->listFetch_s += 4; + descr_len = _libssh2_ntohu32(pkey->listFetch_s); + pkey->listFetch_s += 4; + if (descr_len > (size_t)(end - pkey->listFetch_s)) + pkey->listFetch_s = NULL; + else { + /* description starts at pkey->listFetch_s */ + pkey->listFetch_s += descr_len; + if (4 > end - pkey->listFetch_s) + pkey->listFetch_s = NULL; + else { + lang_len = _libssh2_ntohu32(pkey->listFetch_s); + pkey->listFetch_s += 4; + if (lang_len > (size_t)(end - pkey->listFetch_s)) + pkey->listFetch_s = NULL; + else { + /* lang starts at pkey->listFetch_s */ + pkey->listFetch_s += lang_len; + } + } + } + } + + if (pkey->listFetch_s == NULL) { _libssh2_error(session, LIBSSH2_ERROR_PUBLICKEY_PROTOCOL, "Malformed publickey subsystem packet"); goto err_exit; @@ -887,8 +922,12 @@ libssh2_publickey_list_fetch(LIBSSH2_PUBLICKEY * pkey, unsigned long *num_keys, if (pkey->version == 1) { unsigned long comment_len; + if (4 > end - pkey->listFetch_s) + goto err_exit; comment_len = _libssh2_ntohu32(pkey->listFetch_s); pkey->listFetch_s += 4; + if (comment_len > (size_t)(end - pkey->listFetch_s)) + goto err_exit; if (comment_len) { list[keys].num_attrs = 1; list[keys].attrs = @@ -911,24 +950,42 @@ libssh2_publickey_list_fetch(LIBSSH2_PUBLICKEY * pkey, unsigned long *num_keys, list[keys].num_attrs = 0; list[keys].attrs = NULL; } + if (4 > end - pkey->listFetch_s) + goto err_exit; list[keys].name_len = _libssh2_ntohu32(pkey->listFetch_s); pkey->listFetch_s += 4; + if (list[keys].name_len > (size_t)(end - pkey->listFetch_s)) + goto err_exit; list[keys].name = pkey->listFetch_s; pkey->listFetch_s += list[keys].name_len; + if (4 > end - pkey->listFetch_s) + goto err_exit; list[keys].blob_len = _libssh2_ntohu32(pkey->listFetch_s); pkey->listFetch_s += 4; + if (list[keys].blob_len > (size_t)(end - pkey->listFetch_s)) + goto err_exit; list[keys].blob = pkey->listFetch_s; pkey->listFetch_s += list[keys].blob_len; } else { /* Version == 2 */ + if (4 > end - pkey->listFetch_s) + goto err_exit; list[keys].name_len = _libssh2_ntohu32(pkey->listFetch_s); pkey->listFetch_s += 4; + if (list[keys].name_len > (size_t)(end - pkey->listFetch_s)) + goto err_exit; list[keys].name = pkey->listFetch_s; pkey->listFetch_s += list[keys].name_len; + if (4 > end - pkey->listFetch_s) + goto err_exit; list[keys].blob_len = _libssh2_ntohu32(pkey->listFetch_s); pkey->listFetch_s += 4; + if (list[keys].blob_len > (size_t)(end - pkey->listFetch_s)) + goto err_exit; list[keys].blob = pkey->listFetch_s; pkey->listFetch_s += list[keys].blob_len; + if (4 > end - pkey->listFetch_s) + goto err_exit; list[keys].num_attrs = _libssh2_ntohu32(pkey->listFetch_s); pkey->listFetch_s += 4; if (list[keys].num_attrs) { @@ -943,14 +1000,22 @@ libssh2_publickey_list_fetch(LIBSSH2_PUBLICKEY * pkey, unsigned long *num_keys, goto err_exit; } for(i = 0; i < list[keys].num_attrs; i++) { + if (4 > end - pkey->listFetch_s) + goto err_exit; list[keys].attrs[i].name_len = _libssh2_ntohu32(pkey->listFetch_s); pkey->listFetch_s += 4; + if (list[keys].attrs[i].name_len > (size_t)(end - pkey->listFetch_s)) + goto err_exit; list[keys].attrs[i].name = (char *) pkey->listFetch_s; pkey->listFetch_s += list[keys].attrs[i].name_len; + if (4 > end - pkey->listFetch_s) + goto err_exit; list[keys].attrs[i].value_len = _libssh2_ntohu32(pkey->listFetch_s); pkey->listFetch_s += 4; + if (list[keys].attrs[i].value_len > (size_t)(end - pkey->listFetch_s)) + goto err_exit; list[keys].attrs[i].value = (char *) pkey->listFetch_s; pkey->listFetch_s += list[keys].attrs[i].value_len; diff --git a/src/session.c b/src/session.c index 06e61dd..ba1bad5 100644 --- a/src/session.c +++ b/src/session.c @@ -763,6 +763,7 @@ session_startup(LIBSSH2_SESSION *session, libssh2_socket_t sock) return rc; session->startup_service_length = + (5 > session->startup_data_len) ? 0 : _libssh2_ntohu32(session->startup_data + 1); if ((session->startup_service_length != (sizeof("ssh-userauth") - 1)) @@ -1410,6 +1411,7 @@ libssh2_poll_channel_read(LIBSSH2_CHANNEL *channel, int extended) packet = _libssh2_list_first(&session->packets); while (packet) { + /* XXX assert(packet->data_len >= 5) XXX */ if ( channel->local.id == _libssh2_ntohu32(packet->data + 1)) { if ( extended == 1 && (packet->data[0] == SSH_MSG_CHANNEL_EXTENDED_DATA diff --git a/src/sftp.c b/src/sftp.c index c142713..ad38638 100644 --- a/src/sftp.c +++ b/src/sftp.c @@ -249,6 +249,7 @@ sftp_packet_add(LIBSSH2_SFTP *sftp, unsigned char *data, "Out of sync with the world"); } + /* XXX ??? assert(data_len >= 5); XXX */ request_id = _libssh2_ntohu32(&data[1]); _libssh2_debug(session, LIBSSH2_TRACE_SFTP, "Received packet id %d", @@ -635,21 +636,25 @@ sftp_attr2bin(unsigned char *p, const LIBSSH2_SFTP_ATTRIBUTES * attrs) /* sftp_bin2attr */ -static int -sftp_bin2attr(LIBSSH2_SFTP_ATTRIBUTES * attrs, const unsigned char *p) +static const unsigned char * +sftp_bin2attr(LIBSSH2_SFTP_ATTRIBUTES * attrs, const unsigned char *s, const unsigned char *end) { - const unsigned char *s = p; - memset(attrs, 0, sizeof(LIBSSH2_SFTP_ATTRIBUTES)); + if (4 < end - p) + return NULL; attrs->flags = _libssh2_ntohu32(s); s += 4; if (attrs->flags & LIBSSH2_SFTP_ATTR_SIZE) { + if (8 < end - p) + return NULL; attrs->filesize = _libssh2_ntohu64(s); s += 8; } if (attrs->flags & LIBSSH2_SFTP_ATTR_UIDGID) { + if (4*2 < end - p) + return NULL; attrs->uid = _libssh2_ntohu32(s); s += 4; attrs->gid = _libssh2_ntohu32(s); @@ -657,18 +662,22 @@ sftp_bin2attr(LIBSSH2_SFTP_ATTRIBUTES * attrs, const unsigned char *p) } if (attrs->flags & LIBSSH2_SFTP_ATTR_PERMISSIONS) { + if (4 < end - p) + return NULL; attrs->permissions = _libssh2_ntohu32(s); s += 4; } if (attrs->flags & LIBSSH2_SFTP_ATTR_ACMODTIME) { + if (4*2 < end - p) + return NULL; attrs->atime = _libssh2_ntohu32(s); s += 4; attrs->mtime = _libssh2_ntohu32(s); s += 4; } - return (s - p); + return s; } /* ************ @@ -1698,7 +1707,7 @@ static ssize_t sftp_readdir(LIBSSH2_SFTP_HANDLE *handle, char *buffer, if (attrs) memset(attrs, 0, sizeof(LIBSSH2_SFTP_ATTRIBUTES)); - s += sftp_bin2attr(attrs ? attrs : &attrs_dummy, s); + s = sftp_bin2attr(attrs ? attrs : &attrs_dummy, s, handle->u.dir.names_end); handle->u.dir.next_name = (char *) s; end: @@ -1789,6 +1798,7 @@ static ssize_t sftp_readdir(LIBSSH2_SFTP_HANDLE *handle, char *buffer, handle->u.dir.names_left = num_names; handle->u.dir.names_packet = data; + handle->u.dir.names_end = data + data_len; handle->u.dir.next_name = (char *) data + 9; /* use the name popping mechanism from the start of the function */ @@ -2252,7 +2262,7 @@ static int sftp_fstat(LIBSSH2_SFTP_HANDLE *handle, } } - sftp_bin2attr(attrs, data + 5); + sftp_bin2attr(attrs, data + 5, data + data_len); LIBSSH2_FREE(session, data); return 0; @@ -2559,6 +2569,7 @@ static int sftp_unlink(LIBSSH2_SFTP *sftp, const char *filename, sftp->unlink_state = libssh2_NB_state_idle; + /* XXX ??? assert(data_len >= 5+4); XXX */ retcode = _libssh2_ntohu32(data + 5); LIBSSH2_FREE(session, data); @@ -2669,6 +2680,7 @@ static int sftp_rename(LIBSSH2_SFTP *sftp, const char *source_filename, sftp->rename_state = libssh2_NB_state_idle; + /* XXX ??? assert(data_len >= 5+4); XXX */ retcode = _libssh2_ntohu32(data + 5); LIBSSH2_FREE(session, data); @@ -2793,6 +2805,7 @@ static int sftp_fstatvfs(LIBSSH2_SFTP_HANDLE *handle, LIBSSH2_SFTP_STATVFS *st) "Error waiting for FXP EXTENDED REPLY"); } + /* XXX ??? assert(data_len >= 5+4); XXX */ if (data[0] == SSH_FXP_STATUS) { int retcode = _libssh2_ntohu32(data + 5); sftp->fstatvfs_state = libssh2_NB_state_idle; @@ -2919,6 +2932,7 @@ static int sftp_statvfs(LIBSSH2_SFTP *sftp, const char *path, "Error waiting for FXP EXTENDED REPLY"); } + /* XXX ??? assert(data_len >= 5+4); XXX */ if (data[0] == SSH_FXP_STATUS) { int retcode = _libssh2_ntohu32(data + 5); sftp->statvfs_state = libssh2_NB_state_idle; @@ -3051,6 +3065,7 @@ static int sftp_mkdir(LIBSSH2_SFTP *sftp, const char *path, sftp->mkdir_state = libssh2_NB_state_idle; + /* XXX ??? assert(data_len >= 5+4); XXX */ retcode = _libssh2_ntohu32(data + 5); LIBSSH2_FREE(session, data); @@ -3145,6 +3160,7 @@ static int sftp_rmdir(LIBSSH2_SFTP *sftp, const char *path, sftp->rmdir_state = libssh2_NB_state_idle; + /* XXX ??? assert(data_len >= 5+4); XXX */ retcode = _libssh2_ntohu32(data + 5); LIBSSH2_FREE(session, data); @@ -3188,6 +3204,7 @@ static int sftp_stat(LIBSSH2_SFTP *sftp, const char *path, ((stat_type == LIBSSH2_SFTP_SETSTAT) ? sftp_attrsize(attrs->flags) : 0); unsigned char *s, *data; + unsigned char *data_end; static const unsigned char stat_responses[2] = { SSH_FXP_ATTRS, SSH_FXP_STATUS }; int rc; @@ -3258,6 +3275,8 @@ static int sftp_stat(LIBSSH2_SFTP *sftp, const char *path, sftp->stat_state = libssh2_NB_state_idle; + /* XXX ??? assert(data_len >= 5+4); XXX */ + if (data[0] == SSH_FXP_STATUS) { int retcode; @@ -3273,7 +3292,7 @@ static int sftp_stat(LIBSSH2_SFTP *sftp, const char *path, } memset(attrs, 0, sizeof(LIBSSH2_SFTP_ATTRIBUTES)); - sftp_bin2attr(attrs, data + 5); + sftp_bin2attr(attrs, data + 5, data + data_len); LIBSSH2_FREE(session, data); return 0; @@ -3389,6 +3408,8 @@ static int sftp_symlink(LIBSSH2_SFTP *sftp, const char *path, sftp->symlink_state = libssh2_NB_state_idle; + /* XXX ??? assert(data_len >= 5+4); XXX */ + if (data[0] == SSH_FXP_STATUS) { int retcode; @@ -3410,8 +3431,13 @@ static int sftp_symlink(LIBSSH2_SFTP *sftp, const char *path, "no name entries"); } + /* XXX ??? assert(data_len >= 5+4*2); XXX */ + /* this reads a u32 and stores it into a signed 32bit value */ link_len = _libssh2_ntohu32(data + 9); + + /* XXX ??? assert(data_len-(5+4*2) >= link_len); XXX */ + if (link_len < target_len) { memcpy(target, data + 13, link_len); target[link_len] = 0; diff --git a/src/sftp.h b/src/sftp.h index 2ed32ce..91fc0a7 100644 --- a/src/sftp.h +++ b/src/sftp.h @@ -122,6 +122,7 @@ struct _LIBSSH2_SFTP_HANDLE uint32_t names_left; void *names_packet; char *next_name; + char *names_end; } dir; } u; diff --git a/src/userauth.c b/src/userauth.c index cdfa25e..c799a40 100644 --- a/src/userauth.c +++ b/src/userauth.c @@ -69,6 +69,7 @@ static char *userauth_list(LIBSSH2_SESSION *session, const char *username, service(14)"ssh-connection" + method_len(4) = 27 */ unsigned long methods_len; unsigned char *s; + unsigned char *end; int rc; if (session->userauth_list_state == libssh2_NB_state_idle) { @@ -143,7 +144,18 @@ static char *userauth_list(LIBSSH2_SESSION *session, const char *username, return NULL; } + if (5 > session->userauth_list_data_len) { + /* XXX ??? XXX */ +userauth_packet_overrun: + LIBSSH2_FREE(session, session->userauth_list_data); + session->userauth_list_data = NULL; + session->userauth_list_state = libssh2_NB_state_idle; + return NULL; + } methods_len = _libssh2_ntohu32(session->userauth_list_data + 1); + if (methods_len > session->userauth_list_data_len - 5) { + goto userauth_packet_overrun; + } /* Do note that the memory areas overlap! */ memmove(session->userauth_list_data, session->userauth_list_data + 5, @@ -1561,6 +1573,7 @@ userauth_keyboard_interactive(LIBSSH2_SESSION * session, LIBSSH2_USERAUTH_KBDINT_RESPONSE_FUNC((*response_callback))) { unsigned char *s; + unsigned char *end; int rc; static const unsigned char reply_codes[4] = { @@ -1685,10 +1698,15 @@ userauth_keyboard_interactive(LIBSSH2_SESSION * session, /* server requested PAM-like conversation */ s = session->userauth_kybd_data + 1; + end = session->userauth_kybd_data + session->userauth_kybd_data_len; /* string name (ISO-10646 UTF-8) */ + if (4 > end - s) + goto cleanup; /* XXX ??? XXX */ session->userauth_kybd_auth_name_len = _libssh2_ntohu32(s); s += 4; + if (session->userauth_kybd_auth_name_len > (size_t)(end - s)) + goto cleanup; /* XXX ??? XXX */ if(session->userauth_kybd_auth_name_len) { session->userauth_kybd_auth_name = LIBSSH2_ALLOC(session, @@ -1706,8 +1724,12 @@ userauth_keyboard_interactive(LIBSSH2_SESSION * session, } /* string instruction (ISO-10646 UTF-8) */ + if (4 > end - s) + goto cleanup; /* XXX ??? XXX */ session->userauth_kybd_auth_instruction_len = _libssh2_ntohu32(s); s += 4; + if (session->userauth_kybd_auth_instruction_len > (size_t)(end - s)) + goto cleanup; /* XXX ??? XXX */ if(session->userauth_kybd_auth_instruction_len) { session->userauth_kybd_auth_instruction = LIBSSH2_ALLOC(session, @@ -1725,13 +1747,19 @@ userauth_keyboard_interactive(LIBSSH2_SESSION * session, } /* string language tag (as defined in [RFC-3066]) */ + if (4 > end - s) + goto cleanup; /* XXX ??? XXX */ language_tag_len = _libssh2_ntohu32(s); s += 4; + if (language_tag_len > (size_t)(end - s)) + goto cleanup; /* XXX ??? XXX */ /* ignoring this field as deprecated */ s += language_tag_len; /* int num-prompts */ + if (4 > end - s) + goto cleanup; /* XXX ??? XXX */ session->userauth_kybd_num_prompts = _libssh2_ntohu32(s); s += 4; @@ -1760,9 +1788,13 @@ userauth_keyboard_interactive(LIBSSH2_SESSION * session, for(i = 0; i < session->userauth_kybd_num_prompts; i++) { /* string prompt[1] (ISO-10646 UTF-8) */ + if (4 > end - s) + goto cleanup; /* XXX ??? XXX */ session->userauth_kybd_prompts[i].length = _libssh2_ntohu32(s); s += 4; + if (session->userauth_kybd_prompts[i].length > (size_t)(end - s)) + goto cleanup; /* XXX ??? XXX */ session->userauth_kybd_prompts[i].text = LIBSSH2_CALLOC(session, session->userauth_kybd_prompts[i].length);
_______________________________________________ libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel