[PATCH] Fix several memory and resource leaks
Andreas Schneider
asn at samba.org
Thu Aug 9 14:53:58 UTC 2018
Hi,
please review the attached patch and push if OK.
Thanks,
Andreas
--
Andreas Schneider asn at samba.org
Samba Team www.samba.org
GPG-ID: 8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D
-------------- next part --------------
>From 569a759a415fd29569b592c4debb90bfa3c894e5 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 9 Aug 2018 15:48:48 +0200
Subject: [PATCH 01/10] libcli: Fix a possible memory leak in
smb1cli_req_writev_submit()
Found by covscan.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13567
Pair-Programmed-With: Justin Stephenson <jstephen at redhat.com>
Signed-off-by: Andreas Schneider <asn at samba.org>
Signed-off-by: Justin Stephenson <jstephen at redhat.com>
---
libcli/smb/smbXcli_base.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
index ad1b67b8476..4023d0c0f13 100644
--- a/libcli/smb/smbXcli_base.c
+++ b/libcli/smb/smbXcli_base.c
@@ -1748,6 +1748,7 @@ static NTSTATUS smb1cli_req_writev_submit(struct tevent_req *req,
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0, ("Error in encrypting client message: %s\n",
nt_errstr(status)));
+ SAFE_FREE(enc_buf);
return status;
}
buf = (char *)talloc_memdup(state, enc_buf,
--
2.18.0
>From efbb4934e5156a3b730d7ebd7996c87e8450e7c6 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 9 Aug 2018 15:53:45 +0200
Subject: [PATCH 02/10] wbinfo: Free memory when we leave wbinfo_dsgetdcname()
Found by covscan.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13567
Pair-Programmed-With: Justin Stephenson <jstephen at redhat.com>
Signed-off-by: Andreas Schneider <asn at samba.org>
Signed-off-by: Justin Stephenson <jstephen at redhat.com>
---
nsswitch/wbinfo.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/nsswitch/wbinfo.c b/nsswitch/wbinfo.c
index 1b58c73602a..c456f6eb329 100644
--- a/nsswitch/wbinfo.c
+++ b/nsswitch/wbinfo.c
@@ -747,6 +747,9 @@ static bool wbinfo_dsgetdcname(const char *domain_name, uint32_t flags)
d_printf("%s\n", dc_info->dc_site_name);
d_printf("%s\n", dc_info->client_site_name);
+ wbcFreeMemory(str);
+ wbcFreeMemory(dc_info);
+
return true;
}
--
2.18.0
>From 91bfe0c2fef41532ebbd1f0542fc58418a92bc20 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 9 Aug 2018 15:58:32 +0200
Subject: [PATCH 03/10] s3:client: Avoid a possible fd leak in do_get()
Found by covscan.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13567
Pair-Programmed-With: Justin Stephenson <jstephen at redhat.com>
Signed-off-by: Andreas Schneider <asn at samba.org>
Signed-off-by: Justin Stephenson <jstephen at redhat.com>
---
source3/client/client.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/source3/client/client.c b/source3/client/client.c
index f112b8c4ac1..c1e55739b8d 100644
--- a/source3/client/client.c
+++ b/source3/client/client.c
@@ -1160,6 +1160,7 @@ static int do_get(const char *rname, const char *lname_in, bool reget)
start = lseek(handle, 0, SEEK_END);
if (start == -1) {
d_printf("Error seeking local file\n");
+ close(handle);
return 1;
}
}
@@ -1181,6 +1182,9 @@ static int do_get(const char *rname, const char *lname_in, bool reget)
NULL);
if(!NT_STATUS_IS_OK(status)) {
d_printf("getattrib: %s\n", nt_errstr(status));
+ if (newhandle) {
+ close(handle);
+ }
return 1;
}
}
--
2.18.0
>From fa70afa4fd59fde977e2e1e4d0126a988104967a Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 9 Aug 2018 16:02:16 +0200
Subject: [PATCH 04/10] s3:libads: Fix memory leak in ads_krb5_chg_password()
Found by covscan.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13567
Pair-Programmed-With: Justin Stephenson <jstephen at redhat.com>
Signed-off-by: Andreas Schneider <asn at samba.org>
Signed-off-by: Justin Stephenson <jstephen at redhat.com>
---
source3/libads/krb5_setpw.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/source3/libads/krb5_setpw.c b/source3/libads/krb5_setpw.c
index bc96ac603b1..5fbafdd93bb 100644
--- a/source3/libads/krb5_setpw.c
+++ b/source3/libads/krb5_setpw.c
@@ -224,6 +224,7 @@ static ADS_STATUS ads_krb5_chg_password(const char *kdc_host,
krb5_get_init_creds_opt_free(context, opts);
krb5_free_context(context);
free(realm);
+ SAFE_FREE(addr);
DEBUG(1,("ads_krb5_chg_password: asprintf fail\n"));
return ADS_ERROR_NT(NT_STATUS_NO_MEMORY);
}
@@ -234,6 +235,7 @@ static ADS_STATUS ads_krb5_chg_password(const char *kdc_host,
kerb_prompter, NULL,
0, chpw_princ, opts);
krb5_get_init_creds_opt_free(context, opts);
+ SAFE_FREE(addr);
SAFE_FREE(chpw_princ);
SAFE_FREE(password);
--
2.18.0
>From ad081c93f26226608542d969c1c79a330f2c0cf8 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 9 Aug 2018 16:05:41 +0200
Subject: [PATCH 05/10] s3:passdb: Don't leak memory on error in
fetch_ldap_pw()
Found by covscan.
A candidate to use tallac ...
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13567
Pair-Programmed-With: Justin Stephenson <jstephen at redhat.com>
Signed-off-by: Andreas Schneider <asn at samba.org>
Signed-off-by: Justin Stephenson <jstephen at redhat.com>
---
source3/passdb/secrets.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/source3/passdb/secrets.c b/source3/passdb/secrets.c
index 7533d6b842f..ce215b1f2b2 100644
--- a/source3/passdb/secrets.c
+++ b/source3/passdb/secrets.c
@@ -351,6 +351,8 @@ bool fetch_ldap_pw(char **dn, char** pw)
if (!old_style_key) {
DEBUG(0, ("fetch_ldap_pw: strdup failed!\n"));
+ SAFE_FREE(*pw);
+ SAFE_FREE(*dn);
return False;
}
@@ -361,6 +363,7 @@ bool fetch_ldap_pw(char **dn, char** pw)
if ((data == NULL) || (size < sizeof(old_style_pw))) {
DEBUG(0,("fetch_ldap_pw: neither ldap secret retrieved!\n"));
SAFE_FREE(old_style_key);
+ SAFE_FREE(*pw);
SAFE_FREE(*dn);
SAFE_FREE(data);
return False;
@@ -375,6 +378,7 @@ bool fetch_ldap_pw(char **dn, char** pw)
if (!secrets_store_ldap_pw(*dn, old_style_pw)) {
DEBUG(0,("fetch_ldap_pw: ldap secret could not be upgraded!\n"));
SAFE_FREE(old_style_key);
+ SAFE_FREE(*pw);
SAFE_FREE(*dn);
return False;
}
--
2.18.0
>From a0deeb606575b20c062ab299ba4e21a7a9c4d547 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 9 Aug 2018 16:15:10 +0200
Subject: [PATCH 06/10] s3:registry: Fix possible memory leak in
_reg_perfcount_multi_sz_from_tdb()
Found by covscan.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13567
Pair-Programmed-With: Justin Stephenson <jstephen at redhat.com>
Signed-off-by: Andreas Schneider <asn at samba.org>
Signed-off-by: Justin Stephenson <jstephen at redhat.com>
---
source3/registry/reg_perfcount.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/source3/registry/reg_perfcount.c b/source3/registry/reg_perfcount.c
index db4451ecdeb..a737d96d24e 100644
--- a/source3/registry/reg_perfcount.c
+++ b/source3/registry/reg_perfcount.c
@@ -168,6 +168,7 @@ static uint32_t _reg_perfcount_multi_sz_from_tdb(TDB_CONTEXT *tdb,
TDB_DATA kbuf, dbuf;
char temp[PERFCOUNT_MAX_LEN] = {0};
char *buf1 = *retbuf;
+ char *p = NULL;
uint32_t working_size = 0;
DATA_BLOB name_index, name;
bool ok;
@@ -192,6 +193,7 @@ static uint32_t _reg_perfcount_multi_sz_from_tdb(TDB_CONTEXT *tdb,
}
ok = push_reg_sz(talloc_tos(), &name_index, (const char *)kbuf.dptr);
if (!ok) {
+ SAFE_FREE(buf1);
buffer_size = 0;
return buffer_size;
}
@@ -199,16 +201,19 @@ static uint32_t _reg_perfcount_multi_sz_from_tdb(TDB_CONTEXT *tdb,
buffer_size += working_size;
/* Now encode the actual name */
working_size = (dbuf.dsize + 1)*sizeof(uint16_t);
- buf1 = (char *)SMB_REALLOC(buf1, buffer_size + working_size);
- if(!buf1) {
+ p = (char *)SMB_REALLOC(buf1, buffer_size + working_size);
+ if (p == NULL) {
+ SAFE_FREE(buf1);
buffer_size = 0;
return buffer_size;
}
+ buf1 = p;
memset(temp, 0, sizeof(temp));
memcpy(temp, dbuf.dptr, dbuf.dsize);
SAFE_FREE(dbuf.dptr);
ok = push_reg_sz(talloc_tos(), &name, temp);
if (!ok) {
+ SAFE_FREE(buf1);
buffer_size = 0;
return buffer_size;
}
--
2.18.0
>From 651f04e2b993c363b527ce7c05eb56cc3168c6bc Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 9 Aug 2018 16:19:48 +0200
Subject: [PATCH 07/10] s3:utils: Do not overflow the destination buffer in
net_idmap_restore()
Found by covsan.
error[invalidScanfFormatWidth]: Width 128 given in format string (no. 2)
is larger than destination buffer 'sid_string[128]', use %127s to
prevent overflowing it.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13567
Pair-Programmed-With: Justin Stephenson <jstephen at redhat.com>
Signed-off-by: Andreas Schneider <asn at samba.org>
Signed-off-by: Justin Stephenson <jstephen at redhat.com>
---
source3/utils/net_idmap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/utils/net_idmap.c b/source3/utils/net_idmap.c
index fee8121aa60..4f365662a71 100644
--- a/source3/utils/net_idmap.c
+++ b/source3/utils/net_idmap.c
@@ -417,14 +417,14 @@ static int net_idmap_restore(struct net_context *c, int argc, const char **argv)
if ( (len > 0) && (line[len-1] == '\n') )
line[len-1] = '\0';
- if (sscanf(line, "GID %lu %128s", &idval, sid_string) == 2)
+ if (sscanf(line, "GID %lu %127s", &idval, sid_string) == 2)
{
ret = net_idmap_store_id_mapping(db, ID_TYPE_GID,
idval, sid_string);
if (ret != 0) {
break;
}
- } else if (sscanf(line, "UID %lu %128s", &idval, sid_string) == 2)
+ } else if (sscanf(line, "UID %lu %127s", &idval, sid_string) == 2)
{
ret = net_idmap_store_id_mapping(db, ID_TYPE_UID,
idval, sid_string);
--
2.18.0
>From 650679d4beba6a4adec73eff9646b947aca74243 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 9 Aug 2018 16:30:03 +0200
Subject: [PATCH 08/10] s3:utils: Do not leak memory in new_user()
Found by covscan.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13567
Pair-Programmed-With: Justin Stephenson <jstephen at redhat.com>
Signed-off-by: Andreas Schneider <asn at samba.org>
Signed-off-by: Justin Stephenson <jstephen at redhat.com>
---
source3/utils/pdbedit.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/source3/utils/pdbedit.c b/source3/utils/pdbedit.c
index a353bae7c4e..5c947e2fbde 100644
--- a/source3/utils/pdbedit.c
+++ b/source3/utils/pdbedit.c
@@ -750,7 +750,7 @@ static int new_user(const char *username, const char *fullname,
NTSTATUS status;
struct dom_sid u_sid;
int flags;
- int ret;
+ int ret = -1;
tosctx = talloc_tos();
if (!tosctx) {
@@ -766,10 +766,14 @@ static int new_user(const char *username, const char *fullname,
}
pwd1 = get_pass( "new password:", stdin_get);
+ if (pwd1 == NULL) {
+ fprintf(stderr, "Failed to read passwords.\n");
+ goto done;
+ }
pwd2 = get_pass( "retype new password:", stdin_get);
- if (!pwd1 || !pwd2) {
+ if (pwd2 == NULL) {
fprintf(stderr, "Failed to read passwords.\n");
- return -1;
+ goto done;
}
ret = strcmp(pwd1, pwd2);
if (ret != 0) {
--
2.18.0
>From de9b811a5c386bd3d84956470889f6453f23826d Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 9 Aug 2018 16:38:49 +0200
Subject: [PATCH 09/10] s3:winbind: Fix memory leak in nss_init()
Found by covscan.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13567
Pair-Programmed-With: Justin Stephenson <jstephen at redhat.com>
Signed-off-by: Andreas Schneider <asn at samba.org>
Signed-off-by: Justin Stephenson <jstephen at redhat.com>
---
source3/winbindd/nss_info.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/source3/winbindd/nss_info.c b/source3/winbindd/nss_info.c
index 473b1a3b66e..9c07b967fae 100644
--- a/source3/winbindd/nss_info.c
+++ b/source3/winbindd/nss_info.c
@@ -158,7 +158,7 @@ static NTSTATUS nss_init(const char **nss_list)
NTSTATUS status;
static bool nss_initialized = false;
int i;
- char *backend, *domain;
+ char *backend = NULL, *domain = NULL;
struct nss_function_entry *nss_backend;
/* check for previous successful initializations */
@@ -179,6 +179,9 @@ static NTSTATUS nss_init(const char **nss_list)
as necessary) */
for ( i=0; nss_list && nss_list[i]; i++ ) {
+ /* Loop cleanup, to not leak on 'continue' */
+ SAFE_FREE(backend);
+ SAFE_FREE(domain);
if ( !parse_nss_parm(nss_list[i], &backend, &domain) ) {
DEBUG(0,("nss_init: failed to parse \"%s\"!\n",
@@ -233,15 +236,15 @@ static NTSTATUS nss_init(const char **nss_list)
status = nss_domain_list_add_domain(domain, nss_backend);
if (!NT_STATUS_IS_OK(status)) {
+ SAFE_FREE(backend);
+ SAFE_FREE(domain);
return status;
}
-
- /* cleanup */
-
- SAFE_FREE( backend );
- SAFE_FREE( domain );
}
+ SAFE_FREE(backend);
+ SAFE_FREE(domain);
+
if ( !nss_domain_list ) {
DEBUG(3,("nss_init: no nss backends configured. "
"Defaulting to \"template\".\n"));
--
2.18.0
>From 6c93412eb4f89f2197acdda96befa18710b3217f Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 9 Aug 2018 16:42:43 +0200
Subject: [PATCH 10/10] s4:lib: Fix a possible fd leak in gp_get_file()
Found by covscan.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13567
Pair-Programmed-With: Justin Stephenson <jstephen at redhat.com>
Signed-off-by: Andreas Schneider <asn at samba.org>
Signed-off-by: Justin Stephenson <jstephen at redhat.com>
---
source4/lib/policy/gp_filesys.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/source4/lib/policy/gp_filesys.c b/source4/lib/policy/gp_filesys.c
index d48fc9fd6b0..700c91dccca 100644
--- a/source4/lib/policy/gp_filesys.c
+++ b/source4/lib/policy/gp_filesys.c
@@ -224,11 +224,15 @@ static NTSTATUS gp_get_file (struct smbcli_tree *tree, const char *remote_src,
NT_STATUS_IS_ERR(smbcli_getattrE(tree, fh_remote,
&attr, &file_size, NULL, NULL, NULL))) {
DEBUG(0, ("Failed to get remote file size: %s\n", smbcli_errstr(tree)));
+ close(fh_local);
return NT_STATUS_UNSUCCESSFUL;
}
buf = talloc_zero_array(tree, uint8_t, buf_size);
- NT_STATUS_HAVE_NO_MEMORY(buf);
+ if (buf == NULL) {
+ close(fh_local);
+ return NT_STATUS_NO_MEMORY;
+ }
/* Copy the contents of the file */
while (1) {
@@ -241,6 +245,7 @@ static NTSTATUS gp_get_file (struct smbcli_tree *tree, const char *remote_src,
if (write(fh_local, buf, n) != n) {
DEBUG(0, ("Short write while copying file.\n"));
talloc_free(buf);
+ close(fh_local);
return NT_STATUS_UNSUCCESSFUL;
}
nread += n;
--
2.18.0
More information about the samba-technical
mailing list