Address issues reported by coverity

This commit is contained in:
Jakub Jelen 2018-08-29 12:56:24 +02:00
parent 4c36c2a9ee
commit c60b555ac2
9 changed files with 66 additions and 48 deletions

View File

@ -50,10 +50,10 @@ index 8f32464..18a2ca4 100644
+ +
+ cp = line; + cp = line;
+ arg = strdelim(&cp); + arg = strdelim(&cp);
+ if (*arg == '\0') + if (arg && *arg == '\0')
+ arg = strdelim(&cp); + arg = strdelim(&cp);
+ +
+ if (strcmp(arg, "privsep_preauth") == 0) { + if (arg && strcmp(arg, "privsep_preauth") == 0) {
+ arg = strdelim(&cp); + arg = strdelim(&cp);
+ if (!arg || *arg == '\0') { + if (!arg || *arg == '\0') {
+ debug("%s: privsep_preauth is empty", __func__); + debug("%s: privsep_preauth is empty", __func__);

View File

@ -39,7 +39,7 @@ diff -up openssh-6.8p1/Makefile.in.ctr-cavs openssh-6.8p1/Makefile.in
diff -up openssh-6.8p1/ctr-cavstest.c.ctr-cavs openssh-6.8p1/ctr-cavstest.c diff -up openssh-6.8p1/ctr-cavstest.c.ctr-cavs openssh-6.8p1/ctr-cavstest.c
--- openssh-6.8p1/ctr-cavstest.c.ctr-cavs 2015-03-18 11:22:05.521288952 +0100 --- openssh-6.8p1/ctr-cavstest.c.ctr-cavs 2015-03-18 11:22:05.521288952 +0100
+++ openssh-6.8p1/ctr-cavstest.c 2015-03-18 11:22:05.521288952 +0100 +++ openssh-6.8p1/ctr-cavstest.c 2015-03-18 11:22:05.521288952 +0100
@@ -0,0 +1,208 @@ @@ -0,0 +1,215 @@
+/* +/*
+ * + *
+ * invocation (all of the following are equal): + * invocation (all of the following are equal):
@ -60,6 +60,7 @@ diff -up openssh-6.8p1/ctr-cavstest.c.ctr-cavs openssh-6.8p1/ctr-cavstest.c
+ +
+#include "xmalloc.h" +#include "xmalloc.h"
+#include "log.h" +#include "log.h"
+#include "ssherr.h"
+#include "cipher.h" +#include "cipher.h"
+ +
+/* compatibility with old or broken OpenSSL versions */ +/* compatibility with old or broken OpenSSL versions */
@ -148,7 +149,7 @@ diff -up openssh-6.8p1/ctr-cavstest.c.ctr-cavs openssh-6.8p1/ctr-cavstest.c
+ char *hexiv = "00000000000000000000000000000000"; + char *hexiv = "00000000000000000000000000000000";
+ char *hexdata = NULL; + char *hexdata = NULL;
+ char *p; + char *p;
+ int i; + int i, r;
+ int encrypt = 1; + int encrypt = 1;
+ void *key; + void *key;
+ size_t keylen; + size_t keylen;
@ -221,10 +222,13 @@ diff -up openssh-6.8p1/ctr-cavstest.c.ctr-cavs openssh-6.8p1/ctr-cavstest.c
+ return 2; + return 2;
+ } + }
+ +
+ cipher_init(&cc, c, key, keylen, iv, ivlen, encrypt); + if ((r = cipher_init(&cc, c, key, keylen, iv, ivlen, encrypt)) != 0) {
+ fprintf(stderr, "Error: cipher_init failed: %s\n", ssh_err(r));
+ return 2;
+ }
+ +
+ free(key); + free(key);
+ free(iv); + free(iv);
+ +
+ outdata = malloc(datalen); + outdata = malloc(datalen);
+ if(outdata == NULL) { + if(outdata == NULL) {
@ -232,9 +236,12 @@ diff -up openssh-6.8p1/ctr-cavstest.c.ctr-cavs openssh-6.8p1/ctr-cavstest.c
+ return 2; + return 2;
+ } + }
+ +
+ cipher_crypt(cc, 0, outdata, data, datalen, 0, 0); + if ((r = cipher_crypt(cc, 0, outdata, data, datalen, 0, 0)) != 0) {
+ fprintf(stderr, "Error: cipher_crypt failed: %s\n", ssh_err(r));
+ return 2;
+ }
+ +
+ free(data); + free(data);
+ +
+ cipher_free(cc); + cipher_free(cc);
+ +

View File

@ -103,7 +103,7 @@ index 413b845..54dd383 100644
+ struct passwd *pw = the_authctxt->pw; + struct passwd *pw = the_authctxt->pw;
+ int found_principal = 0; + int found_principal = 0;
+ int ncommands = 0, allcommands = 0; + int ncommands = 0, allcommands = 0;
+ u_long linenum; + u_long linenum = 0;
+ size_t linesize = 0; + size_t linesize = 0;
+ +
+ snprintf(file, sizeof(file), "%s/.k5users", pw->pw_dir); + snprintf(file, sizeof(file), "%s/.k5users", pw->pw_dir);

View File

@ -646,7 +646,7 @@ diff -up openssh-6.8p1/ldap.conf.ldap openssh-6.8p1/ldap.conf
diff -up openssh-6.8p1/ldapbody.c.ldap openssh-6.8p1/ldapbody.c diff -up openssh-6.8p1/ldapbody.c.ldap openssh-6.8p1/ldapbody.c
--- openssh-6.8p1/ldapbody.c.ldap 2015-03-18 11:11:29.031801462 +0100 --- openssh-6.8p1/ldapbody.c.ldap 2015-03-18 11:11:29.031801462 +0100
+++ openssh-6.8p1/ldapbody.c 2015-03-18 11:11:29.031801462 +0100 +++ openssh-6.8p1/ldapbody.c 2015-03-18 11:11:29.031801462 +0100
@@ -0,0 +1,493 @@ @@ -0,0 +1,494 @@
+/* $OpenBSD: ldapbody.c,v 1.1 2009/12/03 03:34:42 jfch Exp $ */ +/* $OpenBSD: ldapbody.c,v 1.1 2009/12/03 03:34:42 jfch Exp $ */
+/* +/*
+ * Copyright (c) 2009 Jan F. Chadima. All rights reserved. + * Copyright (c) 2009 Jan F. Chadima. All rights reserved.
@ -801,7 +801,8 @@ diff -up openssh-6.8p1/ldapbody.c.ldap openssh-6.8p1/ldapbody.c
+ char *logfilename; + char *logfilename;
+ int logfilenamelen; + int logfilenamelen;
+ +
+ logfilenamelen = strlen (LDAP_LOGFILE) + strlen ("000000") + strlen (options.logdir); + logfilenamelen = strlen(LDAP_LOGFILE)
+ + strlen("000000") + strlen (options.logdir);
+ logfilename = xmalloc (logfilenamelen); + logfilename = xmalloc (logfilenamelen);
+ snprintf (logfilename, logfilenamelen, LDAP_LOGFILE, options.logdir, (int) getpid ()); + snprintf (logfilename, logfilenamelen, LDAP_LOGFILE, options.logdir, (int) getpid ());
+ logfilename[logfilenamelen - 1] = 0; + logfilename[logfilenamelen - 1] = 0;

View File

@ -371,7 +371,7 @@ index ed23f06d..bdb3109a 100644
/* If we're already complete - protocol error */ /* If we're already complete - protocol error */
if (maj_status == GSS_S_COMPLETE) if (maj_status == GSS_S_COMPLETE)
packet_disconnect("Protocol error: received token when complete"); packet_disconnect("Protocol error: received token when complete");
@@ -344,4 +345,381 @@ kexgss_client(struct ssh *ssh) { @@ -344,4 +345,382 @@ kexgss_client(struct ssh *ssh) {
return kex_send_newkeys(ssh); return kex_send_newkeys(ssh);
} }
@ -385,7 +385,7 @@ index ed23f06d..bdb3109a 100644
+ u_char *server_pub = NULL; + u_char *server_pub = NULL;
+ u_int server_pub_len = 0; + u_int server_pub_len = 0;
+ BIGNUM *shared_secret = NULL; + BIGNUM *shared_secret = NULL;
+ u_char *kbuf; + u_char *kbuf = NULL;
+ u_char *serverhostkey = NULL; + u_char *serverhostkey = NULL;
+ u_char *empty = ""; + u_char *empty = "";
+ char *msg; + char *msg;
@ -446,8 +446,9 @@ index ed23f06d..bdb3109a 100644
+ sizeof(kex->c25519_client_key)); + sizeof(kex->c25519_client_key));
+#endif +#endif
+ +
+ sshbuf_put_string(Q_C, kex->c25519_client_pubkey, + if ((r = sshbuf_put_string(Q_C, kex->c25519_client_pubkey,
+ sizeof(kex->c25519_client_pubkey)); + sizeof(kex->c25519_client_pubkey))) != 0)
+ fatal("%s: buffer error: %s", __func__, ssh_err(r));
+ break; + break;
+ default: + default:
+ fatal("%s: Unexpected KEX type %d", __func__, kex->kex_type); + fatal("%s: Unexpected KEX type %d", __func__, kex->kex_type);
@ -765,7 +766,7 @@ index b7da8823..a7c42803 100644
extern ServerOptions options; extern ServerOptions options;
@@ -303,4 +304,336 @@ kexgss_server(struct ssh *ssh) @@ -303,4 +304,338 @@ kexgss_server(struct ssh *ssh)
ssh_gssapi_rekey_creds(); ssh_gssapi_rekey_creds();
return 0; return 0;
} }
@ -787,7 +788,7 @@ index b7da8823..a7c42803 100644
+ gss_buffer_desc send_tok = GSS_C_EMPTY_BUFFER; + gss_buffer_desc send_tok = GSS_C_EMPTY_BUFFER;
+ Gssctxt *ctxt = NULL; + Gssctxt *ctxt = NULL;
+ u_int slen, klen = 0; + u_int slen, klen = 0;
+ u_char *kbuf; + u_char *kbuf = NULL;
+ BIGNUM *shared_secret = NULL; + BIGNUM *shared_secret = NULL;
+ int type = 0; + int type = 0;
+ gss_OID oid; + gss_OID oid;
@ -854,8 +855,9 @@ index b7da8823..a7c42803 100644
+ dump_digest("server private key:", c25519_server_key, + dump_digest("server private key:", c25519_server_key,
+ sizeof(c25519_server_key)); + sizeof(c25519_server_key));
+#endif +#endif
+ sshbuf_put_string(Q_S, c25519_server_pubkey, + if ((r = sshbuf_put_string(Q_S, c25519_server_pubkey,
+ sizeof(c25519_server_pubkey)); + sizeof(c25519_server_pubkey))) != 0)
+ fatal("%s: buffer error: %s", __func__, ssh_err(r));
+ break; + break;
+ default: + default:
+ fatal("%s: Unexpected KEX type %d", __func__, kex->kex_type); + fatal("%s: Unexpected KEX type %d", __func__, kex->kex_type);
@ -1036,7 +1038,8 @@ index b7da8823..a7c42803 100644
+ { + {
+ const u_char *ptr; + const u_char *ptr;
+ size_t len; + size_t len;
+ sshbuf_get_string_direct(Q_S, &ptr, &len); + if ((r = sshbuf_get_string_direct(Q_S, &ptr, &len)) != 0)
+ fatal("%s: buffer error: %s", __func__, ssh_err(r));
+ packet_put_string(ptr, len); + packet_put_string(ptr, len);
+ } + }
+ packet_put_string(msg_tok.value, msg_tok.length); + packet_put_string(msg_tok.value, msg_tok.length);

