lib/libssh2
1
0
Fork 0
mirror of https://github.com/libssh2/libssh2.git synced 2025-07-28 01:41:49 +03:00
Code Activity

clang-tidy: fix and/or silence issues found, and more

Browse Source 
- kex: drop unused assigment.
- knownhost: error when salt is NULL.
- mbedtls: avoid unnecessary inline assigments, that were ignored for
  the second block and replaceable with a `ret = 0` initialization for
  the first one.
- mbedtls: fix ignoring an API failure and ending up calling
  `mbedtls_rsa_check_privkey()` unconditionally.
- misc: initialize datalen on error in `_libssh2_base64_decode()`.
- openssl: drop unused assigments.
- openssl: fix unused static function.
- packet: avoid NULL deref.
- packet: avoid NULL in `memcpy` src.
- publickey: optimize struct layout to avoid padding.
- sftp: replace ignored `rc` error assigment with `_libssh2_error()` call.
- transport: fix potential NULL ptr dereferences.
- transport: silence uninitialized value warnings.
- userauth: drop unused assigment.
- userauth: possible use of unitialized pointer.
- userauth: replace `rewind()` with `fseek()`.
  `rewind()` returns an error condition in `errno`. `errno` is
  problematic and reduces portability. Use `fseek()` to avoid it.
- userauth: replace potential NULL deref by returning error from
  `sign_frommemory()`. Possible false positive. `rc` should be set
  upstream if the callback is NULL.
- userauth: replace potential NULL deref by returning error from
  `sign_fromfile()`. clang-tidy did not warn about this one, but
  let's match `sign_frommemory()` anyway.
- wincng: fix potentially unused macros.
- wincng: make sure bignum is not NULL before use.

tests:
- openssh_fixture: drop unused assignment.
- session_fixture: exit if `username` not set, to avoid `strlen(NULL)`.
- session_fixture: replace `rewind()` with `fseek()`.
  `rewind()` returns an error condition in `errno`. `errno` is
  problematic and reduces portability. Use `fseek()` to avoid it.
- test_read: exit if `username` not set, to avoid `strlen(NULL)`.

examples:
- scp_write_nonblock: fix file handle leak.
- sftp_write_nonblock: file handle leak on error.
- sftp_write_sliding: file handle leak on error.
- ssh2_agent_forwarding: fix unused error codes.

Details in the subcommits under the PR.

Thanks-to: Michael Buckley
Thanks-to: Will Cosgrove

Closes #1561
This commit is contained in:
Viktor Szakats
2025-03-19 12:45:34 +01:00
parent 15752e5f0b
commit a1a28ac943
18 changed files with 104 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
example/scp_write_nonblock.c
View File

@ -274,6 +274,8 @@ int main(int argc, char *argv[])
shutdown:
fclose(local);
if(session) {
while(libssh2_session_disconnect(session, "Normal Shutdown") ==
LIBSSH2_ERROR_EAGAIN);

3
example/sftp_write_nonblock.c
View File

@ -269,12 +269,13 @@ int main(int argc, char *argv[])
fprintf(stderr, "%ld bytes in %d seconds makes %.1f bytes/sec\n",
(long)total, duration, (double)total / duration);
fclose(local);
libssh2_sftp_close(sftp_handle);
libssh2_sftp_shutdown(sftp_session);
shutdown:
fclose(local);
if(session) {
while(libssh2_session_disconnect(session, "Normal Shutdown") ==
LIBSSH2_ERROR_EAGAIN);

3
example/sftp_write_sliding.c
View File

@ -280,12 +280,13 @@ int main(int argc, char *argv[])
fprintf(stderr, "%ld bytes in %d seconds makes %.1f bytes/sec\n",
(long)total, duration, (double)total / duration);
fclose(local);
libssh2_sftp_close(sftp_handle);
libssh2_sftp_shutdown(sftp_session);
shutdown:
fclose(local);
if(session) {
while(libssh2_session_disconnect(session, "Normal Shutdown") ==
LIBSSH2_ERROR_EAGAIN);

4
example/ssh2_agent_forwarding.c
View File

@ -271,6 +271,8 @@ int main(int argc, char *argv[])
NULL, NULL, NULL, NULL, NULL);
}
rc = 0;
if(exitsignal) {
fprintf(stderr, "\nGot signal: %s\n",
exitsignal ? exitsignal : "none");
@ -303,5 +305,5 @@ shutdown:
WSACleanup();
#endif
return 0;
return rc;
}

2
src/kex.c
View File

@ -3481,7 +3481,7 @@ static int kexinit(LIBSSH2_SESSION * session)
p += lang_cs_len + 4;
_libssh2_debug((session, LIBSSH2_TRACE_KEX,
"Sent LANG_SC: %s", p));
p += lang_sc_len + 4;
/* p += lang_sc_len + 4; */
}
#endif /* LIBSSH2DEBUG */

