double free in close_internal_rpc_pipe_hnd

[Jeremy Allison]

On Mon, Feb 20, 2006 at 10:51:07AM -0600, Gerald (Jerry) Carter wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> James Peach wrote:
> > Hi Jerry,
> > 
> > FYI, I just came across a crash in top-of-tree. The corresponding
> > talloc_free in src_pipe_hnd.c was introduced in r13316 ... 
> 
> r13316 was the sync from trunk.  I'll look but I doubt that the
> bug is present in SAMBA_3_0_RELEASE.
> 
> 
> > Index: SAMBA_3_0/source/rpc_server/srv_pipe.c
> > ===================================================================
> > --- SAMBA_3_0/source/rpc_server/srv_pipe.c	(revision 13563)
> > +++ SAMBA_3_0/source/rpc_server/srv_pipe.c	(working copy)
> > @@ -617,6 +617,7 @@
> >  
> >  	/* Set up for non-authenticated user. */
> >  	talloc_free(p->pipe_user.nt_user_token);
> > +	p->pipe_user.nt_user_token = NULL;
> >  	p->pipe_user.ut.ngroups = 0;
> >  	SAFE_FREE( p->pipe_user.ut.groups);
> 
> Looks right to me.  This is exactly why I use the
> TALLOC_FREE() macro which resets the point to NULL after
> calling talloc_free().

I don't understand this. The nt_user_token is created at line 352
with :

    352                 p->pipe_user.nt_user_token = dup_nt_token(
    353                         NULL, vuser->nt_user_token);

That's a NULL talloc context, meaning it's being taken as a top
level talloc context. Ah - I've found it. The bug is actually
in pipe_ntlmssp_verify_final(), where pipe_user.nt_user_token
can be freed but not zeroed, and an error path can leave it
non-zero.

I think the correct fix is in pipe_ntlmssp_verify_final() to
use TALLOC_FREE() and also to use TALLOC_FREE() in rpc_server/srv_pipe.c.

I'll commit this.

Jeremy.