s3: winbind: Fix crash when invoking winbind idmap scripts.

Previously the private context was caching a pointer to
a string returned from lp_XXX(). This string can change
on config file reload. Ensure the string is talloc_strup'ed
onto the owning context instead.

Reported by Heinrich Mislik <Heinrich.Mislik@univie.ac.at>

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13956

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

2 files changed, 33 insertions, 9 deletions

diff --git a/source3/winbindd/idmap_script.c b/source3/winbindd/idmap_script.c
index 7ad6b806fb8..f382f896b35 100644
--- a/source3/winbindd/idmap_script.c
+++ b/source3/winbindd/idmap_script.c
@@ -615,6 +615,7 @@ static NTSTATUS idmap_script_db_init(struct idmap_domain *dom)
 	NTSTATUS ret;
 	struct idmap_script_context *ctx;
 	const char * idmap_script = NULL;
+	const char *ctx_script = NULL;
 
 	DEBUG(10, ("%s called ...\n", __func__));
 
@@ -625,7 +626,7 @@ static NTSTATUS idmap_script_db_init(struct idmap_domain *dom)
 		goto failed;
 	}
 
-	ctx->script = idmap_config_const_string(dom->name, "script", NULL);
+	ctx_script = idmap_config_const_string(dom->name, "script", NULL);
 
 	/* Do we even need to handle this? */
 	idmap_script = lp_parm_const_string(-1, "idmap", "script", NULL);
@@ -634,13 +635,24 @@ static NTSTATUS idmap_script_db_init(struct idmap_domain *dom)
 			  " Please use 'idmap config * : script' instead!\n"));
 	}
 
-	if (strequal(dom->name, "*") && ctx->script == NULL) {
+	if (strequal(dom->name, "*") && ctx_script == NULL) {
 		/* fall back to idmap:script for backwards compatibility */
-		ctx->script = idmap_script;
+		ctx_script = idmap_script;
 	}
 
-	if (ctx->script) {
+	if (ctx_script) {
 		DEBUG(1, ("using idmap script '%s'\n", ctx->script));
+		/*
+		 * We must ensure this memory is owned by ctx.
+		 * The ctx_script const pointer is a pointer into
+		 * the config file data and may become invalid
+		 * on config file reload. BUG: 13956
+		 */
+		ctx->script = talloc_strdup(ctx, ctx_script);
+		if (ctx->script == NULL) {
+			ret = NT_STATUS_NO_MEMORY;
+			goto failed;
+		}
 	}
 
 	dom->private_data = ctx;
diff --git a/source3/winbindd/idmap_tdb2.c b/source3/winbindd/idmap_tdb2.c
index b784546bb33..eceab9c0784 100644
--- a/source3/winbindd/idmap_tdb2.c
+++ b/source3/winbindd/idmap_tdb2.c
@@ -522,6 +522,7 @@ static NTSTATUS idmap_tdb2_db_init(struct idmap_domain *dom)
 	struct idmap_tdb_common_context *commonctx;
 	struct idmap_tdb2_context *ctx;
 	const char * idmap_script = NULL;
+	const char *ctx_script = NULL;
 
 	commonctx = talloc_zero(dom, struct idmap_tdb_common_context);
 	if(!commonctx) {
@@ -543,7 +544,7 @@ static NTSTATUS idmap_tdb2_db_init(struct idmap_domain *dom)
 		goto failed;
 	}
 
-	ctx->script = idmap_config_const_string(dom->name, "script", NULL);
+	ctx_script = idmap_config_const_string(dom->name, "script", NULL);
 
 	idmap_script = lp_parm_const_string(-1, "idmap", "script", NULL);
 	if (idmap_script != NULL) {
@@ -551,13 +552,24 @@ static NTSTATUS idmap_tdb2_db_init(struct idmap_domain *dom)
 			  " Please use 'idmap config * : script' instead!\n"));
 	}
 
-	if (strequal(dom->name, "*") && ctx->script == NULL) {
+	if (strequal(dom->name, "*") && ctx_script == NULL) {
 		/* fall back to idmap:script for backwards compatibility */
-		ctx->script = idmap_script;
+		ctx_script = idmap_script;
 	}
 
-	if (ctx->script) {
-		DEBUG(1, ("using idmap script '%s'\n", ctx->script));
+	if (ctx_script) {
+		DEBUG(1, ("using idmap script '%s'\n", ctx_script));
+		/*
+		 * We must ensure this memory is owned by ctx.
+		 * The ctx_script const pointer is a pointer into
+		 * the config file data and may become invalid
+		 * on config file reload. BUG: 13956
+		 */
+		ctx->script = talloc_strdup(ctx, ctx_script);
+		if (ctx->script == NULL) {
+			ret = NT_STATUS_NO_MEMORY;
+			goto failed;
+		}
 	}
 
 	commonctx->max_id = dom->high_id;