swat: bug located

[Michael Douglass]

Great job on SWAT.  I'm a hard-core UNIX guy, and I like my vi and whatnot.
But it's nice to see a functional web interface.  It could use some more
work--but great job so far!  Anyway, I found a core-dump-causing-bug.

I was curious as to whether or not swat limited addition of passwords
to the smbpasswd file by the users that exist in the system's /etc/passwd
file.  I was relativly pleased to learn that it does, but only does so
by virtue of core dumping when you try to do it.  Here's why:

(Source code is from samba 2.0.3 downloaded yesterday)

Note that at swat.c:635, we are calling local_password_change with
trust_account = False.  Then, inside local_password_change (which in
the case that is core dumping is being called by clicking 'add user'
with a username not in /etc/passwd) you check to see if the user exists;
but only if trust_account is true.  I guess this is fine; I don't know
enough about the protocols to know exactly what needs to be done where
and why; but I do know that on line 111 you enter a conditional that
checks "did we find an smb_pwent? no, okay, are we being called with
add_user? yes, okay, let's call add_new_user passing user_name and then
the big clincher (on line 121) pwd->pw_uid....  Remember that the user did
not exist in the /etc/passwd file; thus pwd = null pointer. :)  I'd
supply a patch to fix this; but there are various ways to 'fix' this and
not knowing enough about the protocol...

web/swat.c: line 635

    ret = local_password_change(user_name, False, add_user, enable_user,
                     disable_user, False, new_passwd, err_str, sizeof(err_str),
                     msg_str, sizeof(msg_str));

passdb/smbpasschange.c: line 58

BOOL local_password_change(char *user_name, BOOL trust_account, BOOL add_user,
            BOOL enable_user, BOOL disable_user, BOOL set_no_password,
            char *new_passwd, 
            char *err_str, size_t err_str_len,
            char *msg_str, size_t msg_str_len)
{
   struct passwd  *pwd;
   void *vp;
   struct smb_passwd *smb_pwent;
   uchar           new_p16[16];
   uchar           new_nt_p16[16];
 
   *err_str = '\0';
   *msg_str = '\0';
 
   pwd = getpwnam(user_name);
 
   /*
    * Check for a machine account.
    */
 
   if(trust_account && !pwd) {
      slprintf(err_str, err_str_len - 1, "User %s does not \
exist in system password file (usually /etc/passwd). Cannot add machine \
account without a valid system user.\n", user_name);
      return False;
   }

  ... *SNIP* ... Continue on line 111
   /* Get the smb passwd entry for this user */
   smb_pwent = getsmbpwnam(user_name);
   if (smb_pwent == NULL) {
      if(add_user == False) {
         slprintf(err_str, err_str_len-1,
            "Failed to find entry for user %s.\n", pwd->pw_name);
         endsmbpwent(vp);
         return False;
      }
 
      if (add_new_user(user_name, pwd->pw_uid, trust_account, disable_user,
             set_no_password, new_p16, new_nt_p16)) {
         slprintf(msg_str, msg_str_len-1, "Added user %s.\n", user_name);
         endsmbpwent(vp);
         return True;
      } else {



-- 
Michael Douglass
Texas Networking, Inc.

  Any sufficiently advanced bug is indistinguishable for a feature.
    -- from some indian guy