7
src/knownhost.c
View File

@ -175,6 +175,13 @@ knownhost_add(LIBSSH2_KNOWNHOSTS *hosts,
host, hostlen);
if(rc)
goto error;
if(!salt) {
rc = _libssh2_error(hosts->session, LIBSSH2_ERROR_INVAL,
"Salt is NULL");
goto error;
}
entry->name = ptr;
entry->name_len = ptrlen;

39
src/mbedtls.c
View File

@ -413,11 +413,11 @@ _libssh2_mbedtls_rsa_new(libssh2_rsa_ctx **rsa,
else
return -1;
/* !checksrc! disable ASSIGNWITHINCONDITION 1 */
if((ret = mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(E)),
edata, elen)) ||
(ret = mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(N)),
ndata, nlen))) {
ret = 0;
if(mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(E)),
edata, elen) ||
mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(N)),
ndata, nlen)) {
ret = -1;
}
@ -427,22 +427,23 @@ _libssh2_mbedtls_rsa_new(libssh2_rsa_ctx **rsa,
}
if(!ret && ddata) {
/* !checksrc! disable ASSIGNWITHINCONDITION 1 */
if((ret = mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(D)),
ddata, dlen)) ||
(ret = mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(P)),
pdata, plen)) ||
(ret = mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(Q)),
qdata, qlen)) ||
(ret = mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(DP)),
e1data, e1len)) ||
(ret = mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(DQ)),
e2data, e2len)) ||
(ret = mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(QP)),
coeffdata, coefflen))) {
if(mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(D)),
ddata, dlen) ||
mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(P)),
pdata, plen) ||
mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(Q)),
qdata, qlen) ||
mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(DP)),
e1data, e1len) ||
mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(DQ)),
e2data, e2len) ||
mbedtls_mpi_read_binary(&(ctx->MBEDTLS_PRIVATE(QP)),
coeffdata, coefflen)) {
ret = -1;
}
ret = mbedtls_rsa_check_privkey(ctx);
else {
ret = mbedtls_rsa_check_privkey(ctx);
}
}
else if(!ret) {
ret = mbedtls_rsa_check_pubkey(ctx);

1
src/misc.c
View File

@ -391,6 +391,7 @@ int _libssh2_base64_decode(LIBSSH2_SESSION *session,
short v;
size_t i = 0, len = 0;
*datalen = 0;
*data = LIBSSH2_ALLOC(session, src_len);
d = (unsigned char *) *data;
if(!d) {

6
src/openssl.c
View File

@ -243,6 +243,7 @@ write_bn(unsigned char *buf, const BIGNUM *bn, int bn_bytes)
}
#endif
#ifdef USE_OPENSSL_3
static inline void
_libssh2_swap_bytes(unsigned char *buf, unsigned long len)
{
@ -256,6 +257,7 @@ _libssh2_swap_bytes(unsigned char *buf, unsigned long len)
}
#endif
}
#endif
int
_libssh2_openssl_random(void *buf, size_t len)
@ -1095,7 +1097,6 @@ _libssh2_cipher_crypt(_libssh2_cipher_ctx * ctx,
if(ret >= 1)
#endif
{
rc = 0;
if(IS_LAST(firstlast)) {
/* This is the last block.
encrypt: compute tag, if applicable
@ -2276,7 +2277,6 @@ gen_publickey_from_ed25519_openssh_priv_data(LIBSSH2_SESSION *session,
tmp_len != LIBSSH2_ED25519_PRIVATE_KEY_LEN) {
_libssh2_error(session, LIBSSH2_ERROR_PROTO,
"Wrong private key length");
ret = -1;
goto clean_exit;
}
@ -2290,7 +2290,6 @@ gen_publickey_from_ed25519_openssh_priv_data(LIBSSH2_SESSION *session,
if(_libssh2_get_string(decrypted, &buf, &tmp_len)) {
_libssh2_error(session, LIBSSH2_ERROR_PROTO,
"Unable to read comment");
ret = -1;
goto clean_exit;
}
@ -2313,7 +2312,6 @@ gen_publickey_from_ed25519_openssh_priv_data(LIBSSH2_SESSION *session,
if(*decrypted->dataptr != i) {
_libssh2_error(session, LIBSSH2_ERROR_PROTO,
"Wrong padding");
ret = -1;
goto clean_exit;
}
i++;

4
src/packet.c
View File

@ -879,7 +879,7 @@ _libssh2_packet_add(LIBSSH2_SESSION * session, unsigned char *data,
(int)value_len, value));
}
if(name_len == 15 &&
if(name && name_len == 15 &&
memcmp(name, "server-sig-algs", 15) == 0) {
if(session->server_sign_algorithms) {
LIBSSH2_FREE(session,
@ -890,7 +890,7 @@ _libssh2_packet_add(LIBSSH2_SESSION * session, unsigned char *data,
LIBSSH2_ALLOC(session,
value_len + 1);
if(session->server_sign_algorithms) {
if(value && session->server_sign_algorithms) {
memcpy(session->server_sign_algorithms,
value, value_len);
session->server_sign_algorithms[value_len] = '\0';

43
src/publickey.c
View File

@ -52,18 +52,19 @@
typedef struct _LIBSSH2_PUBLICKEY_CODE_LIST
{
int code;
const char *name;
int name_len;
int code;
} LIBSSH2_PUBLICKEY_CODE_LIST;
#define STRLEN(s) s, sizeof(s) - 1
static const LIBSSH2_PUBLICKEY_CODE_LIST publickey_response_codes[] =
{
{LIBSSH2_PUBLICKEY_RESPONSE_STATUS, "status", sizeof("status") - 1},
{LIBSSH2_PUBLICKEY_RESPONSE_VERSION, "version", sizeof("version") - 1},
{LIBSSH2_PUBLICKEY_RESPONSE_PUBLICKEY, "publickey",
sizeof("publickey") - 1},
{0, NULL, 0}
{STRLEN("status"), LIBSSH2_PUBLICKEY_RESPONSE_STATUS},
{STRLEN("version"), LIBSSH2_PUBLICKEY_RESPONSE_VERSION},
{STRLEN("publickey"), LIBSSH2_PUBLICKEY_RESPONSE_PUBLICKEY},
{NULL, 0, 0}
};
/* PUBLICKEY status codes -- IETF defined */
@ -80,26 +81,20 @@ static const LIBSSH2_PUBLICKEY_CODE_LIST publickey_response_codes[] =
#define LIBSSH2_PUBLICKEY_STATUS_CODE_MAX 8
static const LIBSSH2_PUBLICKEY_CODE_LIST publickey_status_codes[] = {
{LIBSSH2_PUBLICKEY_SUCCESS, "success", sizeof("success") - 1},
{LIBSSH2_PUBLICKEY_ACCESS_DENIED, "access denied",
sizeof("access denied") - 1},
{LIBSSH2_PUBLICKEY_STORAGE_EXCEEDED, "storage exceeded",
sizeof("storage exceeded") - 1},
{LIBSSH2_PUBLICKEY_VERSION_NOT_SUPPORTED, "version not supported",
sizeof("version not supported") - 1},
{LIBSSH2_PUBLICKEY_KEY_NOT_FOUND, "key not found",
sizeof("key not found") - 1},
{LIBSSH2_PUBLICKEY_KEY_NOT_SUPPORTED, "key not supported",
sizeof("key not supported") - 1},
{LIBSSH2_PUBLICKEY_KEY_ALREADY_PRESENT, "key already present",
sizeof("key already present") - 1},
{LIBSSH2_PUBLICKEY_GENERAL_FAILURE, "general failure",
sizeof("general failure") - 1},
{LIBSSH2_PUBLICKEY_REQUEST_NOT_SUPPORTED, "request not supported",
sizeof("request not supported") - 1},
{0, NULL, 0}
{STRLEN("success"), LIBSSH2_PUBLICKEY_SUCCESS},
{STRLEN("access denied"), LIBSSH2_PUBLICKEY_ACCESS_DENIED},
{STRLEN("storage exceeded"), LIBSSH2_PUBLICKEY_STORAGE_EXCEEDED},
{STRLEN("version not supported"), LIBSSH2_PUBLICKEY_VERSION_NOT_SUPPORTED},
{STRLEN("key not found"), LIBSSH2_PUBLICKEY_KEY_NOT_FOUND},
{STRLEN("key not supported"), LIBSSH2_PUBLICKEY_KEY_NOT_SUPPORTED},
{STRLEN("key already present"), LIBSSH2_PUBLICKEY_KEY_ALREADY_PRESENT},
{STRLEN("general failure"), LIBSSH2_PUBLICKEY_GENERAL_FAILURE},
{STRLEN("request not supported"), LIBSSH2_PUBLICKEY_REQUEST_NOT_SUPPORTED},
{NULL, 0, 0}
};
#undef STRLEN
/*
* publickey_status_error
*

3
src/sftp.c
View File

@ -954,7 +954,8 @@ static LIBSSH2_SFTP *sftp_init(LIBSSH2_SESSION *session)
if(_libssh2_get_u32(&buf, &(sftp_handle->version))) {
LIBSSH2_FREE(session, data);
rc = LIBSSH2_ERROR_BUFFER_TOO_SMALL;
_libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL,
"Data too short when extracting version");
goto sftp_init_error;
}

8
src/transport.c
View File

@ -241,6 +241,8 @@ fullpacket(LIBSSH2_SESSION * session, int encrypted /* 1 or 0 */ )
unsigned char *decrypt_buffer;
int blocksize = session->remote.crypt->blocksize;
first_block[0] = 0;
rc = decrypt(session, p->payload + 4,
first_block, blocksize, FIRST_BLOCK);
if(rc) {
@ -382,6 +384,8 @@ int _libssh2_transport_read(LIBSSH2_SESSION * session)
unsigned int auth_len = 0; /* length of the authentication tag */
const LIBSSH2_MAC_METHOD *remote_mac = NULL; /* The remote MAC, if used */
block[4] = 0;
/* default clear the bit */
session->socket_block_directions &= ~LIBSSH2_SESSION_BLOCK_INBOUND;
@ -624,7 +628,7 @@ int _libssh2_transport_read(LIBSSH2_SESSION * session)
/* total_num is the number of bytes following the initial
(5 bytes) packet length and padding length fields */
total_num = p->packet_length - 1 +
(encrypted ? remote_mac->mac_len : 0);
(encrypted && remote_mac ? remote_mac->mac_len : 0);
}
}
else {
@ -1238,7 +1242,7 @@ int _libssh2_transport_send(LIBSSH2_SESSION *session,
/* Call crypt() one last time so it can be filled in with the
MAC */
if(CRYPT_FLAG_L(session, INTEGRATED_MAC)) {
int authlen = local_mac->mac_len;
int authlen = local_mac ? local_mac->mac_len : 0;
assert((size_t)total_length <=
packet_length + session->local.crypt->blocksize);
if(session->local.crypt->crypt(session,

16
src/userauth.c
View File

@ -677,7 +677,7 @@ file_read_publickey(LIBSSH2_SESSION * session, unsigned char **method,
while(!feof(fd) && 1 == fread(&c, 1, 1, fd) && c != '\r' && c != '\n') {
pubkey_len++;
}
rewind(fd);
fseek(fd, 0L, SEEK_SET);
if(pubkey_len <= 1) {
fclose(fd);
@ -856,6 +856,9 @@ sign_frommemory(LIBSSH2_SESSION *session, unsigned char **sig, size_t *sig_len,
if(rc)
return rc;
if(!privkeyobj)
return -1;
libssh2_prepare_iovec(&datavec, 1);
datavec.iov_base = (void *)LIBSSH2_UNCONST(data);
datavec.iov_len = data_len;
@ -892,6 +895,9 @@ sign_fromfile(LIBSSH2_SESSION *session, unsigned char **sig, size_t *sig_len,
if(rc)
return rc;
if(!privkeyobj)
return -1;
libssh2_prepare_iovec(&datavec, 1);
datavec.iov_base = (void *)LIBSSH2_UNCONST(data);
datavec.iov_len = data_len;
@ -1738,7 +1744,7 @@ retry_auth:
if(session->userauth_pblc_state == libssh2_NB_state_sent1) {
unsigned char *buf;
unsigned char *sig;
unsigned char *sig = NULL;
size_t sig_len;
s = buf = LIBSSH2_ALLOC(session, 4 + session->session_id_len
@ -1770,7 +1776,6 @@ retry_auth:
session->userauth_pblc_packet = NULL;
session->userauth_pblc_state = libssh2_NB_state_idle;
rc = LIBSSH2_ERROR_NONE;
goto retry_auth;
}
else if(rc) {
@ -1783,6 +1788,11 @@ retry_auth:
"Callback returned error");
}
if(!sig) {
return _libssh2_error(session, LIBSSH2_ERROR_PUBLICKEY_UNVERIFIED,
"Callback did not return signature");
}
/*
* If this function was restarted, pubkeydata_len might still be 0
* which will cause an unnecessary but harmless realloc here.

15
src/wincng.c
View File

@ -70,12 +70,13 @@
#ifdef HAVE_LIBCRYPT32
#include <wincrypt.h> /* for CryptDecodeObjectEx() */
#endif
#define PEM_RSA_HEADER "-----BEGIN RSA PRIVATE KEY-----"
#define PEM_RSA_FOOTER "-----END RSA PRIVATE KEY-----"
#define PEM_DSA_HEADER "-----BEGIN DSA PRIVATE KEY-----"
#define PEM_DSA_FOOTER "-----END DSA PRIVATE KEY-----"
#endif
#if LIBSSH2_ECDSA
#define PEM_ECDSA_HEADER "-----BEGIN OPENSSH PRIVATE KEY-----"
#define PEM_ECDSA_FOOTER "-----END OPENSSH PRIVATE KEY-----"
@ -83,13 +84,14 @@
/* Define these manually to avoid including <ntstatus.h> and thus
clashing with <windows.h> symbols. */
#ifndef STATUS_NOT_SUPPORTED
#define STATUS_NOT_SUPPORTED ((NTSTATUS)0xC00000BB)
#endif
#ifndef STATUS_INVALID_SIGNATURE
#define STATUS_INVALID_SIGNATURE ((NTSTATUS)0xC000A000)
#endif
#endif
#ifndef STATUS_NOT_SUPPORTED
#define STATUS_NOT_SUPPORTED ((NTSTATUS)0xC00000BB)
#endif
/*******************************************************************/
/*
@ -3602,6 +3604,9 @@ _libssh2_wincng_bignum_rand(_libssh2_bn *rnd, int bits, int top, int bottom)
bignum = rnd->bignum;
if(!bignum)
return -1;
if(_libssh2_wincng_random(bignum, length))
return -1;

3
tests/openssh_fixture.c
View File

@ -241,14 +241,13 @@ static int is_running_inside_a_container(void)
FILE *f;
char *line = NULL;
size_t len = 0;
ssize_t read;
int found = 0;
f = fopen(cgroup_filename, "r");
if(!f) {
/* Don't go further, we are not in a container */
return 0;
}
while((read = getline(&line, &len, f)) != -1) {
while(getline(&line, &len, f) != -1) {
if(strstr(line, "docker")) {
found = 1;
break;

7
tests/session_fixture.c
View File

@ -438,7 +438,7 @@ static int read_file(const char *path, char **out_buffer, size_t *out_len)
fprintf(stderr, "Could not determine input size of: %s\n", path);
return 1;
}
rewind(fp);
fseek(fp, 0L, SEEK_SET);
buffer = calloc(1, (size_t)len + 1);
if(!buffer) {
@ -483,6 +483,11 @@ int test_auth_pubkey(LIBSSH2_SESSION *session, int flags,
}
}
if(!username) {
fprintf(stderr, "username not set\n");
return 1;
}
userauth_list = libssh2_userauth_list(session, username,
(unsigned int)strlen(username));
if(!userauth_list) {

5
tests/test_read.c
View File

@ -44,6 +44,11 @@ int test(LIBSSH2_SESSION *session)
#endif
}
if(!username) {
fprintf(stderr, "username not set\n");
return 1;
}
userauth_list = libssh2_userauth_list(session, username,
(unsigned int)strlen(username));
if(!userauth_list) {