From d39e9ccc5e08a6441075049f1f79ffcb2bbe5a52 Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Fri, 1 Oct 2021 20:09:03 +0000 Subject: [PATCH] windows: fix clang and WinCNG warnings Fix these categories of warning: - in `wincng.c` disagreement in signed/unsigned char when passing around the passphrase string: `warning: pointer targets in passing argument [...] differ in signedness [-Wpointer-sign]` Fixed by using `const unsigned char *` in all static functions and applying/updating casts as necessary. - in each use of `libssh2_*_init()` macros where the result is not used: `warning: value computed is not used [-Wunused-value]` Fixed by using `(void)` casts. - `channel.c:1171:7: warning: 'rc' may be used uninitialized in this function [-Wmaybe-uninitialized]` Fixed by initializing this variable with `LIBSSH2_ERROR_CHANNEL_UNKNOWN`. While there I replaced a few 0 literals with `LIBSSH2_ERROR_NONE`. - in `sftp.c`, several of these two warnings: `warning: 'data' may be used uninitialized in this function [-Wmaybe-uninitialized]` `warning: 'data_len' may be used uninitialized in this function [-Wmaybe-uninitialized]` Fixed by initializing these variables with NULL and 0 respectively. - Also removed the exec attribute from `wincng.h`. Notes: - There are many pre-existing checksrc issues. - The `sftp.c` and `channel.c` warnings may apply to other platforms as well. Closes #628 --- src/bcrypt_pbkdf.c | 6 +++--- src/channel.c | 10 ++++++--- src/hostkey.c | 6 +++--- src/kex.c | 12 +++++------ src/sftp.c | 52 +++++++++++++++++++++++----------------------- src/wincng.c | 27 ++++++++++++------------ src/wincng.h | 0 7 files changed, 59 insertions(+), 54 deletions(-) mode change 100755 => 100644 src/wincng.h diff --git a/src/bcrypt_pbkdf.c b/src/bcrypt_pbkdf.c index f782bcac..50d54209 100644 --- a/src/bcrypt_pbkdf.c +++ b/src/bcrypt_pbkdf.c @@ -127,7 +127,7 @@ bcrypt_pbkdf(const char *pass, size_t passlen, const uint8_t *salt, memcpy(countsalt, salt, saltlen); /* collapse password */ - libssh2_sha512_init(&ctx); + (void)libssh2_sha512_init(&ctx); libssh2_sha512_update(ctx, pass, passlen); libssh2_sha512_final(ctx, sha2pass); @@ -139,7 +139,7 @@ bcrypt_pbkdf(const char *pass, size_t passlen, const uint8_t *salt, countsalt[saltlen + 3] = count & 0xff; /* first round, salt is salt */ - libssh2_sha512_init(&ctx); + (void)libssh2_sha512_init(&ctx); libssh2_sha512_update(ctx, countsalt, saltlen + 4); libssh2_sha512_final(ctx, sha2salt); @@ -148,7 +148,7 @@ bcrypt_pbkdf(const char *pass, size_t passlen, const uint8_t *salt, for(i = 1; i < rounds; i++) { /* subsequent rounds, salt is previous output */ - libssh2_sha512_init(&ctx); + (void)libssh2_sha512_init(&ctx); libssh2_sha512_update(ctx, tmpout, sizeof(tmpout)); libssh2_sha512_final(ctx, sha2salt); diff --git a/src/channel.c b/src/channel.c index 78ed40e8..59133f8a 100644 --- a/src/channel.c +++ b/src/channel.c @@ -1140,6 +1140,8 @@ libssh2_channel_request_auth_agent(LIBSSH2_CHANNEL *channel) if(!channel) return LIBSSH2_ERROR_BAD_USE; + rc = LIBSSH2_ERROR_CHANNEL_UNKNOWN; + /* The current RFC draft for agent forwarding says you're supposed to * send "auth-agent-req," but most SSH servers out there right now * actually expect "auth-agent-req@openssh.com", so we try that @@ -1152,7 +1154,8 @@ libssh2_channel_request_auth_agent(LIBSSH2_CHANNEL *channel) /* If we failed (but not with EAGAIN), then we move onto * the next step to try another request type. */ - if(rc != 0 && rc != LIBSSH2_ERROR_EAGAIN) + if(rc != LIBSSH2_ERROR_NONE && + rc != LIBSSH2_ERROR_EAGAIN) channel->req_auth_agent_try_state = libssh2_NB_state_sent; } @@ -1163,12 +1166,13 @@ libssh2_channel_request_auth_agent(LIBSSH2_CHANNEL *channel) /* If we failed without an EAGAIN, then move on with this * state machine. */ - if(rc != 0 && rc != LIBSSH2_ERROR_EAGAIN) + if(rc != LIBSSH2_ERROR_NONE && + rc != LIBSSH2_ERROR_EAGAIN) channel->req_auth_agent_try_state = libssh2_NB_state_sent1; } /* If things are good, reset the try state. */ - if(rc == 0) + if(rc == LIBSSH2_ERROR_NONE) channel->req_auth_agent_try_state = libssh2_NB_state_idle; return rc; diff --git a/src/hostkey.c b/src/hostkey.c index d87a4c74..d126b611 100644 --- a/src/hostkey.c +++ b/src/hostkey.c @@ -211,7 +211,7 @@ hostkey_method_ssh_rsa_signv(LIBSSH2_SESSION * session, unsigned char hash[SHA_DIGEST_LENGTH]; libssh2_sha1_ctx ctx; - libssh2_sha1_init(&ctx); + (void)libssh2_sha1_init(&ctx); for(i = 0; i < veccount; i++) { libssh2_sha1_update(ctx, datavec[i].iov_base, datavec[i].iov_len); } @@ -438,7 +438,7 @@ hostkey_method_ssh_dss_signv(LIBSSH2_SESSION * session, *signature_len = 2 * SHA_DIGEST_LENGTH; - libssh2_sha1_init(&ctx); + (void)libssh2_sha1_init(&ctx); for(i = 0; i < veccount; i++) { libssh2_sha1_update(ctx, datavec[i].iov_base, datavec[i].iov_len); } @@ -683,7 +683,7 @@ hostkey_method_ssh_ecdsa_sig_verify(LIBSSH2_SESSION * session, unsigned char hash[SHA##digest_type##_DIGEST_LENGTH]; \ libssh2_sha##digest_type##_ctx ctx; \ int i; \ - libssh2_sha##digest_type##_init(&ctx); \ + (void)libssh2_sha##digest_type##_init(&ctx); \ for(i = 0; i < veccount; i++) { \ libssh2_sha##digest_type##_update(ctx, datavec[i].iov_base, \ datavec[i].iov_len); \ diff --git a/src/kex.c b/src/kex.c index 9f3ef799..c300ecb7 100644 --- a/src/kex.c +++ b/src/kex.c @@ -78,7 +78,7 @@ } \ if(value) \ while(len < (unsigned long)reqlen) { \ - libssh2_sha##digest_type##_init(&hash); \ + (void)libssh2_sha##digest_type##_init(&hash); \ libssh2_sha##digest_type##_update(hash, \ exchange_state->k_value, \ exchange_state->k_value_len); \ @@ -108,16 +108,16 @@ static void _libssh2_sha_algo_ctx_init(int sha_algo, void *ctx) { if(sha_algo == 512) { - libssh2_sha512_init((libssh2_sha512_ctx*)ctx); + (void)libssh2_sha512_init((libssh2_sha512_ctx*)ctx); } else if(sha_algo == 384) { - libssh2_sha384_init((libssh2_sha384_ctx*)ctx); + (void)libssh2_sha384_init((libssh2_sha384_ctx*)ctx); } else if(sha_algo == 256) { - libssh2_sha256_init((libssh2_sha256_ctx*)ctx); + (void)libssh2_sha256_init((libssh2_sha256_ctx*)ctx); } else if(sha_algo == 1) { - libssh2_sha1_init((libssh2_sha1_ctx*)ctx); + (void)libssh2_sha1_init((libssh2_sha1_ctx*)ctx); } else { assert(0); @@ -1600,7 +1600,7 @@ kex_method_diffie_hellman_group_exchange_sha256_key_exchange { \ libssh2_sha##digest_type##_ctx ctx; \ exchange_state->exchange_hash = (void *)&ctx; \ - libssh2_sha##digest_type##_init(&ctx); \ + (void)libssh2_sha##digest_type##_init(&ctx); \ if(session->local.banner) { \ _libssh2_htonu32(exchange_state->h_sig_comp, \ strlen((char *) session->local.banner) - 2); \ diff --git a/src/sftp.c b/src/sftp.c index ac7ee016..b1a53527 100644 --- a/src/sftp.c +++ b/src/sftp.c @@ -765,7 +765,7 @@ LIBSSH2_CHANNEL_CLOSE_FUNC(libssh2_sftp_dtor) static LIBSSH2_SFTP *sftp_init(LIBSSH2_SESSION *session) { unsigned char *data; - size_t data_len; + size_t data_len = 0; ssize_t rc; LIBSSH2_SFTP *sftp_handle; struct string_buf buf; @@ -1561,7 +1561,7 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer, while(chunk) { unsigned char *data; - size_t data_len; + size_t data_len = 0; uint32_t rc32; static const unsigned char read_responses[2] = { SSH_FXP_DATA, SSH_FXP_STATUS @@ -1751,7 +1751,7 @@ static ssize_t sftp_readdir(LIBSSH2_SFTP_HANDLE *handle, char *buffer, LIBSSH2_SFTP *sftp = handle->sftp; LIBSSH2_CHANNEL *channel = sftp->channel; LIBSSH2_SESSION *session = channel->session; - size_t data_len; + size_t data_len = 0; uint32_t num_names; /* 13 = packet_len(4) + packet_type(1) + request_id(4) + handle_len(4) */ uint32_t packet_len = handle->handle_len + 13; @@ -2017,10 +2017,10 @@ static ssize_t sftp_write(LIBSSH2_SFTP_HANDLE *handle, const char *buffer, LIBSSH2_SFTP *sftp = handle->sftp; LIBSSH2_CHANNEL *channel = sftp->channel; LIBSSH2_SESSION *session = channel->session; - size_t data_len; + size_t data_len = 0; uint32_t retcode; uint32_t packet_len; - unsigned char *s, *data; + unsigned char *s, *data = NULL; ssize_t rc; struct sftp_pipeline_chunk *chunk; struct sftp_pipeline_chunk *next; @@ -2245,8 +2245,8 @@ static int sftp_fsync(LIBSSH2_SFTP_HANDLE *handle) /* 34 = packet_len(4) + packet_type(1) + request_id(4) + string_len(4) + strlen("fsync@openssh.com")(17) + handle_len(4) */ uint32_t packet_len = handle->handle_len + 34; - size_t data_len; - unsigned char *packet, *s, *data; + size_t data_len = 0; + unsigned char *packet, *s, *data = NULL; ssize_t rc; uint32_t retcode; @@ -2350,11 +2350,11 @@ static int sftp_fstat(LIBSSH2_SFTP_HANDLE *handle, LIBSSH2_SFTP *sftp = handle->sftp; LIBSSH2_CHANNEL *channel = sftp->channel; LIBSSH2_SESSION *session = channel->session; - size_t data_len; + size_t data_len = 0; /* 13 = packet_len(4) + packet_type(1) + request_id(4) + handle_len(4) */ uint32_t packet_len = handle->handle_len + 13 + (setstat ? sftp_attrsize(attrs->flags) : 0); - unsigned char *s, *data; + unsigned char *s, *data = NULL; static const unsigned char fstat_responses[2] = { SSH_FXP_ATTRS, SSH_FXP_STATUS }; ssize_t rc; @@ -2575,7 +2575,7 @@ sftp_close_handle(LIBSSH2_SFTP_HANDLE *handle) LIBSSH2_SFTP *sftp = handle->sftp; LIBSSH2_CHANNEL *channel = sftp->channel; LIBSSH2_SESSION *session = channel->session; - size_t data_len; + size_t data_len = 0; /* 13 = packet_len(4) + packet_type(1) + request_id(4) + handle_len(4) */ uint32_t packet_len = handle->handle_len + 13; unsigned char *s, *data = NULL; @@ -2705,11 +2705,11 @@ static int sftp_unlink(LIBSSH2_SFTP *sftp, const char *filename, { LIBSSH2_CHANNEL *channel = sftp->channel; LIBSSH2_SESSION *session = channel->session; - size_t data_len; + size_t data_len = 0; int retcode; /* 13 = packet_len(4) + packet_type(1) + request_id(4) + filename_len(4) */ uint32_t packet_len = filename_len + 13; - unsigned char *s, *data; + unsigned char *s, *data = NULL; int rc; if(sftp->unlink_state == libssh2_NB_state_idle) { @@ -2809,14 +2809,14 @@ static int sftp_rename(LIBSSH2_SFTP *sftp, const char *source_filename, { LIBSSH2_CHANNEL *channel = sftp->channel; LIBSSH2_SESSION *session = channel->session; - size_t data_len; + size_t data_len = 0; int retcode; uint32_t packet_len = source_filename_len + dest_filename_len + 17 + (sftp->version >= 5 ? 4 : 0); /* packet_len(4) + packet_type(1) + request_id(4) + source_filename_len(4) + dest_filename_len(4) + flags(4){SFTP5+) */ - unsigned char *data; + unsigned char *data = NULL; ssize_t rc; if(sftp->version < 2) { @@ -2949,12 +2949,12 @@ static int sftp_fstatvfs(LIBSSH2_SFTP_HANDLE *handle, LIBSSH2_SFTP_STATVFS *st) LIBSSH2_SFTP *sftp = handle->sftp; LIBSSH2_CHANNEL *channel = sftp->channel; LIBSSH2_SESSION *session = channel->session; - size_t data_len; + size_t data_len = 0; /* 17 = packet_len(4) + packet_type(1) + request_id(4) + ext_len(4) + handle_len (4) */ /* 20 = strlen ("fstatvfs@openssh.com") */ uint32_t packet_len = handle->handle_len + 20 + 17; - unsigned char *packet, *s, *data; + unsigned char *packet, *s, *data = NULL; ssize_t rc; unsigned int flag; static const unsigned char responses[2] = @@ -3085,12 +3085,12 @@ static int sftp_statvfs(LIBSSH2_SFTP *sftp, const char *path, { LIBSSH2_CHANNEL *channel = sftp->channel; LIBSSH2_SESSION *session = channel->session; - size_t data_len; + size_t data_len = 0; /* 17 = packet_len(4) + packet_type(1) + request_id(4) + ext_len(4) + path_len (4) */ /* 19 = strlen ("statvfs@openssh.com") */ uint32_t packet_len = path_len + 19 + 17; - unsigned char *packet, *s, *data; + unsigned char *packet, *s, *data = NULL; ssize_t rc; unsigned int flag; static const unsigned char responses[2] = @@ -3225,10 +3225,10 @@ static int sftp_mkdir(LIBSSH2_SFTP *sftp, const char *path, LIBSSH2_SFTP_ATTRIBUTES attrs = { 0, 0, 0, 0, 0, 0, 0 }; - size_t data_len; + size_t data_len = 0; int retcode; ssize_t packet_len; - unsigned char *packet, *s, *data; + unsigned char *packet, *s, *data = NULL; int rc; if(mode != LIBSSH2_SFTP_DEFAULT_MODE) { @@ -3340,11 +3340,11 @@ static int sftp_rmdir(LIBSSH2_SFTP *sftp, const char *path, { LIBSSH2_CHANNEL *channel = sftp->channel; LIBSSH2_SESSION *session = channel->session; - size_t data_len; + size_t data_len = 0; int retcode; /* 13 = packet_len(4) + packet_type(1) + request_id(4) + path_len(4) */ ssize_t packet_len = path_len + 13; - unsigned char *s, *data; + unsigned char *s, *data = NULL; int rc; if(sftp->rmdir_state == libssh2_NB_state_idle) { @@ -3442,13 +3442,13 @@ static int sftp_stat(LIBSSH2_SFTP *sftp, const char *path, { LIBSSH2_CHANNEL *channel = sftp->channel; LIBSSH2_SESSION *session = channel->session; - size_t data_len; + size_t data_len = 0; /* 13 = packet_len(4) + packet_type(1) + request_id(4) + path_len(4) */ ssize_t packet_len = path_len + 13 + ((stat_type == LIBSSH2_SFTP_SETSTAT) ? sftp_attrsize(attrs->flags) : 0); - unsigned char *s, *data; + unsigned char *s, *data = NULL; static const unsigned char stat_responses[2] = { SSH_FXP_ATTRS, SSH_FXP_STATUS }; int rc; @@ -3580,12 +3580,12 @@ static int sftp_symlink(LIBSSH2_SFTP *sftp, const char *path, { LIBSSH2_CHANNEL *channel = sftp->channel; LIBSSH2_SESSION *session = channel->session; - size_t data_len, link_len; + size_t data_len = 0, link_len; /* 13 = packet_len(4) + packet_type(1) + request_id(4) + path_len(4) */ ssize_t packet_len = path_len + 13 + ((link_type == LIBSSH2_SFTP_SYMLINK) ? (4 + target_len) : 0); - unsigned char *s, *data; + unsigned char *s, *data = NULL; static const unsigned char link_responses[2] = { SSH_FXP_NAME, SSH_FXP_STATUS }; int retcode; diff --git a/src/wincng.c b/src/wincng.c index cbb2b61c..9ae8ddee 100644 --- a/src/wincng.c +++ b/src/wincng.c @@ -664,7 +664,7 @@ _libssh2_wincng_key_sha1_verify(_libssh2_wincng_key_ctx *ctx, static int _libssh2_wincng_load_pem(LIBSSH2_SESSION *session, const char *filename, - const char *passphrase, + const unsigned char *passphrase, const char *headerbegin, const char *headerend, unsigned char **data, @@ -690,7 +690,7 @@ _libssh2_wincng_load_pem(LIBSSH2_SESSION *session, static int _libssh2_wincng_load_private(LIBSSH2_SESSION *session, const char *filename, - const char *passphrase, + const unsigned char *passphrase, unsigned char **ppbEncoded, unsigned long *pcbEncoded, int tryLoadRSA, int tryLoadDSA) @@ -723,7 +723,7 @@ static int _libssh2_wincng_load_private_memory(LIBSSH2_SESSION *session, const char *privatekeydata, size_t privatekeydata_len, - const char *passphrase, + const unsigned char *passphrase, unsigned char **ppbEncoded, unsigned long *pcbEncoded, int tryLoadRSA, int tryLoadDSA) @@ -1148,8 +1148,7 @@ _libssh2_wincng_rsa_new_private(libssh2_rsa_ctx **rsa, (void)session; - ret = _libssh2_wincng_load_private(session, filename, - (const char *)passphrase, + ret = _libssh2_wincng_load_private(session, filename, passphrase, &pbEncoded, &cbEncoded, 1, 0); if(ret) { return -1; @@ -1173,7 +1172,7 @@ _libssh2_wincng_rsa_new_private_frommemory(libssh2_rsa_ctx **rsa, LIBSSH2_SESSION *session, const char *filedata, size_t filedata_len, - unsigned const char *passphrase) + const unsigned char *passphrase) { #ifdef HAVE_LIBCRYPT32 unsigned char *pbEncoded; @@ -1183,7 +1182,7 @@ _libssh2_wincng_rsa_new_private_frommemory(libssh2_rsa_ctx **rsa, (void)session; ret = _libssh2_wincng_load_private_memory(session, filedata, filedata_len, - (const char *)passphrase, + passphrase, &pbEncoded, &cbEncoded, 1, 0); if(ret) { return -1; @@ -1447,8 +1446,7 @@ _libssh2_wincng_dsa_new_private(libssh2_dsa_ctx **dsa, unsigned long cbEncoded; int ret; - ret = _libssh2_wincng_load_private(session, filename, - (const char *)passphrase, + ret = _libssh2_wincng_load_private(session, filename, passphrase, &pbEncoded, &cbEncoded, 0, 1); if(ret) { return -1; @@ -1472,7 +1470,7 @@ _libssh2_wincng_dsa_new_private_frommemory(libssh2_dsa_ctx **dsa, LIBSSH2_SESSION *session, const char *filedata, size_t filedata_len, - unsigned const char *passphrase) + const unsigned char *passphrase) { #ifdef HAVE_LIBCRYPT32 unsigned char *pbEncoded; @@ -1480,7 +1478,7 @@ _libssh2_wincng_dsa_new_private_frommemory(libssh2_dsa_ctx **dsa, int ret; ret = _libssh2_wincng_load_private_memory(session, filedata, filedata_len, - (const char *)passphrase, + passphrase, &pbEncoded, &cbEncoded, 0, 1); if(ret) { return -1; @@ -1728,7 +1726,8 @@ _libssh2_wincng_pub_priv_keyfile(LIBSSH2_SESSION *session, unsigned long cbEncoded; int ret; - ret = _libssh2_wincng_load_private(session, privatekey, passphrase, + ret = _libssh2_wincng_load_private(session, privatekey, + (const unsigned char *)passphrase, &pbEncoded, &cbEncoded, 1, 1); if(ret) { return -1; @@ -1767,7 +1766,9 @@ _libssh2_wincng_pub_priv_keyfilememory(LIBSSH2_SESSION *session, int ret; ret = _libssh2_wincng_load_private_memory(session, privatekeydata, - privatekeydata_len, passphrase, + privatekeydata_len, + (const unsigned char *) + passphrase, &pbEncoded, &cbEncoded, 1, 1); if(ret) { return -1; diff --git a/src/wincng.h b/src/wincng.h old mode 100755 new mode 100644