From 12284b75fa5e5f6439223c07cbeb49a5eadc9c6b Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Wed, 27 Jun 2018 14:10:26 +0200 Subject: [PATCH] buffer: Add and use ssh_buffer_allocate_size() Add a small helper for ssh_buffer to ensure that the buffer has a certain amount of space already preallocated. This can be useful in case it is known how much data is going to be added to a buffer, to avoid multiple reallocations. Make use of it in few places in the library. Signed-off-by: Pino Toscano Reviewed-by: Andreas Schneider --- include/libssh/buffer.h | 1 + src/agent.c | 15 ++++ src/buffer.c | 28 ++++++++ src/dh.c | 18 +++++ src/pcap.c | 12 ++++ src/sftp.c | 147 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 221 insertions(+) diff --git a/include/libssh/buffer.h b/include/libssh/buffer.h index 0765ce07..2119f2cc 100644 --- a/include/libssh/buffer.h +++ b/include/libssh/buffer.h @@ -51,6 +51,7 @@ int ssh_buffer_add_u64(ssh_buffer buffer, uint64_t data); int ssh_buffer_validate_length(struct ssh_buffer_struct *buffer, size_t len); void *ssh_buffer_allocate(struct ssh_buffer_struct *buffer, uint32_t len); +int ssh_buffer_allocate_size(struct ssh_buffer_struct *buffer, uint32_t len); int ssh_buffer_pack_va(struct ssh_buffer_struct *buffer, const char *format, int argc, diff --git a/src/agent.c b/src/agent.c index 2b063074..114dadf1 100644 --- a/src/agent.c +++ b/src/agent.c @@ -514,6 +514,21 @@ ssh_string ssh_agent_sign_data(ssh_session session, return NULL; } + /* + * make sure it already can contain all the expected content: + * - 1 x uint8_t + * - 2 x uint32_t + * - 1 x ssh_string (uint8_t + data) + */ + rc = ssh_buffer_allocate_size(request, + sizeof(uint8_t) * 2 + + sizeof(uint32_t) * 2 + + ssh_string_len(key_blob)); + if (rc < 0) { + ssh_buffer_free(request); + return NULL; + } + /* adds len + blob */ rc = ssh_buffer_add_ssh_string(request, key_blob); ssh_string_free(key_blob); diff --git a/src/buffer.c b/src/buffer.c index 5c583831..51ecc49f 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -248,6 +248,34 @@ int ssh_buffer_add_data(struct ssh_buffer_struct *buffer, const void *data, uint return 0; } +/** + * @brief Ensure the buffer has at least a certain preallocated size. + * + * @param[in] buffer The buffer to enlarge. + * + * @param[in] len The length to ensure as allocated. + * + * @return 0 on success, < 0 on error. + */ +int ssh_buffer_allocate_size(struct ssh_buffer_struct *buffer, + uint32_t len) +{ + buffer_verify(buffer); + + if (buffer->allocated < len) { + if (buffer->pos > 0) { + buffer_shift(buffer); + } + if (realloc_buffer(buffer, len) < 0) { + return -1; + } + } + + buffer_verify(buffer); + + return 0; +} + /** * @internal * diff --git a/src/dh.c b/src/dh.c index 1d422c82..e0d2965a 100644 --- a/src/dh.c +++ b/src/dh.c @@ -731,11 +731,20 @@ error: } int ssh_hashbufout_add_cookie(ssh_session session) { + int rc; + session->out_hashbuf = ssh_buffer_new(); if (session->out_hashbuf == NULL) { return -1; } + rc = ssh_buffer_allocate_size(session->out_hashbuf, + sizeof(uint8_t) + 16); + if (rc < 0) { + ssh_buffer_reinit(session->out_hashbuf); + return -1; + } + if (ssh_buffer_add_u8(session->out_hashbuf, 20) < 0) { ssh_buffer_reinit(session->out_hashbuf); return -1; @@ -759,11 +768,20 @@ int ssh_hashbufout_add_cookie(ssh_session session) { } int ssh_hashbufin_add_cookie(ssh_session session, unsigned char *cookie) { + int rc; + session->in_hashbuf = ssh_buffer_new(); if (session->in_hashbuf == NULL) { return -1; } + rc = ssh_buffer_allocate_size(session->in_hashbuf, + sizeof(uint8_t) + 20 + 16); + if (rc < 0) { + ssh_buffer_reinit(session->in_hashbuf); + return -1; + } + if (ssh_buffer_add_u8(session->in_hashbuf, 20) < 0) { ssh_buffer_reinit(session->in_hashbuf); return -1; diff --git a/src/pcap.c b/src/pcap.c index 0ec2fbb4..ffb074cf 100644 --- a/src/pcap.c +++ b/src/pcap.c @@ -165,6 +165,12 @@ int ssh_pcap_file_write_packet(ssh_pcap_file pcap, ssh_buffer packet, uint32_t o if(header == NULL) return SSH_ERROR; gettimeofday(&now,NULL); + err = ssh_buffer_allocate_size(header, + sizeof(uint32_t) * 4 + + ssh_buffer_get_len(packet)); + if (err < 0) { + goto error; + } err = ssh_buffer_add_u32(header,htonl(now.tv_sec)); if (err < 0) { goto error; @@ -209,6 +215,12 @@ int ssh_pcap_file_open(ssh_pcap_file pcap, const char *filename){ header=ssh_buffer_new(); if(header==NULL) return SSH_ERROR; + err = ssh_buffer_allocate_size(header, + sizeof(uint32_t) * 5 + + sizeof(uint16_t) * 2); + if (err < 0) { + goto error; + } err = ssh_buffer_add_u32(header,htonl(PCAP_MAGIC)); if (err < 0) { goto error; diff --git a/src/sftp.c b/src/sftp.c index 80870d4e..3dd4c60e 100644 --- a/src/sftp.c +++ b/src/sftp.c @@ -318,6 +318,11 @@ sftp_packet sftp_packet_read(sftp_session sftp) { return NULL; } + r = ssh_buffer_allocate_size(packet->payload, 4); + if (r < 0) { + ssh_set_error_oom(sftp->session); + goto error; + } r=0; do { // read from channel until 4 bytes have been read or an error occurs @@ -355,6 +360,11 @@ sftp_packet sftp_packet_read(sftp_session sftp) { } size--; + r = ssh_buffer_allocate_size(packet->payload, size); + if (r < 0) { + ssh_set_error_oom(sftp->session); + goto error; + } while (size > 0 && size < UINT_MAX) { r=ssh_channel_read(sftp->channel,buffer, sizeof(buffer)>size ? size:sizeof(buffer),0); @@ -849,6 +859,7 @@ sftp_dir sftp_opendir(sftp_session sftp, const char *path){ ssh_string path_s; ssh_buffer payload; uint32_t id; + int rc; payload = ssh_buffer_new(); if (payload == NULL) { @@ -864,6 +875,14 @@ sftp_dir sftp_opendir(sftp_session sftp, const char *path){ } id = sftp_get_new_id(sftp); + rc = ssh_buffer_allocate_size(payload, + sizeof(uint32_t) * 2 + ssh_string_len(path_s)); + if (rc < 0) { + ssh_set_error_oom(sftp->session); + ssh_buffer_free(payload); + ssh_string_free(path_s); + return NULL; + } if (ssh_buffer_add_u32(payload, htonl(id)) < 0 || ssh_buffer_add_ssh_string(payload, path_s) < 0) { ssh_set_error_oom(sftp->session); @@ -1384,6 +1403,7 @@ sftp_attributes sftp_readdir(sftp_session sftp, sftp_dir dir) { sftp_attributes attr; ssh_buffer payload; uint32_t id; + int rc; if (dir->buffer == NULL) { payload = ssh_buffer_new(); @@ -1392,6 +1412,14 @@ sftp_attributes sftp_readdir(sftp_session sftp, sftp_dir dir) { return NULL; } + rc = ssh_buffer_allocate_size(payload, + sizeof(uint32_t) * 2 + + ssh_string_len(dir->handle)); + if (rc < 0) { + ssh_set_error_oom(sftp->session); + ssh_buffer_free(payload); + return NULL; + } id = sftp_get_new_id(sftp); if (ssh_buffer_add_u32(payload, htonl(id)) < 0 || ssh_buffer_add_ssh_string(payload, dir->handle) < 0) { @@ -1509,6 +1537,7 @@ static int sftp_handle_close(sftp_session sftp, ssh_string handle) { sftp_message msg = NULL; ssh_buffer buffer = NULL; uint32_t id; + int rc; buffer = ssh_buffer_new(); if (buffer == NULL) { @@ -1516,6 +1545,15 @@ static int sftp_handle_close(sftp_session sftp, ssh_string handle) { return -1; } + rc = ssh_buffer_allocate_size(buffer, + sizeof(uint32_t) * 2 + + ssh_string_len(handle)); + if (rc < 0) { + ssh_set_error_oom(sftp->session); + ssh_buffer_free(buffer); + return -1; + } + id = sftp_get_new_id(sftp); if (ssh_buffer_add_u32(buffer, htonl(id)) < 0 || ssh_buffer_add_ssh_string(buffer, handle) < 0) { @@ -1609,6 +1647,7 @@ sftp_file sftp_open(sftp_session sftp, const char *file, int flags, sftp_attributes stat_data; uint32_t sftp_flags = 0; uint32_t id; + int rc; buffer = ssh_buffer_new(); if (buffer == NULL) { @@ -1644,6 +1683,15 @@ sftp_file sftp_open(sftp_session sftp, const char *file, int flags, sftp_flags |= SSH_FXF_APPEND; } SSH_LOG(SSH_LOG_PACKET,"Opening file %s with sftp flags %x",file,sftp_flags); + rc = ssh_buffer_allocate_size(buffer, + sizeof(uint32_t) * 4 + + ssh_string_len(filename)); + if (rc < 0) { + ssh_set_error_oom(sftp->session); + ssh_buffer_free(buffer); + ssh_string_free(filename); + return NULL; + } id = sftp_get_new_id(sftp); if (ssh_buffer_add_u32(buffer, htonl(id)) < 0 || ssh_buffer_add_ssh_string(buffer, filename) < 0) { @@ -2215,6 +2263,7 @@ int sftp_mkdir(sftp_session sftp, const char *directory, mode_t mode) { ssh_buffer buffer; ssh_string path; uint32_t id; + int rc; buffer = ssh_buffer_new(); if (buffer == NULL) { @@ -2233,6 +2282,16 @@ int sftp_mkdir(sftp_session sftp, const char *directory, mode_t mode) { attr.permissions = mode; attr.flags = SSH_FILEXFER_ATTR_PERMISSIONS; + rc = ssh_buffer_allocate_size(buffer, + sizeof(uint32_t) * 2 + + ssh_string_len(path)); + if (rc < 0) { + ssh_set_error_oom(sftp->session); + ssh_buffer_free(buffer); + ssh_string_free(path); + return -1; + } + id = sftp_get_new_id(sftp); if (ssh_buffer_add_u32(buffer, htonl(id)) < 0 || ssh_buffer_add_ssh_string(buffer, path) < 0 || @@ -2385,6 +2444,7 @@ int sftp_setstat(sftp_session sftp, const char *file, sftp_attributes attr) { ssh_string path; sftp_message msg = NULL; sftp_status_message status = NULL; + int rc; buffer = ssh_buffer_new(); if (buffer == NULL) { @@ -2399,6 +2459,16 @@ int sftp_setstat(sftp_session sftp, const char *file, sftp_attributes attr) { return -1; } + rc = ssh_buffer_allocate_size(buffer, + sizeof(uint32_t) * 2 + + ssh_string_len(path)); + if (rc < 0) { + ssh_set_error_oom(sftp->session); + ssh_buffer_free(buffer); + ssh_string_free(path); + return -1; + } + id = sftp_get_new_id(sftp); if (ssh_buffer_add_u32(buffer, htonl(id)) < 0 || ssh_buffer_add_ssh_string(buffer, path) < 0 || @@ -2592,6 +2662,7 @@ char *sftp_readlink(sftp_session sftp, const char *path) { char *lnk; uint32_t ignored; uint32_t id; + int rc; if (sftp == NULL) return NULL; @@ -2616,6 +2687,16 @@ char *sftp_readlink(sftp_session sftp, const char *path) { return NULL; } + rc = ssh_buffer_allocate_size(buffer, + sizeof(uint32_t) * 2 + + ssh_string_len(path_s)); + if (rc < 0) { + ssh_set_error_oom(sftp->session); + ssh_buffer_free(buffer); + ssh_string_free(path_s); + return NULL; + } + id = sftp_get_new_id(sftp); if (ssh_buffer_add_u32(buffer, htonl(id)) < 0 || ssh_buffer_add_ssh_string(buffer, path_s) < 0) { @@ -2710,6 +2791,7 @@ sftp_statvfs_t sftp_statvfs(sftp_session sftp, const char *path) { ssh_string ext; ssh_buffer buffer; uint32_t id; + int rc; if (sftp == NULL) return NULL; @@ -2743,6 +2825,18 @@ sftp_statvfs_t sftp_statvfs(sftp_session sftp, const char *path) { return NULL; } + rc = ssh_buffer_allocate_size(buffer, + sizeof(uint32_t) * 3 + + ssh_string_len(ext) + + ssh_string_len(pathstr)); + if (rc < 0) { + ssh_set_error_oom(sftp->session); + ssh_buffer_free(buffer); + ssh_string_free(ext); + ssh_string_free(pathstr); + return NULL; + } + id = sftp_get_new_id(sftp); if (ssh_buffer_add_u32(buffer, htonl(id)) < 0 || ssh_buffer_add_ssh_string(buffer, ext) < 0 || @@ -2823,6 +2917,15 @@ int sftp_fsync(sftp_file file) goto done; } + rc = ssh_buffer_allocate_size(buffer, + sizeof(uint32_t) * 3 + + ssh_string_len(ext) + + ssh_string_len(file->handle)); + if (rc < 0) { + ssh_set_error_oom(sftp->session); + goto done; + } + id = sftp_get_new_id(sftp); rc = ssh_buffer_add_u32(buffer, htonl(id)); if (rc < 0) { @@ -2915,6 +3018,7 @@ sftp_statvfs_t sftp_fstatvfs(sftp_file file) { ssh_string ext; ssh_buffer buffer; uint32_t id; + int rc; if (file == NULL) { return NULL; @@ -2934,6 +3038,17 @@ sftp_statvfs_t sftp_fstatvfs(sftp_file file) { return NULL; } + rc = ssh_buffer_allocate_size(buffer, + sizeof(uint32_t) * 3 + + ssh_string_len(ext) + + ssh_string_len(file->handle)); + if (rc < 0) { + ssh_set_error_oom(sftp->session); + ssh_buffer_free(buffer); + ssh_string_free(ext); + return NULL; + } + id = sftp_get_new_id(sftp); if (ssh_buffer_add_u32(buffer, htonl(id)) < 0 || ssh_buffer_add_ssh_string(buffer, ext) < 0 || @@ -3002,6 +3117,7 @@ char *sftp_canonicalize_path(sftp_session sftp, const char *path) { char *cname; uint32_t ignored; uint32_t id; + int rc; if (sftp == NULL) return NULL; @@ -3023,6 +3139,16 @@ char *sftp_canonicalize_path(sftp_session sftp, const char *path) { return NULL; } + rc = ssh_buffer_allocate_size(buffer, + sizeof(uint32_t) * 2 + + ssh_string_len(pathstr)); + if (rc < 0) { + ssh_set_error_oom(sftp->session); + ssh_buffer_free(buffer); + ssh_string_free(pathstr); + return NULL; + } + id = sftp_get_new_id(sftp); if (ssh_buffer_add_u32(buffer, htonl(id)) < 0 || ssh_buffer_add_ssh_string(buffer, pathstr) < 0) { @@ -3087,6 +3213,7 @@ static sftp_attributes sftp_xstat(sftp_session sftp, const char *path, ssh_string pathstr; ssh_buffer buffer; uint32_t id; + int rc; if (path == NULL) { ssh_set_error_invalid(sftp->session); @@ -3106,6 +3233,16 @@ static sftp_attributes sftp_xstat(sftp_session sftp, const char *path, return NULL; } + rc = ssh_buffer_allocate_size(buffer, + sizeof(uint32_t) * 2 + + ssh_string_len(pathstr)); + if (rc < 0) { + ssh_set_error_oom(sftp->session); + ssh_buffer_free(buffer); + ssh_string_free(pathstr); + return NULL; + } + id = sftp_get_new_id(sftp); if (ssh_buffer_add_u32(buffer, htonl(id)) < 0 || ssh_buffer_add_ssh_string(buffer, pathstr) < 0) { @@ -3166,6 +3303,7 @@ sftp_attributes sftp_fstat(sftp_file file) { sftp_message msg = NULL; ssh_buffer buffer; uint32_t id; + int rc; buffer = ssh_buffer_new(); if (buffer == NULL) { @@ -3173,6 +3311,15 @@ sftp_attributes sftp_fstat(sftp_file file) { return NULL; } + rc = ssh_buffer_allocate_size(buffer, + sizeof(uint32_t) * 2 + + ssh_string_len(file->handle)); + if (rc < 0) { + ssh_set_error_oom(file->sftp->session); + ssh_buffer_free(buffer); + return NULL; + } + id = sftp_get_new_id(file->sftp); if (ssh_buffer_add_u32(buffer, htonl(id)) < 0 || ssh_buffer_add_ssh_string(buffer, file->handle) < 0) {