When smbd is using the ldapsam backend, and the sambaLMPassword and sambaNTPassword entries are missing from a user account, smbd crashes when an attempt is made to login with this account.
Created attachment 5726 [details] Fix segfault at authentication when nt_pw is NULL This patch fixes the segfault. It ensures that nt_pw is not NULL, just before the call that caused the crash. Not entirely sure if NT_STATUS_WRONG_PASSWORD is the right return code, though.
Created attachment 5727 [details] Fix segfault at authentication when nt_pw is NULL v2 V2 of patch: we better check nt_pw at the start of the block, though in normal use case it doesn't matter much.
Created attachment 5731 [details] git-am format patch for 3.5.x.
Perfectly correct - thanks ! Applied to master. Re-assigning to Karolin for inclusion in 3.5.x. Jeremy.
Sorry, I do need a review from a second Samba developer first...
Comment on attachment 5731 [details] git-am format patch for 3.5.x. I'll give it a +, because it seems to fix the immediate problem. But the patch looks wrong: What happens in the ntlm_password_check() case for example? Nevertheless, Karolin, please pull this for 3.5
As far as I could see, ntlm_password_check() checks both before using them, but I haven't explicitly tested it. Maybe we should add an early check that returns NT_STATUS_WRONG_PASSWORD if neither lm_hash nor nt_hash are set: in that case, it's not useful even trying to run the lower functions, since they always need at least one stored password to verify against.
As we do have enough time to wait for a "proper" fix for 3.5.4, I am going to wait a bit before picking the patch.
(In reply to comment #6) > (From update of attachment 5731 [details]) > I'll give it a +, because it seems to fix the immediate problem. But the patch > looks wrong: What happens in the ntlm_password_check() case for example? The patch was meant to protect the call to SMBsesskeygen_ntv1(). As far as I can see, both hash_password_check() and ntlm_password_check() do their own checking for presence of stored hashes. If you'd like to see this done differently, could you point me in the right direction?
Karolin, Roel van Meer is correct. I went through the code paths and all other uses of nt_pw are correctly checked. I'm afraid this is the "proper patch" :-). I can't see a different way of doing this. Please pull for 3.5.x. Volker, if you want to confirm feel free but I agree with Roel. Jeremy.
Okay, thanks for commenting again! Pushed patch to v3-5-test. Closing out bug report. Please feel free to re-open if there are any issues left. Roel, thanks a lot for reporting and providing the patch!