Subject: Re: [PATCH][WIP][v2] Fix out-of-buffer-boundary reads

Re: [PATCH][WIP][v2] Fix out-of-buffer-boundary reads

From: Yuriy M. Kaminskiy <yumkam_at_gmail.com>
Date: Wed, 3 Apr 2019 23:45:07 +0300

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 <yumkam_at_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_online_at_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
Received on 2019-04-03