The Samba-Bugzilla – Bug 11367
use after free in source3/auth/auth_domain.c:domain_client_validate
Last modified: 2015-07-11 20:03:48 UTC
Created attachment 11208 [details]
I'm running Samba 4.2.2 on OmniOS r151014 and I find that when samba is joined to one of our domains (we use windows 2012r2 domain controllers) it can't authenticate users in that domain, failing with this error:
[2015/06/24 17:49:52.718039, 0, pid=3060, effective(0, 0), real(0, 0), class=auth] ../source3/auth/auth_domain.c:302(domain_client_validate)
domain_client_validate: unable to validate password for user xjono in domain LABS to Domain controller LAVENDER.LABS.EAIT.UQ.EDU.AU. Error was NT_STATUS_LOCK_NOT_GRANTED.
This turns out to be because the struct netlogon_creds_cli_context allocated in connect_to_domain_password_server is allocated in a talloc context that is freed at the end of that function. The memory allocator on OmniOS seems to reuse freed memory immediately, so in my case the struct netr_NetworkInfo allocated in rpccli_netlogon_network_logon overlaps it, and around line 470 of source3/rpc_client/cli_netlogon.c it writes a pointer where the netlogon_creds_cli_context keeps its locked_state pointer - so netlogon_creds_cli_LogonSamLogon, called immediately afterwards, fails with the LOCK_NOT_GRANTED error because it thinks the context is already locked.
The patch I'm attaching moves the netlogon_creds_cli_context into the talloc context provided to domain_client_validate, which ensures it lives long enough. I haven't tested this extensively but it fixes the immediate problem for me.
Thanks for the detailed information!
In Samba master, this is already fixed with commit 91e4cbc46f0 in
Can you confirm this fixes your problem as well?
Yes, that commit also fixes the problem I'm seeing.
Created attachment 11226 [details]
Patch from master
Thanks for the confirmation. We're preferring code that already exists in all branches. "Obsoleting" your patch does not mean it's not good, it's just different :-)
Comment on attachment 11226 [details]
Patch from master
Karolin, please add to 4.2 (it is not required for 4.1)
Pushed to autobuild-v4-2-test.
(In reply to Karolin Seeger from comment #6)
Pushed to v4-2-test.
Closing out bug report.