View File

@ -29,7 +29,7 @@ diff -up openssh-7.6p1/ssh-pkcs11-client.c.pkcs11-ecdsa openssh-7.6p1/ssh-pkcs11
{ {
static RSA_METHOD helper_rsa; static RSA_METHOD helper_rsa;
@@ -152,6 +160,85 @@ wrap_key(RSA *rsa) @@ -152,6 +160,88 @@ wrap_key(RSA *rsa)
return (0); return (0);
} }
@ -38,19 +38,20 @@ diff -up openssh-7.6p1/ssh-pkcs11-client.c.pkcs11-ecdsa openssh-7.6p1/ssh-pkcs11
+pkcs11_ecdsa_private_sign(const unsigned char *from, int flen, +pkcs11_ecdsa_private_sign(const unsigned char *from, int flen,
+ const BIGNUM *inv, const BIGNUM *rp, EC_KEY * ecdsa) + const BIGNUM *inv, const BIGNUM *rp, EC_KEY * ecdsa)
+{ +{
+ struct sshkey *key; + struct sshkey *key = NULL;
+ u_char *blob, *signature = NULL; + u_char *blob, *signature = NULL;
+ size_t blen, slen = 0; + size_t blen, slen = 0;
+ struct sshbuf *msg; + struct sshbuf *msg = NULL;
+ ECDSA_SIG *ret = NULL; + ECDSA_SIG *ret = NULL;
+ BIGNUM *r = NULL, *s = NULL; + BIGNUM *r = NULL, *s = NULL;
+ int rv; + int rv;
+ +
+ key = sshkey_new(KEY_ECDSA); + if ((key = sshkey_new(KEY_ECDSA)) == NULL)
+ fatal("%s: sshkey_new failed", __func__);
+ key->ecdsa = ecdsa; + key->ecdsa = ecdsa;
+ key->ecdsa_nid = sshkey_ecdsa_key_to_nid(ecdsa); + key->ecdsa_nid = sshkey_ecdsa_key_to_nid(ecdsa);
+ if (sshkey_to_blob(key, &blob, &blen) == 0) + if (sshkey_to_blob(key, &blob, &blen) == 0)
+ return NULL; + goto out;
+ if ((msg = sshbuf_new()) == NULL) + if ((msg = sshbuf_new()) == NULL)
+ fatal("%s: sshbuf_new failed", __func__); + fatal("%s: sshbuf_new failed", __func__);
+ if ((rv = sshbuf_put_u8(msg, SSH2_AGENTC_SIGN_REQUEST)) != 0 || + if ((rv = sshbuf_put_u8(msg, SSH2_AGENTC_SIGN_REQUEST)) != 0 ||
@ -76,6 +77,8 @@ diff -up openssh-7.6p1/ssh-pkcs11-client.c.pkcs11-ecdsa openssh-7.6p1/ssh-pkcs11
+ } + }
+ free(signature); + free(signature);
+ } + }
+out:
+ sshkey_free(key);
+ sshbuf_free(msg); + sshbuf_free(msg);
+ return (ret); + return (ret);
+} +}

