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>

diff --git a/source3/winbindd/idmap_script.c b/source3/winbindd/idmap_script.c
index 7ad6b806fb8ef238c387011a293b5317aafdde8c..f382f896b351eea5da661ac25255df4b7c2ec643 100644 (file)
--- 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 b784546bb330a05fded8e728e1a01eea01a03a67..eceab9c07840d24196de5c716cd278b799801064 100644 (file)
--- 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;