Attachment 3287 Details for Bug 2687 – New coverity issues from 8.0p1 release

[patch] New coverity issues from 8.0p1 release

openssh-coverity.patch (text/plain), 6.81 KB, created by Jakub Jelen on 2019-05-27 22:19:46 AEST

(hide)

Description:

Filename:

MIME Type:

Creator: Jakub Jelen

Created: 2019-05-27 22:19:46 AEST

Size: 6.81 KB

patch

obsolete

>From b4d650207810ea6a0ef2bf0d66429052e8ae5f90 Mon Sep 17 00:00:00 2001
>From: Jakub Jelen <jjelen@redhat.com>
>Date: Mon, 27 May 2019 13:27:37 +0200
>Subject: [PATCH 1/6] Coverity warning: leaked storage
>
> 28. openssh-8.0p1/ssh_api.c:388: leaked_storage: Variable "cp" going out of scope leaks the storage it points to.
>
> 25. openssh-8.0p1/ssh_api.c:378: leaked_storage: Variable "remote_version" going out of scope leaks the storage it points to.
>
> 25. openssh-8.0p1/ssh_api.c:378: leaked_storage: Variable "cp" going out of scope leaks the storage it points to.
>
> 23. openssh-8.0p1/ssh_api.c:370: leaked_storage: Variable "cp" going out of scope leaks the storage it points to.
>---
> ssh_api.c | 31 +++++++++++++++++++++----------
> 1 file changed, 21 insertions(+), 10 deletions(-)
>
>diff --git a/ssh_api.c b/ssh_api.c
>index 57509973..79dcd7e0 100644
>--- a/ssh_api.c
>+++ b/ssh_api.c
>@@ -320,7 +320,7 @@ _ssh_read_banner(struct ssh *ssh, struct sshbuf *banner)
> 	const char *mismatch = "Protocol mismatch.\r\n";
> 	const u_char *s = sshbuf_ptr(input);
> 	u_char c;
>-	char *cp, *remote_version;
>+	char *cp = NULL, *remote_version = NULL;
> 	int r, remote_major, remote_minor, expect_nl;
> 	size_t n, j;
> 
>@@ -363,19 +363,25 @@ _ssh_read_banner(struct ssh *ssh, struct sshbuf *banner)
> 	if ((r = sshbuf_consume(input, j)) != 0)
> 		return r;
> 
>-	if ((cp = sshbuf_dup_string(banner)) == NULL)
>-		return SSH_ERR_ALLOC_FAIL;
>+	if ((cp = sshbuf_dup_string(banner)) == NULL) {
>+		r = SSH_ERR_ALLOC_FAIL;
>+		goto out;
>+	}
> 	/* XXX remote version must be the same size as banner for sscanf */
>-	if ((remote_version = calloc(1, sshbuf_len(banner))) == NULL)
>-		return SSH_ERR_ALLOC_FAIL;
>+	if ((remote_version = calloc(1, sshbuf_len(banner))) == NULL) {
>+		r = SSH_ERR_ALLOC_FAIL;
>+		goto out;
>+	}
> 
> 	/*
> 	 * Check that the versions match.  In future this might accept
> 	 * several versions and set appropriate flags to handle them.
> 	 */
> 	if (sscanf(cp, "SSH-%d.%d-%[^\n]\n",
>-	    &remote_major, &remote_minor, remote_version) != 3)
>-		return SSH_ERR_INVALID_FORMAT;
>+	    &remote_major, &remote_minor, remote_version) != 3) {
>+		r = SSH_ERR_INVALID_FORMAT;
>+		goto out;
>+	}
> 	debug("Remote protocol version %d.%d, remote software version %.100s",
> 	    remote_major, remote_minor, remote_version);
> 
>@@ -384,11 +390,16 @@ _ssh_read_banner(struct ssh *ssh, struct sshbuf *banner)
> 		remote_major = 2;
> 		remote_minor = 0;
> 	}
>-	if (remote_major != 2)
>-		return SSH_ERR_PROTOCOL_MISMATCH;
>+	if (remote_major != 2) {
>+		r = SSH_ERR_PROTOCOL_MISMATCH;
>+		goto out;
>+	}
> 	debug("Remote version string %.100s", cp);
>+
>+out:
>+	free(remote_version);
> 	free(cp);
>-	return 0;
>+	return r;
> }
> 
> /* Send our own protocol version identification. */
>-- 
>2.20.1
>
>
>From cfd602f834960814ba17bb756135684e30e065e3 Mon Sep 17 00:00:00 2001
>From: Jakub Jelen <jjelen@redhat.com>
>Date: Mon, 27 May 2019 13:40:42 +0200
>Subject: [PATCH 2/6] Coverity warning: leaked storage
>
> 64. openssh-8.0p1/ssh-keygen.c:2291: leaked_storage: Variable "blob" going out of scope leaks the storage it points to.
>---
> ssh-keygen.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/ssh-keygen.c b/ssh-keygen.c
>index 91c43b14..7c2c4527 100644
>--- a/ssh-keygen.c
>+++ b/ssh-keygen.c
>@@ -2228,6 +2228,9 @@ update_krl_from_file(struct passwd *pw, const char *file, int wild_ca,
> 			cp = cp + strspn(cp, " \t");
> 			hash_to_blob(cp, &blob, &blen, file, lnum);
> 			r = ssh_krl_revoke_key_sha256(krl, blob, blen);
>+			freezero(blob, blen);
>+			blob = NULL;
>+			blen = 0;
> 		} else {
> 			if (strncasecmp(cp, "key:", 4) == 0) {
> 				cp += 4;
>-- 
>2.20.1
>
>
>From 73bf5d1f21852f8e6ea315c64a6960a27f1c2c0d Mon Sep 17 00:00:00 2001
>From: Jakub Jelen <jjelen@redhat.com>
>Date: Mon, 27 May 2019 14:02:12 +0200
>Subject: [PATCH 3/6] Coverity: leaked storage
>
> 43. openssh-8.0p1/auth-options.c:538: leaked_storage: Variable "opt" going out of scope leaks the storage it points to.
>---
> auth-options.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/auth-options.c b/auth-options.c
>index b05d6d6f..f7ef56d3 100644
>--- a/auth-options.c
>+++ b/auth-options.c
>@@ -469,8 +469,10 @@ sshauthopt_parse(const char *opts, const char **errstrp)
> 				errstr = "invalid environment string";
> 				goto fail;
> 			}
>-			if ((cp = strdup(opt)) == NULL)
>+			if ((cp = strdup(opt)) == NULL) {
>+				free(opt);
> 				goto alloc_fail;
>+			}
> 			cp[tmp - opt] = '\0'; /* truncate at '=' */
> 			if (!valid_env_name(cp)) {
> 				free(cp);
>-- 
>2.20.1
>
>
>From e6e54a94be55110d7b4bf2a220d8f7c2f5d4062d Mon Sep 17 00:00:00 2001
>From: Jakub Jelen <jjelen@redhat.com>
>Date: Mon, 27 May 2019 14:06:23 +0200
>Subject: [PATCH 4/6] Coverity: Remove unused variable max
>
> 1. openssh-8.0p1/ssh-pkcs11-helper.c:356:3: warning: Value stored to 'max' is never read
>---
> ssh-pkcs11-helper.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
>diff --git a/ssh-pkcs11-helper.c b/ssh-pkcs11-helper.c
>index 97fb1212..9421ae9a 100644
>--- a/ssh-pkcs11-helper.c
>+++ b/ssh-pkcs11-helper.c
>@@ -312,7 +312,7 @@ cleanup_exit(int i)
> int
> main(int argc, char **argv)
> {
>-	int r, ch, in, out, max, log_stderr = 0;
>+	int r, ch, in, out, log_stderr = 0;
> 	ssize_t len;
> 	SyslogFacility log_facility = SYSLOG_FACILITY_AUTH;
> 	LogLevel log_level = SYSLOG_LEVEL_ERROR;
>@@ -349,12 +349,6 @@ main(int argc, char **argv)
> 	in = STDIN_FILENO;
> 	out = STDOUT_FILENO;
> 
>-	max = 0;
>-	if (in > max)
>-		max = in;
>-	if (out > max)
>-		max = out;
>-
> 	if ((iqueue = sshbuf_new()) == NULL)
> 		fatal("%s: sshbuf_new failed", __func__);
> 	if ((oqueue = sshbuf_new()) == NULL)
>-- 
>2.20.1
>
>
>From 34896a924eddfbe8e1820955d6e4cd3f370880ca Mon Sep 17 00:00:00 2001
>From: Jakub Jelen <jjelen@redhat.com>
>Date: Mon, 27 May 2019 14:08:06 +0200
>Subject: [PATCH 5/6] Coverity: unused return value
>
> 1. openssh-8.0p1/ssh-keygen.c:2236:4: warning: Value stored to 'r' is never read
>---
> ssh-keygen.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/ssh-keygen.c b/ssh-keygen.c
>index 7c2c4527..42d85ac8 100644
>--- a/ssh-keygen.c
>+++ b/ssh-keygen.c
>@@ -2231,6 +2231,9 @@ update_krl_from_file(struct passwd *pw, const char *file, int wild_ca,
> 			freezero(blob, blen);
> 			blob = NULL;
> 			blen = 0;
>+			if (r != 0)
>+				fatal("%s: revoke key failed: %s",
>+				    __func__, ssh_err(r));
> 		} else {
> 			if (strncasecmp(cp, "key:", 4) == 0) {
> 				cp += 4;
>-- 
>2.20.1
>
>
>From 14eab3a31c68b5aaed307fdf6a3260c3a3035d7f Mon Sep 17 00:00:00 2001
>From: Jakub Jelen <jjelen@redhat.com>
>Date: Mon, 27 May 2019 14:14:58 +0200
>Subject: [PATCH 6/6] Coverity: Unused variable r
>
> 1. openssh-8.0p1/auth2.c:221:2: warning: Value stored to 'r' is never read
>---
> auth2.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/auth2.c b/auth2.c
>index 16ae1a36..4603daa7 100644
>--- a/auth2.c
>+++ b/auth2.c
>@@ -219,7 +219,7 @@ input_service_request(int type, u_int32_t seq, struct ssh *ssh)
> 	r = 0;
>  out:
> 	free(service);
>-	return 0;
>+	return r;
> }
> 
> #define MIN_FAIL_DELAY_SECONDS 0.005
>-- 
>2.20.1
>

Actions: View | Diff

Attachments on bug 2687: 2953 | 2954 | 3176 | 3287