[PATCH] Fix several memory and resource leaks
Jeremy Allison
jra at samba.org
Thu Aug 9 19:01:31 UTC 2018
On Thu, Aug 09, 2018 at 11:16:08AM -0700, Jeremy Allison via samba-technical wrote:
>
> Ah, Looks like Volker also had some good comments.
>
> I won't push anything, I'll wait until you
> address both his and my comments (some of
> which were the same :-).
>
> Feel free to push the ones that both Volker
> and I reviewed without comment though :-).
Just to be helpful, these are the ones that
both Volker and I +1'ed (although you might
want to look at his comments about re-arranging
around the 'continue's in the last patch.
Cheers,
Jeremy
-------------- next part --------------
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>
Reviewed-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
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 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>
Reviewed-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
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 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>
Reviewed-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
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>
Reviewed-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
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>
Reviewed-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
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
More information about the samba-technical
mailing list