[PATCH 2/6] gpo: don't leak cache_path onto talloc tos
David Disseldorp
ddiss at samba.org
Mon Oct 6 10:21:14 MDT 2014
Also check for allocation failures.
Reported-by: Franz Pförtsch <franz.pfoertsch at brose.com>
Signed-off-by: David Disseldorp <ddiss at samba.org>
---
source3/libgpo/gpext/registry.c | 20 +++++++++++++++-----
source3/libgpo/gpext/scripts.c | 24 +++++++++++++++++-------
source3/libgpo/gpext/security.c | 7 ++++++-
source3/utils/net_ads_gpo.c | 16 ++++++++++++----
4 files changed, 50 insertions(+), 17 deletions(-)
diff --git a/source3/libgpo/gpext/registry.c b/source3/libgpo/gpext/registry.c
index b51bc30..a24485c 100644
--- a/source3/libgpo/gpext/registry.c
+++ b/source3/libgpo/gpext/registry.c
@@ -287,6 +287,10 @@ static NTSTATUS registry_process_group_policy(TALLOC_CTX *mem_ctx,
size_t num_entries = 0;
char *unix_path = NULL;
const struct GROUP_POLICY_OBJECT *gpo;
+ char *gpo_cache_path = cache_path(GPO_CACHE_DIR);
+ if (gpo_cache_path == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
/* implementation of the policy callback function, see
* http://msdn.microsoft.com/en-us/library/aa373494%28v=vs.85%29.aspx
@@ -304,9 +308,11 @@ static NTSTATUS registry_process_group_policy(TALLOC_CTX *mem_ctx,
gpext_debug_header(0, "registry_process_group_policy", flags,
gpo, GP_EXT_GUID_REGISTRY, NULL);
- status = gpo_get_unix_path(mem_ctx, cache_path(GPO_CACHE_DIR),
+ status = gpo_get_unix_path(mem_ctx, gpo_cache_path,
gpo, &unix_path);
- NT_STATUS_NOT_OK_RETURN(status);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto err_cache_path_free;
+ }
status = reg_parse_registry(mem_ctx,
flags,
@@ -316,7 +322,7 @@ static NTSTATUS registry_process_group_policy(TALLOC_CTX *mem_ctx,
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0,("failed to parse registry: %s\n",
nt_errstr(status)));
- return status;
+ goto err_cache_path_free;
}
dump_reg_entries(flags, "READ", entries, num_entries);
@@ -326,11 +332,15 @@ static NTSTATUS registry_process_group_policy(TALLOC_CTX *mem_ctx,
if (!W_ERROR_IS_OK(werr)) {
DEBUG(0,("failed to apply registry: %s\n",
win_errstr(werr)));
- return werror_to_ntstatus(werr);
+ status = werror_to_ntstatus(werr);
+ goto err_cache_path_free;
}
}
+ status = NT_STATUS_OK;
- return NT_STATUS_OK;
+err_cache_path_free:
+ talloc_free(gpo_cache_path);
+ return status;
}
/****************************************************************
diff --git a/source3/libgpo/gpext/scripts.c b/source3/libgpo/gpext/scripts.c
index e2841c0..da6f5cc 100644
--- a/source3/libgpo/gpext/scripts.c
+++ b/source3/libgpo/gpext/scripts.c
@@ -357,6 +357,10 @@ static NTSTATUS scripts_process_group_policy(TALLOC_CTX *mem_ctx,
GP_SCRIPTS_INI_LOGOFF
};
const struct GROUP_POLICY_OBJECT *gpo;
+ char *gpo_cache_path = cache_path(GPO_CACHE_DIR);
+ if (gpo_cache_path == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
/* implementation of the policy callback function, see
* http://msdn.microsoft.com/en-us/library/aa373494%28v=vs.85%29.aspx
@@ -374,13 +378,17 @@ static NTSTATUS scripts_process_group_policy(TALLOC_CTX *mem_ctx,
gpext_debug_header(0, "scripts_process_group_policy", flags,
gpo, GP_EXT_GUID_SCRIPTS, NULL);
- status = gpo_get_unix_path(mem_ctx, cache_path(GPO_CACHE_DIR),
+ status = gpo_get_unix_path(mem_ctx, gpo_cache_path,
gpo, &unix_path);
- NT_STATUS_NOT_OK_RETURN(status);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto err_cache_path_free;
+ }
status = gp_inifile_init_context(mem_ctx, flags, unix_path,
GP_SCRIPTS_INI, &ini_ctx);
- NT_STATUS_NOT_OK_RETURN(status);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto err_cache_path_free;
+ }
for (i = 0; i < ARRAY_SIZE(list); i++) {
@@ -394,7 +402,8 @@ static NTSTATUS scripts_process_group_policy(TALLOC_CTX *mem_ctx,
}
if (!NT_STATUS_IS_OK(status)) {
- return status;
+ TALLOC_FREE(ini_ctx);
+ goto err_cache_path_free;
}
dump_reg_entries(flags, "READ", entries, num_entries);
@@ -403,15 +412,16 @@ static NTSTATUS scripts_process_group_policy(TALLOC_CTX *mem_ctx,
flags, list[i], gpo, entries, num_entries);
if (!W_ERROR_IS_OK(werr)) {
continue; /* FIXME: finally fix storing emtpy strings and REG_QWORD! */
- TALLOC_FREE(ini_ctx);
- return werror_to_ntstatus(werr);
}
}
TALLOC_FREE(ini_ctx);
}
+ status = NT_STATUS_OK;
- return NT_STATUS_OK;
+err_cache_path_free:
+ talloc_free(gpo_cache_path);
+ return status;
}
/****************************************************************
diff --git a/source3/libgpo/gpext/security.c b/source3/libgpo/gpext/security.c
index 5360222..2f46184 100644
--- a/source3/libgpo/gpext/security.c
+++ b/source3/libgpo/gpext/security.c
@@ -152,6 +152,10 @@ static NTSTATUS security_process_group_policy(TALLOC_CTX *mem_ctx,
char *unix_path = NULL;
struct gp_inifile_context *ini_ctx = NULL;
const struct GROUP_POLICY_OBJECT *gpo;
+ char *gpo_cache_path = cache_path(GPO_CACHE_DIR);
+ if (gpo_cache_path == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
/* implementation of the policy callback function, see
* http://msdn.microsoft.com/en-us/library/aa373494%28v=vs.85%29.aspx
@@ -172,7 +176,7 @@ static NTSTATUS security_process_group_policy(TALLOC_CTX *mem_ctx,
/* this handler processes the gpttmpl files and merge output to the
* registry */
- status = gpo_get_unix_path(mem_ctx, cache_path(GPO_CACHE_DIR),
+ status = gpo_get_unix_path(mem_ctx, gpo_cache_path,
gpo, &unix_path);
if (!NT_STATUS_IS_OK(status)) {
goto out;
@@ -198,6 +202,7 @@ static NTSTATUS security_process_group_policy(TALLOC_CTX *mem_ctx,
nt_errstr(status)));
}
TALLOC_FREE(ini_ctx);
+ talloc_free(gpo_cache_path);
return status;
}
diff --git a/source3/utils/net_ads_gpo.c b/source3/utils/net_ads_gpo.c
index 79793b8..8b789e5 100644
--- a/source3/utils/net_ads_gpo.c
+++ b/source3/utils/net_ads_gpo.c
@@ -39,6 +39,7 @@ static int net_ads_gpo_refresh(struct net_context *c, int argc, const char **arg
struct GROUP_POLICY_OBJECT *gpo;
NTSTATUS result;
struct security_token *token = NULL;
+ char *gpo_cache_path;
if (argc < 1 || c->display_usage) {
d_printf("%s\n%s\n%s",
@@ -99,10 +100,17 @@ static int net_ads_gpo_refresh(struct net_context *c, int argc, const char **arg
d_printf(_("finished\n"));
d_printf(_("* Refreshing Group Policy Data "));
- if (!NT_STATUS_IS_OK(result = check_refresh_gpo_list(ads, mem_ctx,
- cache_path(GPO_CACHE_DIR),
- flags,
- gpo_list))) {
+ gpo_cache_path = cache_path(GPO_CACHE_DIR);
+ if (gpo_cache_path == NULL) {
+ d_printf(_("failed: %s\n"), nt_errstr(NT_STATUS_NO_MEMORY));
+ goto out;
+ }
+ result = check_refresh_gpo_list(ads, mem_ctx,
+ gpo_cache_path,
+ flags,
+ gpo_list);
+ TALLOC_FREE(gpo_cache_path);
+ if (!NT_STATUS_IS_OK(result)) {
d_printf(_("failed: %s\n"), nt_errstr(result));
goto out;
}
--
1.8.4.5
More information about the samba-technical
mailing list