On 2019-03-31 11:38 , Will Cosgrove wrote: > The 1.8.x branch fixes only resolves the issues brought up by the > Conicanal review while master contains a more exhaustive review and is > highly recommended to use.
Stable distro won't use git master. You can say it is "downstream problem", but it won't change the end result: either upstream provides backport(ed|able) security fixes for few previous branches, or most of end users will use vulnerable code for years. And fixes in libssh2 git master are not backportable, 1.8.x branch is. > I will submit a patch on Monday for the check length function, thanks > for bringing that to our attention. Not sure if it is still possible to practically exploit it (as of c07bc647f), but those (int) casts are wrong (and unneeded). Anyway, /proper/ check is: - if(len > buf->len) - return 0; - - return ((int)(buf->dataptr - buf->data) <= (int)(buf->len - len)) ? 1 : 0; + return len <= (size_t)((buf->data + buf->len) - buf->dataptr); > That said, the project is in need of people who contribute and it > would be very helpful if you would submit a PR regardless of past PRs > not being taken for whatever reason. I'm able to identify certain class of problems (as I said below in OP, "code around _libssh2_ntohu32 often looks wrong, please review and fix it"), but I don't understand libssh2 code to extent I can design replacement. >> On Mar 31, 2019, at 11:22 AM, Yuriy M. Kaminskiy <yum...@gmail.com> wrote: >> >>> On 31.03.2019 14:23, Yuriy M. Kaminskiy wrote: >>> FTR, (some) problems that was addressed by this patch was (apparently >>> independently) rediscovered 3 years later, assigned CVE-2019-38{55...63} >>> and fixed (differently; I have not checked if fixed code covers all >>> cases was covered by my patch). >>> >>> BTW, _libssh2_check_length() that is extensively used by current code is >>> broken/incorrect; e.g. suppose >>> >>> buf->dataptr = buf->data, buf->len = 5, len = 0xfffffff7 >>> >>> then _libssh2_check_length(buf, len) will return 1; uh-doh. >>> >>> With obvious security implications. >>> >>> (No, I'm not going to compose patch to be ignored for another 3 years). >> >> Ah, yeah, forgot to look at 1.8.x branch. No _check_length there, but >> other problematic code present instead: >> >> uint32_t len = _libssh2_ntohu32(data + 5); >> ... >> if((len + 9) < datalen) >> >> Broken when len > UINT32_MAX - 9. >> >> if(datalen >= 9) { >> message_len = _libssh2_ntohu32(data + 5); >> if(message_len < datalen-13) { >> >> Broken when datalen >= 9 && datalen < 13 (and there are more similar code). >> >> etc. >> >>>> On 2016-03-27 22:28 , Yuriy M. Kaminskiy wrote: >>>> Ping? I'd like to stress out this issue has security imlications. At >>>> very least, DoS (and this is not a standalone application, so it is not >>>> a minor issue), and maybe host memory exposure too. (However, it is only >>>> heap over-reads, without heap/stack over-writes, so no risk of >>>> escalating to remote code execution). >>>> >>>>> On 02/25/16 03:10 , Yuriy M. Kaminskiy wrote: >>>>> "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 https://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel