[PATCH] pdb_ldap: Don't use autofree if "mods" still changes

[Jeremy Allison]

On Tue, Apr 19, 2016 at 05:16:03PM +0200, Volker Lendecke wrote:
> Hi!
> 
> Review appreciated!

Oh, good catch ! The code there is *horrible* :-).
Are there any other places we do the same mistake ?

Pushed !

> -- 
> SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> phone: +49-551-370000-0, fax: +49-551-370000-9
> AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
> http://www.sernet.de, mailto:kontakt at sernet.de

> From 7cc7b3e575381e1ff83660332cc4f3ef0d2bcd1e Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Fri, 4 Mar 2016 10:51:33 +0100
> Subject: [PATCH] pdb_ldap: Don't use autofree if "mods" still changes
> 
> This will prevent some use-after-free's, potentially it might for example fix
> bugzilla 11851. Not directly related, but it's a crash related to ldap-backed
> user creation.
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source3/passdb/pdb_ldap.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/source3/passdb/pdb_ldap.c b/source3/passdb/pdb_ldap.c
> index 50ab3a7..4383785 100644
> --- a/source3/passdb/pdb_ldap.c
> +++ b/source3/passdb/pdb_ldap.c
> @@ -5291,10 +5291,10 @@ static NTSTATUS ldapsam_create_user(struct pdb_methods *my_methods,
>  	}
>  
>  	init_okay = init_ldap_from_sam(ldap_state, entry, &mods, user, pdb_element_is_set_or_changed);
> -	smbldap_talloc_autofree_ldapmod(tmp_ctx, mods);
>  
>  	if (!init_okay) {
>  		DEBUG(1,("ldapsam_create_user: Unable to fill user structs\n"));
> +		ldap_mods_free(mods, true);
>  		return NT_STATUS_UNSUCCESSFUL;
>  	}
>  
> @@ -5312,12 +5312,14 @@ static NTSTATUS ldapsam_create_user(struct pdb_methods *my_methods,
>  		if (!sid_compose(&group_sid, get_global_sam_sid(), DOMAIN_RID_USERS) ||
>  		    !sid_to_gid(&group_sid, &gid)) {
>  			DEBUG (0, ("ldapsam_create_user: Unable to get the Domain Users gid: bailing out!\n"));
> +			ldap_mods_free(mods, true);
>  			return NT_STATUS_INVALID_PRIMARY_GROUP;
>  		}
>  
>  		/* lets allocate a new userid for this user */
>  		if (!winbind_allocate_uid(&uid)) {
>  			DEBUG (0, ("ldapsam_create_user: Unable to allocate a new user id: bailing out!\n"));
> +			ldap_mods_free(mods, true);
>  			return NT_STATUS_UNSUCCESSFUL;
>  		}
>  
> @@ -5354,6 +5356,7 @@ static NTSTATUS ldapsam_create_user(struct pdb_methods *my_methods,
>  		escape_name = escape_rdn_val_string_alloc(name);
>  		if (!escape_name) {
>  			DEBUG (0, ("ldapsam_create_user: Out of memory!\n"));
> +			ldap_mods_free(mods, true);
>  			return NT_STATUS_NO_MEMORY;
>  		}
>  
> @@ -5367,6 +5370,7 @@ static NTSTATUS ldapsam_create_user(struct pdb_methods *my_methods,
>  
>  		if (!homedir || !shell || !uidstr || !gidstr || !dn) {
>  			DEBUG (0, ("ldapsam_create_user: Out of memory!\n"));
> +			ldap_mods_free(mods, true);
>  			return NT_STATUS_NO_MEMORY;
>  		}
>  
> @@ -5385,6 +5389,8 @@ static NTSTATUS ldapsam_create_user(struct pdb_methods *my_methods,
>  		rc = smbldap_modify(ldap_state->smbldap_state, dn, mods);
>  	}	
>  
> +	ldap_mods_free(mods, true);
> +
>  	if (rc != LDAP_SUCCESS) {
>  		DEBUG(0,("ldapsam_create_user: failed to create a new user [%s] (dn = %s)\n", name ,dn));
>  		return NT_STATUS_UNSUCCESSFUL;
> -- 
> 2.1.4
>