Skip to content
Permalink
Browse files Browse the repository at this point in the history
packet.c: improve message parsing (#402)
* packet.c: improve parsing of packets

file: packet.c

notes:
Use _libssh2_get_string API in SSH_MSG_DEBUG/SSH_MSG_DISCONNECT. Additional uint32 bounds check in SSH_MSG_GLOBAL_REQUEST.
  • Loading branch information
willco007 committed Aug 30, 2019
1 parent e573299 commit dedcbd1
Showing 1 changed file with 29 additions and 39 deletions.
68 changes: 29 additions & 39 deletions src/packet.c
Expand Up @@ -419,8 +419,8 @@ _libssh2_packet_add(LIBSSH2_SESSION * session, unsigned char *data,
size_t datalen, int macstate)
{
int rc = 0;
char *message = NULL;
char *language = NULL;
unsigned char *message = NULL;
unsigned char *language = NULL;
size_t message_len = 0;
size_t language_len = 0;
LIBSSH2_CHANNEL *channelp = NULL;
Expand Down Expand Up @@ -472,33 +472,23 @@ _libssh2_packet_add(LIBSSH2_SESSION * session, unsigned char *data,

case SSH_MSG_DISCONNECT:
if(datalen >= 5) {
size_t reason = _libssh2_ntohu32(data + 1);
uint32_t reason = 0;
struct string_buf buf;
buf.data = (unsigned char *)data;
buf.dataptr = buf.data;
buf.len = datalen;
buf.dataptr++; /* advance past type */

if(datalen >= 9) {
message_len = _libssh2_ntohu32(data + 5);
_libssh2_get_u32(&buf, &reason);
_libssh2_get_string(&buf, &message, &message_len);
_libssh2_get_string(&buf, &language, &language_len);

if(message_len < datalen-13) {
/* 9 = packet_type(1) + reason(4) + message_len(4) */
message = (char *) data + 9;

language_len =
_libssh2_ntohu32(data + 9 + message_len);
language = (char *) data + 9 + message_len + 4;

if(language_len > (datalen-13-message_len)) {
/* bad input, clear info */
language = message = NULL;
language_len = message_len = 0;
}
}
else
/* bad size, clear it */
message_len = 0;
}
if(session->ssh_msg_disconnect) {
LIBSSH2_DISCONNECT(session, reason, message,
message_len, language, language_len);
LIBSSH2_DISCONNECT(session, reason, (const char *)message,
message_len, (const char *)language,
language_len);
}

_libssh2_debug(session, LIBSSH2_TRACE_TRANS,
"Disconnect(%d): %s(%s)", reason,
message, language);
Expand Down Expand Up @@ -539,24 +529,24 @@ _libssh2_packet_add(LIBSSH2_SESSION * session, unsigned char *data,
int always_display = data[1];

if(datalen >= 6) {
message_len = _libssh2_ntohu32(data + 2);

if(message_len <= (datalen - 10)) {
/* 6 = packet_type(1) + display(1) + message_len(4) */
message = (char *) data + 6;
language_len = _libssh2_ntohu32(data + 6 +
message_len);

if(language_len <= (datalen - 10 - message_len))
language = (char *) data + 10 + message_len;
}
struct string_buf buf;
buf.data = (unsigned char *)data;
buf.dataptr = buf.data;
buf.len = datalen;
buf.dataptr += 2; /* advance past type & always display */

_libssh2_get_string(&buf, &message, &message_len);
_libssh2_get_string(&buf, &language, &language_len);
}

if(session->ssh_msg_debug) {
LIBSSH2_DEBUG(session, always_display, message,
message_len, language, language_len);
LIBSSH2_DEBUG(session, always_display,
(const char *)message,
message_len, (const char *)language,
language_len);
}
}

/*
* _libssh2_debug will actually truncate this for us so
* that it's not an inordinate about of data
Expand All @@ -579,7 +569,7 @@ _libssh2_packet_add(LIBSSH2_SESSION * session, unsigned char *data,
uint32_t len = 0;
unsigned char want_reply = 0;
len = _libssh2_ntohu32(data + 1);
if(datalen >= (6 + len)) {
if((len <= (UINT_MAX - 6)) && (datalen >= (6 + len))) {
want_reply = data[5 + len];
_libssh2_debug(session,
LIBSSH2_TRACE_CONN,
Expand Down

2 comments on commit dedcbd1

@kloczek
Copy link

@kloczek kloczek commented on dedcbd1 Dec 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like only after that commit it would be good to make new release (1.9.1?)

If may I ask on that ocation consider change tagging convention to use in tag only version because github automatically generatet tar balls are using <repo_name>-<tag> and by this tar like https://github.com/libssh2/libssh2/archive/libssh2-1.9.0.tar.gz has base directory libssh2-libssh2-1.9.0/.

@Beyond-My
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,When I added this patch, the error was reported as follows: Can you tell me the reason?

15:05:53 DEBUG: packet.c: In function '_libssh2_packet_add':
15:05:53 DEBUG: packet.c:473:36: error: storage size of 'buf' isn't known
15:05:53 DEBUG: struct string_buf buf;
15:05:53 DEBUG: ^
15:05:53 DEBUG: packet.c:526:36: error: storage size of 'buf' isn't known
15:05:53 DEBUG: struct string_buf buf;

Please sign in to comment.