Bugzilla – Attachment 3164 Details for
Bug 2883
Coverity issues
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
resolve memory leaks
openssh_coverity.log (text/plain), 7.28 KB, created by
Jakub Jelen
on 2018-07-19 00:46:44 AEST
(
hide
)
Description:
resolve memory leaks
Filename:
MIME Type:
Creator:
Jakub Jelen
Created:
2018-07-19 00:46:44 AEST
Size:
7.28 KB
patch
obsolete
>From 984749e0d2a2292d51eeb6a18e0fca6ff143e59d Mon Sep 17 00:00:00 2001 >From: Jakub Jelen <jjelen@redhat.com> >Date: Wed, 18 Jul 2018 14:19:56 +0200 >Subject: [PATCH 1/4] coverity: RESOURCE_LEAK (CWE-772) > >addrmatch.c:217: leaked_storage: Variable "ai" going out of scope leaks the storage it points to. > 215| > 216| if (ai == NULL || ai->ai_addr == NULL) > 217|-> return -1; > 218| > 219| if (n != NULL && >--- > addrmatch.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > >diff --git a/addrmatch.c b/addrmatch.c >index 8658e105..74433fe0 100644 >--- a/addrmatch.c >+++ b/addrmatch.c >@@ -213,9 +213,14 @@ addr_pton(const char *p, struct xaddr *n) > if (p == NULL || getaddrinfo(p, NULL, &hints, &ai) != 0) > return -1; > >- if (ai == NULL || ai->ai_addr == NULL) >+ if (ai == NULL) > return -1; > >+ if (ai == NULL || ai->ai_addr == NULL) { >+ freeaddrinfo(ai); >+ return -1; >+ } >+ > if (n != NULL && > addr_sa_to_xaddr(ai->ai_addr, ai->ai_addrlen, n) == -1) { > freeaddrinfo(ai); >-- >2.17.1 > > >From 188974954591c7e908b12090d4209c1a72b25346 Mon Sep 17 00:00:00 2001 >From: Jakub Jelen <jjelen@redhat.com> >Date: Wed, 18 Jul 2018 14:52:56 +0200 >Subject: [PATCH 2/4] coverity: RESOURCE_LEAK (CWE-772) > >(this ran for older version, so the code is a bit different now) > >openssh-7.7p1/compat.c:212: overwrite_var: Overwriting "p" in "p = match_filter_list(p, "diffie-hellman-group-exchange-sha256,diffie-hellman-group-exchange-sha1")" leaks the storage that "p" points to. > 210| fatal("match_filter_list failed"); > 211| if ((datafellows & SSH_OLD_DHGEX) != 0) { > 212|-> if ((p = match_filter_list(p, > 213| "diffie-hellman-group-exchange-sha256," > 214| "diffie-hellman-group-exchange-sha1")) == NULL) >--- > compat.c | 31 ++++++++++++++++++++++++------- > 1 file changed, 24 insertions(+), 7 deletions(-) > >diff --git a/compat.c b/compat.c >index d0afe908..2e6942c6 100644 >--- a/compat.c >+++ b/compat.c >@@ -186,11 +186,16 @@ proto_spec(const char *spec) > char * > compat_cipher_proposal(char *cipher_prop) > { >+ char *np = NULL; >+ > if (!(datafellows & SSH_BUG_BIGENDIANAES)) > return cipher_prop; > debug2("%s: original cipher proposal: %s", __func__, cipher_prop); >- if ((cipher_prop = match_filter_blacklist(cipher_prop, "aes*")) == NULL) >+ np = match_filter_blacklist(cipher_prop, "aes*"); >+ if (np == NULL) > fatal("match_filter_blacklist failed"); >+ free(cipher_prop); >+ cipher_prop = np; > debug2("%s: compat cipher proposal: %s", __func__, cipher_prop); > if (*cipher_prop == '\0') > fatal("No supported ciphers found"); >@@ -200,11 +205,16 @@ compat_cipher_proposal(char *cipher_prop) > char * > compat_pkalg_proposal(char *pkalg_prop) > { >+ char *np = NULL; >+ > if (!(datafellows & SSH_BUG_RSASIGMD5)) > return pkalg_prop; > debug2("%s: original public key proposal: %s", __func__, pkalg_prop); >- if ((pkalg_prop = match_filter_blacklist(pkalg_prop, "ssh-rsa")) == NULL) >+ np = match_filter_blacklist(pkalg_prop, "ssh-rsa"); >+ if (np == NULL) > fatal("match_filter_blacklist failed"); >+ free(pkalg_prop); >+ pkalg_prop = np; > debug2("%s: compat public key proposal: %s", __func__, pkalg_prop); > if (*pkalg_prop == '\0') > fatal("No supported PK algorithms found"); >@@ -217,15 +227,22 @@ compat_kex_proposal(char *p) > if ((datafellows & (SSH_BUG_CURVE25519PAD|SSH_OLD_DHGEX)) == 0) > return p; > debug2("%s: original KEX proposal: %s", __func__, p); >- if ((datafellows & SSH_BUG_CURVE25519PAD) != 0) >- if ((p = match_filter_blacklist(p, >- "curve25519-sha256@libssh.org")) == NULL) >+ if ((datafellows & SSH_BUG_CURVE25519PAD) != 0) { >+ char *np = match_filter_blacklist(p, >+ "curve25519-sha256@libssh.org"); >+ if (np == NULL) > fatal("match_filter_blacklist failed"); >+ free(p); >+ p = np; >+ } > if ((datafellows & SSH_OLD_DHGEX) != 0) { >- if ((p = match_filter_blacklist(p, >+ char *np = match_filter_blacklist(p, > "diffie-hellman-group-exchange-sha256," >- "diffie-hellman-group-exchange-sha1")) == NULL) >+ "diffie-hellman-group-exchange-sha1"); >+ if (np == NULL) > fatal("match_filter_blacklist failed"); >+ free(p); >+ p = np; > } > debug2("%s: compat KEX proposal: %s", __func__, p); > if (*p == '\0') >-- >2.17.1 > > >From e7156fa766b4bd01b7f6b72c60b5b7c80f7d98fa Mon Sep 17 00:00:00 2001 >From: Jakub Jelen <jjelen@redhat.com> >Date: Wed, 18 Jul 2018 15:15:41 +0200 >Subject: [PATCH 3/4] coverity: RESOURCE_LEAK (CWE-772) > >openssh-7.7p1/mux.c:1063: leaked_storage: Variable "chost" going out of scope leaks the storage it points to. > 1061| > 1062| /* reply is deferred, sent by mux_session_confirm */ > 1063|-> return 0; > 1064| } > 1065| >--- > mux.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/mux.c b/mux.c >index 6394e3e1..143ee145 100644 >--- a/mux.c >+++ b/mux.c >@@ -1042,6 +1042,7 @@ process_mux_stdio_fwd(struct ssh *ssh, u_int rid, > set_nonblock(new_fd[1]); > > nc = channel_connect_stdio_fwd(ssh, chost, cport, new_fd[0], new_fd[1]); >+ free(chost); > > nc->ctl_chan = c->self; /* link session -> control channel */ > c->remote_id = nc->self; /* link control -> session channel */ >-- >2.17.1 > > >From ab015ffbd89b39aee7087c2a4e046ee68d938fb0 Mon Sep 17 00:00:00 2001 >From: Jakub Jelen <jjelen@redhat.com> >Date: Wed, 18 Jul 2018 15:19:23 +0200 >Subject: [PATCH 4/4] sftp-client: RESOURCE_LEAK (CWE-772) > >openssh-7.7p1/sftp-client.c:1504: overwrite_var: Overwriting "new_dst" in "new_dst = path_append(dst, filename)" leaks the storage that "new_dst" points to. > 1502| filename = dir_entries[i]->filename; > 1503| > 1504|-> new_dst = path_append(dst, filename); > 1505| new_src = path_append(src, filename); > 1506| >--- > sftp-client.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > >diff --git a/sftp-client.c b/sftp-client.c >index be4fd621..0e0a5d21 100644 >--- a/sftp-client.c >+++ b/sftp-client.c >@@ -1501,13 +1501,16 @@ download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst, > for (i = 0; dir_entries[i] != NULL && !interrupted; i++) { > filename = dir_entries[i]->filename; > >+ /* skip special directories */ >+ if (S_ISDIR(dir_entries[i]->a.perm)) >+ if (strcmp(filename, ".") == 0 || >+ strcmp(filename, "..") == 0) >+ continue; >+ > new_dst = path_append(dst, filename); > new_src = path_append(src, filename); > > if (S_ISDIR(dir_entries[i]->a.perm)) { >- if (strcmp(filename, ".") == 0 || >- strcmp(filename, "..") == 0) >- continue; > if (download_dir_internal(conn, new_src, new_dst, > depth + 1, &(dir_entries[i]->a), preserve_flag, > print_flag, resume_flag, fsync_flag) == -1) >@@ -1842,9 +1845,15 @@ upload_dir_internal(struct sftp_conn *conn, const char *src, const char *dst, > } > > while (((dp = readdir(dirp)) != NULL) && !interrupted) { >+ filename = dp->d_name; >+ > if (dp->d_ino == 0) > continue; >- filename = dp->d_name; >+ if (S_ISDIR(sb.st_mode)) >+ if (strcmp(filename, ".") == 0 || >+ strcmp(filename, "..") == 0) >+ continue; >+ > new_dst = path_append(dst, filename); > new_src = path_append(src, filename); > >@@ -1853,10 +1862,6 @@ upload_dir_internal(struct sftp_conn *conn, const char *src, const char *dst, > strerror(errno)); > ret = -1; > } else if (S_ISDIR(sb.st_mode)) { >- if (strcmp(filename, ".") == 0 || >- strcmp(filename, "..") == 0) >- continue; >- > if (upload_dir_internal(conn, new_src, new_dst, > depth + 1, preserve_flag, print_flag, resume, > fsync_flag) == -1) >-- >2.17.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 2883
: 3164 |
3166