View File

@ -3469,7 +3469,7 @@ index 00000000..da15c164
+ struct sshbuf *value) + struct sshbuf *value)
+{ +{
+ char *new_part; + char *new_part;
+ size_t size; + size_t size = 0;
+ +
+ if (value == NULL) + if (value == NULL)
+ return NULL; + return NULL;
@ -3482,7 +3482,7 @@ index 00000000..da15c164
+ sshbuf_free(value); + sshbuf_free(value);
+ free(part); + free(part);
+ +
+ if (size < 0) + if (size <= 0)
+ return NULL; + return NULL;
+ return new_part; + return new_part;
+} +}
@ -3490,7 +3490,7 @@ index 00000000..da15c164
+char * +char *
+pkcs11_uri_get(struct pkcs11_uri *uri) +pkcs11_uri_get(struct pkcs11_uri *uri)
+{ +{
+ size_t size = -1; + size_t size = 0;
+ char *p = NULL, *path = NULL, *query = NULL; + char *p = NULL, *path = NULL, *query = NULL;
+ +
+ /* compose a percent-encoded ID */ + /* compose a percent-encoded ID */
@ -3549,7 +3549,7 @@ index 00000000..da15c164
+err: +err:
+ free(query); + free(query);
+ free(path); + free(path);
+ if (size < 0) + if (size <= 0)
+ return NULL; + return NULL;
+ return p; + return p;
+} +}

