[PATCH] Check result from ALL getsam21pwnam calls (SAMBA 2.2.0cvs)

[Andrew Bartlett]

I think I found a live one...

In an older CVS version of samba, domain logons did a search of the
entire smbpasswd file for NT domain logons.  Fortunately this behavior
has changed, but I was still interested in chasing up a bug - samba
would report 'smbpasswd file is corrupt' and would promptly die with
SIG11.  (The actual case was for both domain logons and remote access
from Win98 to a printer shared on that workstation).

I presumed what was happening was that while iterating over the
smbpasswd file, something was calling getpwnam, and something else
wasn't checking the results.

In any case, I decided to see if the bug was still there, but I don't
have a copy of NT4 setup to test it, so I decided to find it by hand.

A few hours later, (grep and emacs are your friend) I found this:

srv_netlog_nt.c:_net_sam_logon()

        /*
         * Do any case conversions.
         */

        pw=Get_Pwnam(nt_username, True);
        
        become_root();
        sam_pass = getsam21pwnam(nt_username);
        smb_pass = getsmbpwnam(nt_username);
        unbecome_root();
        
        if (smb_pass == NULL)
                return NT_STATUS_NO_SUCH_USER;
        else if (smb_pass->acct_ctrl & ACB_DISABLED)
                return NT_STATUS_ACCOUNT_DISABLED;
    
        /* Validate password - if required. */

This is the only case where any of the getsam21pw functions are used
without checking that the result is not null.  The Get_Pwnam output
should also be checked.

Index: source/rpc_server/srv_netlog_nt.c
===================================================================
RCS file: /cvsroot/samba/source/rpc_server/srv_netlog_nt.c,v
retrieving revision 1.1.2.5
diff -u -d -r1.1.2.5 srv_netlog_nt.c
--- source/rpc_server/srv_netlog_nt.c	2001/03/14 21:48:59	1.1.2.5
+++ source/rpc_server/srv_netlog_nt.c	2001/04/07 02:52:33
@@ -527,7 +527,7 @@
 	smb_pass = getsmbpwnam(nt_username);
 	unbecome_root();
         
-	if (smb_pass == NULL)
+	if ((smb_pass == NULL) || (sam_pass == NULL) || (pw == NULL))
 		return NT_STATUS_NO_SUCH_USER;
 	else if (smb_pass->acct_ctrl & ACB_DISABLED)
 		return NT_STATUS_ACCOUNT_DISABLED;


There is some good news as well, it seems my original bug has been
flushed out when the smbpasswd iteration was removed - I'll need to get
a newer version installed to check.

I also feel like attacking the rest of this function, so if I get VMware
working again I'll give both of these a test.

Andrew Bartlett
abartlet at pcug.org.au
-- 
Andrew Bartlett
abartlet at pcug.org.au