From 41fbd4476ab18814002082d5c972d32550182e89 Mon Sep 17 00:00:00 2001 From: Michael Buckley Date: Fri, 7 Dec 2018 15:10:07 -0800 Subject: [PATCH] Use string_buf in sftp_init(). --- include/libssh2_sftp.h | 8 +-- src/misc.c | 13 +++++ src/misc.h | 1 + src/sftp.c | 108 ++++++++++++----------------------------- 4 files changed, 50 insertions(+), 80 deletions(-) diff --git a/include/libssh2_sftp.h b/include/libssh2_sftp.h index ef5ec7d2..4eea32c2 100644 --- a/include/libssh2_sftp.h +++ b/include/libssh2_sftp.h @@ -97,12 +97,12 @@ struct _LIBSSH2_SFTP_ATTRIBUTES { /* If flags & ATTR_* bit is set, then the value in this struct will be * meaningful Otherwise it should be ignored */ - unsigned long flags; + uint32_t flags; libssh2_uint64_t filesize; - unsigned long uid, gid; - unsigned long permissions; - unsigned long atime, mtime; + uint32_t uid, gid; + uint32_t permissions; + uint32_t atime, mtime; }; struct _LIBSSH2_SFTP_STATVFS { diff --git a/src/misc.c b/src/misc.c index e1ea7b04..e02f60fc 100644 --- a/src/misc.c +++ b/src/misc.c @@ -744,6 +744,19 @@ int _libssh2_get_u32(struct string_buf *buf, uint32_t *out) return 0; } +int _libssh2_get_u64(struct string_buf *buf, uint64_t *out) +{ + if (!_libssh2_check_length(buf, 8)) { + return -1; + } + + unsigned char *p = buf->dataptr; + *out = _libssh2_ntohu64(p); + buf->dataptr += 8; + buf->offset -= 8; + return 0; +} + int _libssh2_match_string(struct string_buf *buf, const char *match) { unsigned char *out; diff --git a/src/misc.h b/src/misc.h index 858f3567..d569c921 100644 --- a/src/misc.h +++ b/src/misc.h @@ -91,6 +91,7 @@ void _libssh2_explicit_zero(void *buf, size_t size); struct string_buf* _libssh2_string_buf_new(LIBSSH2_SESSION *session); void _libssh2_string_buf_free(LIBSSH2_SESSION *session, struct string_buf *buf); int _libssh2_get_u32(struct string_buf *buf, uint32_t *out); +int _libssh2_get_u64(struct string_buf *buf, uint64_t *out); int _libssh2_match_string(struct string_buf *buf, const char *match); int _libssh2_get_c_string(struct string_buf *buf, unsigned char **outbuf); int _libssh2_get_bignum_bytes(struct string_buf *buf, unsigned char **outbuf); diff --git a/src/sftp.c b/src/sftp.c index 31038341..66e66f9f 100644 --- a/src/sftp.c +++ b/src/sftp.c @@ -670,66 +670,43 @@ sftp_attr2bin(unsigned char *p, const LIBSSH2_SFTP_ATTRIBUTES * attrs) static int sftp_bin2attr(LIBSSH2_SFTP_ATTRIBUTES * attrs, const unsigned char *p, size_t data_len) { - const unsigned char *s = p; + struct string_buf buf; + buf.data = (unsigned char *)p; + buf.dataptr = buf.data; + buf.len = data_len; + buf.offset = 0; - if (data_len >= 4) { - memset(attrs, 0, sizeof(LIBSSH2_SFTP_ATTRIBUTES)); - attrs->flags = _libssh2_ntohu32(s); - s += 4; - data_len -= 4; - } - else { + if (_libssh2_get_u32(&buf, &(attrs->flags)) != 0) { return LIBSSH2_ERROR_BUFFER_TOO_SMALL; } if (attrs->flags & LIBSSH2_SFTP_ATTR_SIZE) { - if (data_len >= 8) { - attrs->filesize = _libssh2_ntohu64(s); - s += 8; - data_len -= 8; - } - else { + if (_libssh2_get_u64(&buf, &(attrs->filesize)) != 0) { return LIBSSH2_ERROR_BUFFER_TOO_SMALL; } } if (attrs->flags & LIBSSH2_SFTP_ATTR_UIDGID) { - if (data_len >= 8) { - attrs->uid = _libssh2_ntohu32(s); - s += 4; - attrs->gid = _libssh2_ntohu32(s); - s += 4; - data_len -= 8; - } - else { + if (_libssh2_get_u32(&buf, &(attrs->uid)) != 0 || + _libssh2_get_u32(&buf, &(attrs->gid)) != 0) { return LIBSSH2_ERROR_BUFFER_TOO_SMALL; } } if (attrs->flags & LIBSSH2_SFTP_ATTR_PERMISSIONS) { - if (data_len >= 4) { - attrs->permissions = _libssh2_ntohu32(s); - s += 4; - data_len -= 4; - } - else { + if (_libssh2_get_u32(&buf, &(attrs->permissions)) != 0) { return LIBSSH2_ERROR_BUFFER_TOO_SMALL; } } if (attrs->flags & LIBSSH2_SFTP_ATTR_ACMODTIME) { - if (data_len >= 8) { - attrs->atime = _libssh2_ntohu32(s); - s += 4; - attrs->mtime = _libssh2_ntohu32(s); - s += 4; - } - else { + if (_libssh2_get_u32(&buf, &(attrs->atime)) != 0 || + _libssh2_get_u32(&buf, &(attrs->mtime)) != 0) { return LIBSSH2_ERROR_BUFFER_TOO_SMALL; } } - return (s - p); + return (buf.dataptr - buf.data); } /* ************ @@ -768,7 +745,7 @@ LIBSSH2_CHANNEL_CLOSE_FUNC(libssh2_sftp_dtor) */ static LIBSSH2_SFTP *sftp_init(LIBSSH2_SESSION *session) { - unsigned char *data, *s; + unsigned char *data; size_t data_len; ssize_t rc; LIBSSH2_SFTP *sftp_handle; @@ -917,9 +894,18 @@ static LIBSSH2_SFTP *sftp_init(LIBSSH2_SESSION *session) goto sftp_init_error; } - s = data + 1; - sftp_handle->version = _libssh2_ntohu32(s); - s += 4; + struct string_buf buf; + buf.data = data; + buf.dataptr = buf.data + 1; + buf.len = data_len; + buf.offset = 1; + + if (_libssh2_get_u32(&buf, &(sftp_handle->version)) != 0) { + LIBSSH2_FREE(session, data); + rc = LIBSSH2_ERROR_BUFFER_TOO_SMALL; + goto sftp_init_error; + } + if(sftp_handle->version > LIBSSH2_SFTP_VERSION) { _libssh2_debug(session, LIBSSH2_TRACE_SFTP, "Truncating remote SFTP version from %lu", @@ -929,50 +915,20 @@ static LIBSSH2_SFTP *sftp_init(LIBSSH2_SESSION *session) _libssh2_debug(session, LIBSSH2_TRACE_SFTP, "Enabling SFTP version %lu compatibility", sftp_handle->version); - while(s < (data + data_len)) { - size_t extname_len, extdata_len; + while (buf.offset < buf.len) { + unsigned char *extname, *extdata; - if( s + 4 <= data + data_len ) { - extname_len = _libssh2_ntohu32(s); - s += 4; - } - else { + if (_libssh2_get_c_string(&buf, &extname) < 0) { LIBSSH2_FREE(session, data); _libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL, - "Data too short when extracting extname_len"); + "Data too short when extracting extname"); goto sftp_init_error; } - if(s + extname_len <= data + data_len) { - /* the extension name starts here */ - s += extname_len; - } - else { + if (_libssh2_get_c_string(&buf, &extdata) < 0) { LIBSSH2_FREE(session, data); _libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL, - "Data too short for extname"); - goto sftp_init_error; - } - - if( s + 4 <= data + data_len ) { - extdata_len = _libssh2_ntohu32(s); - s += 4; - } - else { - LIBSSH2_FREE(session, data); - _libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL, - "Data too short when extracting extdata_len"); - goto sftp_init_error; - } - - if(s + extdata_len <= data + data_len) { - /* TODO: Actually process extensions */ - s += extdata_len; - } - else { - LIBSSH2_FREE(session, data); - _libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL, - "Data too short for extdata"); + "Data too short when extracting extdata"); goto sftp_init_error; } }