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>
(cherry picked from commit a1f95ba5db6fc017fad35377fbf76c048f2dd8ab)

diff --git a/source3/winbindd/idmap_script.c b/source3/winbindd/idmap_script.c
index 7b7f8844c36c7301ed0948225dc73268f6a72045..5f4f96cd768cd919a2d95f74da27adfa03a7d95d 100644 (file)
--- a/source3/winbindd/idmap_script.c
+++ b/source3/winbindd/idmap_script.c
@@ -583,6 +583,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__));
 
@@ -593,7 +594,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);
@@ -602,13 +603,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 24654731a480c041dc2f6e2b5694fde987c468e9..b52571cdb34589a9b2986b4a1ea57e95e3b2da9d 100644 (file)
--- a/source3/winbindd/idmap_tdb2.c
+++ b/source3/winbindd/idmap_tdb2.c
@@ -534,6 +534,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) {
@@ -555,7 +556,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) {
@@ -563,13 +564,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;