View File

@ -92,21 +92,21 @@ diff -up openssh/pam_ssh_agent_auth-0.10.3/iterate_ssh_agent_keys.c.psaa-compat
void void
-agent_action(Buffer *buf, char ** action, size_t count) -agent_action(Buffer *buf, char ** action, size_t count)
+agent_action(struct sshbuf *buf, char ** action, size_t count) +agent_action(struct sshbuf **buf, char ** action, size_t count)
{ {
size_t i; size_t i;
- pamsshagentauth_buffer_init(buf); - pamsshagentauth_buffer_init(buf);
+ int r; + int r;
- pamsshagentauth_buffer_put_int(buf, count); - pamsshagentauth_buffer_put_int(buf, count);
+ if ((buf = sshbuf_new()) == NULL) + if ((*buf = sshbuf_new()) == NULL)
+ fatal("%s: sshbuf_new failed", __func__); + fatal("%s: sshbuf_new failed", __func__);
+ if ((r = sshbuf_put_u32(buf, count)) != 0) + if ((r = sshbuf_put_u32(*buf, count)) != 0)
+ fatal("%s: buffer error: %s", __func__, ssh_err(r)); + fatal("%s: buffer error: %s", __func__, ssh_err(r));
for (i = 0; i < count; i++) { for (i = 0; i < count; i++) {
- pamsshagentauth_buffer_put_cstring(buf, action[i]); - pamsshagentauth_buffer_put_cstring(buf, action[i]);
+ if ((r = sshbuf_put_cstring(buf, action[i])) != 0) + if ((r = sshbuf_put_cstring(*buf, action[i])) != 0)
+ fatal("%s: buffer error: %s", __func__, ssh_err(r)); + fatal("%s: buffer error: %s", __func__, ssh_err(r));
} }
} }
@ -152,7 +152,7 @@ diff -up openssh/pam_ssh_agent_auth-0.10.3/iterate_ssh_agent_keys.c.psaa-compat
free_logbuf = 1; free_logbuf = 1;
action_logbuf = log_action(reported_argv, count); action_logbuf = log_action(reported_argv, count);
- agent_action(&action_agentbuf, reported_argv, count); - agent_action(&action_agentbuf, reported_argv, count);
+ agent_action(action_agentbuf, reported_argv, count); + agent_action(&action_agentbuf, reported_argv, count);
pamsshagentauth_free_command_line(reported_argv, count); pamsshagentauth_free_command_line(reported_argv, count);
} }
else { else {

View File

@ -44,15 +44,16 @@ diff -up openssh/pam_ssh_agent_auth-0.10.3/iterate_ssh_agent_keys.c.psaa-agent o
if ((ac = ssh_get_authentication_connection_for_uid(uid))) { if ((ac = ssh_get_authentication_connection_for_uid(uid))) {
verbose("Contacted ssh-agent of user %s (%u)", ruser, uid); verbose("Contacted ssh-agent of user %s (%u)", ruser, uid);
- for (key = ssh_get_first_identity(ac, &comment, 2); key != NULL; key = ssh_get_next_identity(ac, &comment, 2)) - for (key = ssh_get_first_identity(ac, &comment, 2); key != NULL; key = ssh_get_next_identity(ac, &comment, 2))
+ if ((r = ssh_fetch_identitylist(ac->fd, &idlist)) != 0) { - {
+ if (r != SSH_ERR_AGENT_NO_IDENTITIES)
+ fprintf(stderr, "error fetching identities for "
+ "protocol %d: %s\n", 2, ssh_err(r));
+ } else {
+ for (i = 0; i < idlist->nkeys; i++)
{
- if(key != NULL) { - if(key != NULL) {
+ if(idlist->keys[i] != NULL) { + if ((r = ssh_fetch_identitylist(ac->fd, &idlist)) != 0) {
+ if (r != SSH_ERR_AGENT_NO_IDENTITIES)
+ fprintf(stderr, "error fetching identities for "
+ "protocol %d: %s\n", 2, ssh_err(r));
+ } else {
+ for (i = 0; i < idlist->nkeys; i++)
+ {
+ if (idlist->keys[i] != NULL) {
id = xcalloc(1, sizeof(*id)); id = xcalloc(1, sizeof(*id));
- id->key = key; - id->key = key;
- id->filename = comment; - id->filename = comment;
@ -67,14 +68,17 @@ diff -up openssh/pam_ssh_agent_auth-0.10.3/iterate_ssh_agent_keys.c.psaa-agent o
free(id); free(id);
if(retval == 1) if(retval == 1)
break; break;
} - }
} - }
sshbuf_free(session_id2); + }
+ }
- sshbuf_free(session_id2);
- ssh_close_authentication_connection(ac); - ssh_close_authentication_connection(ac);
+ ssh_free_identitylist(idlist); + sshbuf_free(session_id2);
+ ssh_free_identitylist(idlist);
+ }
+ ssh_close_authentication_socket(ac->fd); + ssh_close_authentication_socket(ac->fd);
+ free(ac); + free(ac);
+ }
} }
else { else {
verbose("No ssh-agent could be contacted"); verbose("No ssh-agent could be contacted");