Patch: Attempt to fix segfaults in push_ucs2_talloc when domain, workgroup etc are NULL in creds

Jeremy Allison jra at samba.org
Mon Aug 24 23:12:51 UTC 2015


On Tue, Aug 18, 2015 at 09:32:17AM -0700, Richard Sharpe wrote:
>     options=0x0) at ../common/ldb.c:260
> #13 0x00007fffee7a0995 in py_ldb_connect (self=<value optimized out>,
>     args=<value optimized out>, kwargs=<value optimized out>)
>     at ../pyldb.c:1071
> #14 0x00007ffff7b0c9e4 in PyEval_EvalFrameEx ()
>    from /usr/lib64/libpython2.6.so.1.0
> #15 0x00007ffff7b0e657 in PyEval_EvalCodeEx ()
>    from /usr/lib64/libpython2.6.so.1.0
> 
> Here is a possible fix for it. It is caused because I did not set the
> workgroup or workstation parts of the creds:
> 
> ----------------------------------------
> diff --git a/auth/ntlmssp/ntlmssp_client.c b/auth/ntlmssp/ntlmssp_client.c
> index f99257d..1f22241 100644
> --- a/auth/ntlmssp/ntlmssp_client.c
> +++ b/auth/ntlmssp/ntlmssp_client.c
> @@ -146,7 +146,7 @@ NTSTATUS ntlmssp_client_challenge(struct gensec_security *ge
>         DATA_BLOB encrypted_session_key = data_blob(NULL, 0);
>         NTSTATUS nt_status;
>         int flags = 0;
> -       const char *user, *domain;
> +       const char *user, *domain, *workstation;
> 
>         TALLOC_CTX *mem_ctx = talloc_new(out_mem_ctx);
>         if (!mem_ctx) {
> @@ -224,6 +224,13 @@ NTSTATUS ntlmssp_client_challenge(struct gensec_security *g
>         cli_credentials_get_ntlm_username_domain(gensec_security->credentials, m
>                                                  &user, &domain);
> 
> +       workstation = cli_credentials_get_workstation(gensec_security->credentia
> +
> +       if (user = NULL || domain == NULL || workstation == NULL) {
> +               DEBUG(10, ("One or more of user, domain or workstation is NULL\n
> +               return NT_STATUS_INVALID_PARAMETER;
> +       }
> +
>         if (ntlmssp_state->neg_flags & NTLMSSP_NEGOTIATE_NTLM2) {
>                 flags |= CLI_CRED_NTLM2;
>         }
> @@ -305,7 +312,7 @@ NTSTATUS ntlmssp_client_challenge(struct gensec_security *ge
>                        nt_response.data, nt_response.length,
>                        domain,
>                        user,
> -                      cli_credentials_get_workstation(gensec_security->credenti
> +                      workstation,
>                        encrypted_session_key.data, encrypted_session_key.length,
>                        ntlmssp_state->neg_flags);
>         if (!NT_STATUS_IS_OK(nt_status)) {
> -------------------------------------
> 
> I imagine there will be comments ...

Actually this looks pretty good to me. A few changes.

1). Add your 'Signed-off-by'.
2). Initialize *user, *domain, *workstation all
to NULL in the declaration (yeah, strictly not
needed but we have so many uninitialized variable
bugs...).
3). Split (user = NULL || domain == NULL || workstation == NULL)
into individual checks I think. Make it at least easy for a
programmer to track down.

Cheers,

	Jeremy.



More information about the samba-technical mailing list