From 8b091b1d0ca4a51a92644f5b1951acfd67752da7 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 9 Aug 2018 15:53:45 +0200 Subject: [PATCH 01/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 Signed-off-by: Andreas Schneider Signed-off-by: Justin Stephenson Reviewed-by: Volker Lendecke Reviewed-by: Jeremy Allison (cherry picked from commit e6689c3e14c2dfaebaf1109f21e53184fea45d41) --- 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 4bc7044e94f8ac2aa570e6544fcfe824fe981326 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 9 Aug 2018 16:05:41 +0200 Subject: [PATCH 02/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 Signed-off-by: Andreas Schneider Signed-off-by: Justin Stephenson Reviewed-by: Volker Lendecke Reviewed-by: Jeremy Allison (cherry picked from commit e4f4f5eb7303a0cce4f426dd9cfd1d6a488495b0) --- 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 fc49500a9be4f7f350243ec479876c92ee2265d4 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 9 Aug 2018 16:19:48 +0200 Subject: [PATCH 03/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 Signed-off-by: Andreas Schneider Signed-off-by: Justin Stephenson Reviewed-by: Volker Lendecke Reviewed-by: Jeremy Allison (cherry picked from commit f20150fb1ea5292f099862af6268d06844954d5e) --- 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 dc60aaa5e246f6cbd8cd4c38bc72ca8000d4f9f9 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 9 Aug 2018 16:30:03 +0200 Subject: [PATCH 04/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 Signed-off-by: Andreas Schneider Signed-off-by: Justin Stephenson Reviewed-by: Volker Lendecke Reviewed-by: Jeremy Allison (cherry picked from commit b7b4fc51d0eadbbc94576dda75ae80098a205a24) --- 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 827ffa50c93805a8398ccc6f4d13afd905b1d2ff Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 9 Aug 2018 16:42:43 +0200 Subject: [PATCH 05/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 Signed-off-by: Andreas Schneider Signed-off-by: Justin Stephenson Reviewed-by: Jeremy Allison (cherry picked from commit d4fb124adfc10de8b7eb1f72b74d7ca83f8415dd) --- source4/lib/policy/gp_filesys.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/source4/lib/policy/gp_filesys.c b/source4/lib/policy/gp_filesys.c index d48fc9fd6b0..267762dd27d 100644 --- a/source4/lib/policy/gp_filesys.c +++ b/source4/lib/policy/gp_filesys.c @@ -215,6 +215,7 @@ static NTSTATUS gp_get_file (struct smbcli_tree *tree, const char *remote_src, fh_local = open(local_dst, O_WRONLY | O_CREAT | O_TRUNC, 0644); if (fh_local == -1) { DEBUG(0, ("Failed to open local file: %s\n", local_dst)); + smbcli_close(tree, fh_remote); return NT_STATUS_UNSUCCESSFUL; } @@ -224,11 +225,17 @@ 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))); + smbcli_close(tree, fh_remote); + 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) { + smbcli_close(tree, fh_remote); + close(fh_local); + return NT_STATUS_NO_MEMORY; + } /* Copy the contents of the file */ while (1) { @@ -240,27 +247,28 @@ 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")); + smbcli_close(tree, fh_remote); + close(fh_local); talloc_free(buf); return NT_STATUS_UNSUCCESSFUL; } nread += n; } + /* Close the files */ + smbcli_close(tree, fh_remote); + close(fh_local); + + talloc_free(buf); + /* Bytes read should match the file size, or the copy was incomplete */ if (nread != file_size) { DEBUG(0, ("Remote/local file size mismatch after copying file: " "%s (remote %zu, local %zu).\n", remote_src, file_size, nread)); - close(fh_local); - talloc_free(buf); return NT_STATUS_UNSUCCESSFUL; } - /* Close the files */ - smbcli_close(tree, fh_remote); - close(fh_local); - - talloc_free(buf); return NT_STATUS_OK; } -- 2.18.0 From a392359ea69ae6bd8c9b0bc189f48973a432102c Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 9 Aug 2018 15:58:32 +0200 Subject: [PATCH 06/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 Signed-off-by: Andreas Schneider Signed-off-by: Justin Stephenson Reviewed-by: Jeremy Allison (cherry picked from commit 3d32c0263b072e19335eba1451840284409ecb61) --- source3/client/client.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source3/client/client.c b/source3/client/client.c index f112b8c4ac1..25ba01d6216 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; } } @@ -1193,6 +1197,9 @@ static int do_get(const char *rname, const char *lname_in, bool reget) if (!NT_STATUS_IS_OK(status)) { d_fprintf(stderr, "parallel_read returned %s\n", nt_errstr(status)); + if (newhandle) { + close(handle); + } cli_close(targetcli, fnum); return 1; } -- 2.18.0 From d09836ecc88b1e3f03fb059f2010ecda43084905 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 9 Aug 2018 16:02:16 +0200 Subject: [PATCH 07/10] s3:libads: Fix memory leaks in ads_krb5_chg_password() Found by covscan. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13567 Pair-Programmed-With: Justin Stephenson Signed-off-by: Andreas Schneider Signed-off-by: Justin Stephenson Reviewed-by: Jeremy Allison (cherry picked from commit dbdbd4875ecac3e7334750f46f1f494b7afe6628) --- 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..0418fec5ad3 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); + smb_krb5_free_addresses(context, 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); + smb_krb5_free_addresses(context, addr); SAFE_FREE(chpw_princ); SAFE_FREE(password); -- 2.18.0 From a94d8938c1f1ff3613242f61df74fa44f9421ec6 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 9 Aug 2018 16:15:10 +0200 Subject: [PATCH 08/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 Signed-off-by: Andreas Schneider Signed-off-by: Justin Stephenson Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Sat Aug 11 04:43:15 CEST 2018 on sn-devel-144 (cherry picked from commit 3e6ce5c6e679fdb39ed8142bf5e1ed4105164826) --- source3/registry/reg_perfcount.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/source3/registry/reg_perfcount.c b/source3/registry/reg_perfcount.c index db4451ecdeb..e31f8991642 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; @@ -185,13 +186,16 @@ static uint32_t _reg_perfcount_multi_sz_from_tdb(TDB_CONTEXT *tdb, } /* First encode the name_index */ working_size = (kbuf.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; 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 +203,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 78c7489bd7d2cc06f6bada7fc9d9d7c24d1e0416 Mon Sep 17 00:00:00 2001 From: Andreas Schneider 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 Signed-off-by: Andreas Schneider Signed-off-by: Justin Stephenson Reviewed-by: Jeremy Allison (cherry picked from commit 4c0b49b3f982a3a3013a3b6fef3c10b1ca7d2ab0) --- source3/winbindd/nss_info.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/source3/winbindd/nss_info.c b/source3/winbindd/nss_info.c index 473b1a3b66e..1a8325ce7dc 100644 --- a/source3/winbindd/nss_info.c +++ b/source3/winbindd/nss_info.c @@ -84,7 +84,10 @@ static struct nss_function_entry *nss_get_backend(const char *name ) /******************************************************************** *******************************************************************/ -static bool parse_nss_parm( const char *config, char **backend, char **domain ) +static bool parse_nss_parm(TALLOC_CTX *mem_ctx, + const char *config, + char **backend, + char **domain) { char *p; @@ -98,17 +101,17 @@ static bool parse_nss_parm( const char *config, char **backend, char **domain ) /* if no : then the string must be the backend name only */ if ( !p ) { - *backend = SMB_STRDUP( config ); + *backend = talloc_strdup(mem_ctx, config); return (*backend != NULL); } /* split the string and return the two parts */ if ( strlen(p+1) > 0 ) { - *domain = SMB_STRDUP( p+1 ); + *domain = talloc_strdup(mem_ctx, p + 1); } - *backend = SMB_STRNDUP(config, PTR_DIFF(p, config)); + *backend = talloc_strndup(mem_ctx, config, PTR_DIFF(p, config)); return (*backend != NULL); } @@ -158,8 +161,9 @@ 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; + TALLOC_CTX *frame; /* check for previous successful initializations */ @@ -167,6 +171,8 @@ static NTSTATUS nss_init(const char **nss_list) return NT_STATUS_OK; } + frame = talloc_stackframe(); + /* The "template" backend should always be registered as it is a static module */ @@ -179,8 +185,10 @@ static NTSTATUS nss_init(const char **nss_list) as necessary) */ for ( i=0; nss_list && nss_list[i]; i++ ) { + bool ok; - if ( !parse_nss_parm(nss_list[i], &backend, &domain) ) { + ok = parse_nss_parm(frame, nss_list[i], &backend, &domain); + if (!ok) { DEBUG(0,("nss_init: failed to parse \"%s\"!\n", nss_list[i])); continue; @@ -238,10 +246,11 @@ static NTSTATUS nss_init(const char **nss_list) /* cleanup */ - SAFE_FREE( backend ); - SAFE_FREE( domain ); + TALLOC_FREE(domain); + TALLOC_FREE(backend); } + if ( !nss_domain_list ) { DEBUG(3,("nss_init: no nss backends configured. " "Defaulting to \"template\".\n")); @@ -252,6 +261,7 @@ static NTSTATUS nss_init(const char **nss_list) nss_initialized = true; + TALLOC_FREE(frame); return NT_STATUS_OK; } -- 2.18.0 From abd3d3f8c43204158ccf06f086f533bdc6c1d487 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Tue, 14 Aug 2018 18:55:33 +0200 Subject: [PATCH 10/10] s3:libads: Free addr before we free the context Introduced by dbdbd4875ecac3e7334750f46f1f494b7afe6628 CID 1438395 BUG: https://bugzilla.samba.org/show_bug.cgi?id=13567 Signed-off-by: Andreas Schneider Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Tue Aug 14 22:02:06 CEST 2018 on sn-devel-144 (cherry picked from commit 9eccf6a16f5b198181a4fa80b835b1a65b40ed76) --- source3/libads/krb5_setpw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/libads/krb5_setpw.c b/source3/libads/krb5_setpw.c index 0418fec5ad3..8f9098853b9 100644 --- a/source3/libads/krb5_setpw.c +++ b/source3/libads/krb5_setpw.c @@ -222,9 +222,9 @@ static ADS_STATUS ads_krb5_chg_password(const char *kdc_host, /* We have to obtain an INITIAL changepw ticket for changing password */ if (asprintf(&chpw_princ, "kadmin/changepw@%s", realm) == -1) { krb5_get_init_creds_opt_free(context, opts); + smb_krb5_free_addresses(context, addr); krb5_free_context(context); free(realm); - smb_krb5_free_addresses(context, addr); DEBUG(1,("ads_krb5_chg_password: asprintf fail\n")); return ADS_ERROR_NT(NT_STATUS_NO_MEMORY); } -- 2.18.0