From c60b555ac213744f96317b15ad8e3a5046892c6a Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Wed, 29 Aug 2018 12:56:24 +0200 Subject: [PATCH] Address issues reported by coverity --- openssh-6.6.1p1-selinux-contexts.patch | 4 +-- openssh-6.6p1-ctr-cavstest.patch | 21 ++++++++----- openssh-6.6p1-force_krb.patch | 2 +- openssh-6.7p1-ldap.patch | 5 ++-- openssh-7.5p1-gssapi-kex-with-ec.patch | 21 +++++++------ openssh-7.6p1-pkcs11-ecdsa.patch | 13 ++++---- openssh-7.6p1-pkcs11-uri.patch | 8 ++--- pam_ssh_agent_auth-0.10.2-compat.patch | 10 +++---- ...ssh_agent_auth-0.9.3-agent_structure.patch | 30 +++++++++++-------- 9 files changed, 66 insertions(+), 48 deletions(-) diff --git a/openssh-6.6.1p1-selinux-contexts.patch b/openssh-6.6.1p1-selinux-contexts.patch index 90e8627..8bf6c78 100644 --- a/openssh-6.6.1p1-selinux-contexts.patch +++ b/openssh-6.6.1p1-selinux-contexts.patch @@ -50,10 +50,10 @@ index 8f32464..18a2ca4 100644 + + cp = line; + arg = strdelim(&cp); -+ if (*arg == '\0') ++ if (arg && *arg == '\0') + arg = strdelim(&cp); + -+ if (strcmp(arg, "privsep_preauth") == 0) { ++ if (arg && strcmp(arg, "privsep_preauth") == 0) { + arg = strdelim(&cp); + if (!arg || *arg == '\0') { + debug("%s: privsep_preauth is empty", __func__); diff --git a/openssh-6.6p1-ctr-cavstest.patch b/openssh-6.6p1-ctr-cavstest.patch index 42c399d..c56313d 100644 --- a/openssh-6.6p1-ctr-cavstest.patch +++ b/openssh-6.6p1-ctr-cavstest.patch @@ -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 --- 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 -@@ -0,0 +1,208 @@ +@@ -0,0 +1,215 @@ +/* + * + * 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 "log.h" ++#include "ssherr.h" +#include "cipher.h" + +/* 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 *hexdata = NULL; + char *p; -+ int i; ++ int i, r; + int encrypt = 1; + void *key; + 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; + } + -+ 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(iv); ++ free(key); ++ free(iv); + + outdata = malloc(datalen); + 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; + } + -+ 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); + diff --git a/openssh-6.6p1-force_krb.patch b/openssh-6.6p1-force_krb.patch index 288963e..aeee77f 100644 --- a/openssh-6.6p1-force_krb.patch +++ b/openssh-6.6p1-force_krb.patch @@ -103,7 +103,7 @@ index 413b845..54dd383 100644 + struct passwd *pw = the_authctxt->pw; + int found_principal = 0; + int ncommands = 0, allcommands = 0; -+ u_long linenum; ++ u_long linenum = 0; + size_t linesize = 0; + + snprintf(file, sizeof(file), "%s/.k5users", pw->pw_dir); diff --git a/openssh-6.7p1-ldap.patch b/openssh-6.7p1-ldap.patch index 328b95e..ee44fdb 100644 --- a/openssh-6.7p1-ldap.patch +++ b/openssh-6.7p1-ldap.patch @@ -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 --- 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 -@@ -0,0 +1,493 @@ +@@ -0,0 +1,494 @@ +/* $OpenBSD: ldapbody.c,v 1.1 2009/12/03 03:34:42 jfch Exp $ */ +/* + * 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; + int logfilenamelen; + -+ logfilenamelen = strlen (LDAP_LOGFILE) + strlen ("000000") + strlen (options.logdir); ++ logfilenamelen = strlen(LDAP_LOGFILE) ++ + strlen("000000") + strlen (options.logdir); + logfilename = xmalloc (logfilenamelen); + snprintf (logfilename, logfilenamelen, LDAP_LOGFILE, options.logdir, (int) getpid ()); + logfilename[logfilenamelen - 1] = 0; diff --git a/openssh-7.5p1-gssapi-kex-with-ec.patch b/openssh-7.5p1-gssapi-kex-with-ec.patch index 1e2d650..5f2763a 100644 --- a/openssh-7.5p1-gssapi-kex-with-ec.patch +++ b/openssh-7.5p1-gssapi-kex-with-ec.patch @@ -371,7 +371,7 @@ index ed23f06d..bdb3109a 100644 /* If we're already complete - protocol error */ if (maj_status == GSS_S_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); } @@ -385,7 +385,7 @@ index ed23f06d..bdb3109a 100644 + u_char *server_pub = NULL; + u_int server_pub_len = 0; + BIGNUM *shared_secret = NULL; -+ u_char *kbuf; ++ u_char *kbuf = NULL; + u_char *serverhostkey = NULL; + u_char *empty = ""; + char *msg; @@ -446,8 +446,9 @@ index ed23f06d..bdb3109a 100644 + sizeof(kex->c25519_client_key)); +#endif + -+ sshbuf_put_string(Q_C, kex->c25519_client_pubkey, -+ sizeof(kex->c25519_client_pubkey)); ++ if ((r = sshbuf_put_string(Q_C, kex->c25519_client_pubkey, ++ sizeof(kex->c25519_client_pubkey))) != 0) ++ fatal("%s: buffer error: %s", __func__, ssh_err(r)); + break; + default: + fatal("%s: Unexpected KEX type %d", __func__, kex->kex_type); @@ -765,7 +766,7 @@ index b7da8823..a7c42803 100644 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(); return 0; } @@ -787,7 +788,7 @@ index b7da8823..a7c42803 100644 + gss_buffer_desc send_tok = GSS_C_EMPTY_BUFFER; + Gssctxt *ctxt = NULL; + u_int slen, klen = 0; -+ u_char *kbuf; ++ u_char *kbuf = NULL; + BIGNUM *shared_secret = NULL; + int type = 0; + gss_OID oid; @@ -854,8 +855,9 @@ index b7da8823..a7c42803 100644 + dump_digest("server private key:", c25519_server_key, + sizeof(c25519_server_key)); +#endif -+ sshbuf_put_string(Q_S, c25519_server_pubkey, -+ sizeof(c25519_server_pubkey)); ++ if ((r = sshbuf_put_string(Q_S, c25519_server_pubkey, ++ sizeof(c25519_server_pubkey))) != 0) ++ fatal("%s: buffer error: %s", __func__, ssh_err(r)); + break; + default: + fatal("%s: Unexpected KEX type %d", __func__, kex->kex_type); @@ -1036,7 +1038,8 @@ index b7da8823..a7c42803 100644 + { + const u_char *ptr; + 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(msg_tok.value, msg_tok.length); diff --git a/openssh-7.6p1-pkcs11-ecdsa.patch b/openssh-7.6p1-pkcs11-ecdsa.patch index 9b19b68..2e73d45 100644 --- a/openssh-7.6p1-pkcs11-ecdsa.patch +++ b/openssh-7.6p1-pkcs11-ecdsa.patch @@ -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; -@@ -152,6 +160,85 @@ wrap_key(RSA *rsa) +@@ -152,6 +160,88 @@ wrap_key(RSA *rsa) 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, + const BIGNUM *inv, const BIGNUM *rp, EC_KEY * ecdsa) +{ -+ struct sshkey *key; ++ struct sshkey *key = NULL; + u_char *blob, *signature = NULL; + size_t blen, slen = 0; -+ struct sshbuf *msg; ++ struct sshbuf *msg = NULL; + ECDSA_SIG *ret = NULL; + BIGNUM *r = NULL, *s = NULL; + 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_nid = sshkey_ecdsa_key_to_nid(ecdsa); + if (sshkey_to_blob(key, &blob, &blen) == 0) -+ return NULL; ++ goto out; + if ((msg = sshbuf_new()) == NULL) + fatal("%s: sshbuf_new failed", __func__); + 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); + } ++out: ++ sshkey_free(key); + sshbuf_free(msg); + return (ret); +} diff --git a/openssh-7.6p1-pkcs11-uri.patch b/openssh-7.6p1-pkcs11-uri.patch index c032ed5..6eca876 100644 --- a/openssh-7.6p1-pkcs11-uri.patch +++ b/openssh-7.6p1-pkcs11-uri.patch @@ -3469,7 +3469,7 @@ index 00000000..da15c164 + struct sshbuf *value) +{ + char *new_part; -+ size_t size; ++ size_t size = 0; + + if (value == NULL) + return NULL; @@ -3482,7 +3482,7 @@ index 00000000..da15c164 + sshbuf_free(value); + free(part); + -+ if (size < 0) ++ if (size <= 0) + return NULL; + return new_part; +} @@ -3490,7 +3490,7 @@ index 00000000..da15c164 +char * +pkcs11_uri_get(struct pkcs11_uri *uri) +{ -+ size_t size = -1; ++ size_t size = 0; + char *p = NULL, *path = NULL, *query = NULL; + + /* compose a percent-encoded ID */ @@ -3549,7 +3549,7 @@ index 00000000..da15c164 +err: + free(query); + free(path); -+ if (size < 0) ++ if (size <= 0) + return NULL; + return p; +} diff --git a/pam_ssh_agent_auth-0.10.2-compat.patch b/pam_ssh_agent_auth-0.10.2-compat.patch index a7fda5b..6352bfa 100644 --- a/pam_ssh_agent_auth-0.10.2-compat.patch +++ b/pam_ssh_agent_auth-0.10.2-compat.patch @@ -92,21 +92,21 @@ diff -up openssh/pam_ssh_agent_auth-0.10.3/iterate_ssh_agent_keys.c.psaa-compat void -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; - pamsshagentauth_buffer_init(buf); + int r; - pamsshagentauth_buffer_put_int(buf, count); -+ if ((buf = sshbuf_new()) == NULL) ++ if ((*buf = sshbuf_new()) == NULL) + 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)); for (i = 0; i < count; 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)); } } @@ -152,7 +152,7 @@ diff -up openssh/pam_ssh_agent_auth-0.10.3/iterate_ssh_agent_keys.c.psaa-compat free_logbuf = 1; 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); pamsshagentauth_free_command_line(reported_argv, count); } else { diff --git a/pam_ssh_agent_auth-0.9.3-agent_structure.patch b/pam_ssh_agent_auth-0.9.3-agent_structure.patch index 26ae902..1f2c02c 100644 --- a/pam_ssh_agent_auth-0.9.3-agent_structure.patch +++ b/pam_ssh_agent_auth-0.9.3-agent_structure.patch @@ -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))) { 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)) -+ 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(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->key = key; - 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); if(retval == 1) break; - } - } - sshbuf_free(session_id2); +- } +- } ++ } ++ } +- sshbuf_free(session_id2); - ssh_close_authentication_connection(ac); -+ ssh_free_identitylist(idlist); ++ sshbuf_free(session_id2); ++ ssh_free_identitylist(idlist); ++ } + ssh_close_authentication_socket(ac->fd); + free(ac); -+ } } else { verbose("No ssh-agent could be contacted");