Some cleanups:

- Don't use pstrcpy into an allocated string - use safe_strcpy() directly
instead.

- Keep a copy of the 'server_info' attached to the vuid.  In future use this
for things like the session key, homedir and full name instead of current
copies.

- Try to avoid memory leak/segfault on Realloc failure

- clear up #endif comments

Andrew Bartlett
(This used to be commit 162477bb086827950b6cb71afa9bef62c2753c2e)

diff --git a/source3/auth/auth_ntlmssp.c b/source3/auth/auth_ntlmssp.c
index 3e650a7a2a7ec8b10db484e7f1df3e5c412fe01d..43542b24744660e117c28d5f0c3d8d54246a6dd4 100644 (file)
--- a/source3/auth/auth_ntlmssp.c
+++ b/source3/auth/auth_ntlmssp.c
@@ -125,7 +125,6 @@ NTSTATUS auth_ntlmssp_end(AUTH_NTLMSSP_STATE **auth_ntlmssp_state)
        if ((*auth_ntlmssp_state)->server_info) {
                free_server_info(&(*auth_ntlmssp_state)->server_info);
        }
-
        talloc_destroy(mem_ctx);
        *auth_ntlmssp_state = NULL;
        return NT_STATUS_OK;

diff --git a/source3/auth/auth_unix.c b/source3/auth/auth_unix.c
index 1251432b871462ebb187ea8991e54f85761fda10..4f44767a81aeb884ce199c186d82678bf2db2fc8 100644 (file)
--- a/source3/auth/auth_unix.c
+++ b/source3/auth/auth_unix.c
@@ -106,7 +106,7 @@ static NTSTATUS check_unix_security(const struct auth_context *auth_context,
        
        unbecome_root();
 
-       if NT_STATUS_IS_OK(nt_status) {
+       if (NT_STATUS_IS_OK(nt_status)) {
                if (pass) {
                        make_server_info_pw(server_info, pass);
                } else {

diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
index 5fdfd0694a1610c2f05c283bdfc33cc84663650f..bbe0c7cf43298c3c2171f292073213df43ba6ba3 100644 (file)
--- a/source3/auth/auth_util.c
+++ b/source3/auth/auth_util.c
@@ -671,14 +671,22 @@ static NTSTATUS get_user_groups_from_local_sam(const DOM_SID *user_sid,
        };
        
        n_unix_groups = groups_max();
-       if ((*unix_groups = malloc( sizeof(gid_t) * groups_max() ) ) == NULL) {
+       if ((*unix_groups = malloc( sizeof(gid_t) * n_unix_groups ) ) == NULL) {
                DEBUG(0, ("get_user_groups_from_local_sam: Out of memory allocating unix group list\n"));
                passwd_free(&usr);
                return NT_STATUS_NO_MEMORY;
        }
        
        if (sys_getgrouplist(usr->pw_name, usr->pw_gid, *unix_groups, &n_unix_groups) == -1) {
-               *unix_groups = Realloc(*unix_groups, sizeof(gid_t) * n_unix_groups);
+               gid_t *groups_tmp;
+               groups_tmp = Realloc(*unix_groups, sizeof(gid_t) * n_unix_groups);
+               if (!groups_tmp) {
+                       SAFE_FREE(*unix_groups);
+                       passwd_free(&usr);
+                       return NT_STATUS_NO_MEMORY;
+               }
+               *unix_groups = groups_tmp;
+
                if (sys_getgrouplist(usr->pw_name, usr->pw_gid, *unix_groups, &n_unix_groups) == -1) {
                        DEBUG(0, ("get_user_groups_from_local_sam: failed to get the unix group list\n"));
                        SAFE_FREE(*unix_groups);

diff --git a/source3/auth/pass_check.c b/source3/auth/pass_check.c
index e1783bfd1e2f72a7d57dcf58e50f70c3ddbd42a5..88b82e34746eba77a1204e18926b9a7c891aa15a 100644 (file)
--- a/source3/auth/pass_check.c
+++ b/source3/auth/pass_check.c
@@ -579,7 +579,7 @@ static NTSTATUS password_check(const char *password)
        }
 #endif /* HAVE_CRYPT */
 #endif /* HAVE_BIGCRYPT && HAVE_CRYPT && USE_BOTH_CRYPT_CALLS */
-#endif /* WITH_PAM || KRB4_AUTH || KRB5_AUTH */
+#endif /* WITH_PAM */
 }
 
 

diff --git a/source3/include/smb.h b/source3/include/smb.h
index 279b79eace3f9c466222b6e94032550f03610b7d..f96a19954a24770b395f273224e6b339bdd33526 100644 (file)
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -1642,6 +1642,8 @@ typedef struct user_struct
                                 TDB key string */
        int homes_snum;
 
+       struct auth_serversupplied_info *server_info;
+
 } user_struct;
 
 

diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index 9e8de2979b91b7e84d8acd930d6177a822ce2855..e022d26ea3c1943f7f57c56b5dabd59d2171ea91 100644 (file)
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -907,7 +907,7 @@ void *OpenDir(connection_struct *conn, const char *name, BOOL use_veto)
                        dirp->current = dirp->data;
                }
 
-               pstrcpy(dirp->data+used,n);
+               safe_strcpy(dirp->data+used,n, dirp->mallocsize - used - 1);
                used += l;
                dirp->numentries++;
        }

diff --git a/source3/smbd/password.c b/source3/smbd/password.c
index 4ce99e96bbe36fe7f9e86cb77b914eceb127bdda..5274028db4a4854a518b52cc0344cbfd23c7f538 100644 (file)
--- a/source3/smbd/password.c
+++ b/source3/smbd/password.c
@@ -62,11 +62,15 @@ void invalidate_vuid(uint16 vuid)
 
        if (vuser == NULL)
                return;
-
+       
        SAFE_FREE(vuser->homedir);
-
+       SAFE_FREE(vuser->unix_homedir);
+       SAFE_FREE(vuser->logon_script);
+       
        session_yield(vuser);
 
+       free_server_info(&vuser->server_info);
+
        DLIST_REMOVE(validated_users, vuser);
 
        /* clear the vuid from the 'cache' on each connection, and
@@ -93,11 +97,15 @@ void invalidate_all_vuids(void)
        }
 }
 
-/****************************************************************************
-register a uid/name pair as being valid and that a valid password
-has been given. vuid is biased by an offset. This allows us to
-tell random client vuid's (normally zero) from valid vuids.
-****************************************************************************/
+/**
+ *  register that a valid login has been performed, establish 'session'.
+ *  @param server_info The token returned from the authentication process. 
+ *   (now 'owned' by register_vuid)
+ *
+ *  @return Newly allocated vuid, biased by an offset. (This allows us to
+ *   tell random client vuid's (normally zero) from valid vuids.)
+ *
+ */
 
 int register_vuid(auth_serversupplied_info *server_info, const char *smb_name)
 {
@@ -136,6 +144,7 @@ int register_vuid(auth_serversupplied_info *server_info, const char *smb_name)
        if (!IS_SAM_UNIX_USER(server_info->sam_account)) {
                DEBUG(0,("Attempted session setup with invalid user.  No uid/gid in SAM_ACCOUNT\n"));
                free(vuser);
+               free_server_info(&server_info);
                return UID_FIELD_INVALID;
        }
        
@@ -147,20 +156,24 @@ int register_vuid(auth_serversupplied_info *server_info, const char *smb_name)
                if (!(vuser->groups = memdup(server_info->groups, sizeof(gid_t) * vuser->n_groups))) {
                        DEBUG(0,("register_vuid: failed to memdup vuser->groups\n"));
                        free(vuser);
+                       free_server_info(&server_info);
                        return UID_FIELD_INVALID;
                }
        }
 
        vuser->guest = server_info->guest;
        fstrcpy(vuser->user.unix_name, pdb_get_username(server_info->sam_account)); 
-       fstrcpy(vuser->user.smb_name, smb_name); 
+
+       /* This is a potentially untrusted username */
+       alpha_strcpy(vuser->user.smb_name, smb_name, ". _-$", sizeof(vuser->user.smb_name));
+
        fstrcpy(vuser->user.domain, pdb_get_domain(server_info->sam_account));
        fstrcpy(vuser->user.full_name, pdb_get_fullname(server_info->sam_account));
 
        {
                /* Keep the homedir handy */
                const char *homedir = pdb_get_homedir(server_info->sam_account);
-               const char *unix_homedir = pdb_get_unix_homedir(server_info->sam_account); /* should be optained by SMS */
+               const char *unix_homedir = pdb_get_unix_homedir(server_info->sam_account);
                const char *logon_script = pdb_get_logon_script(server_info->sam_account);
                if (homedir) {
                        vuser->homedir = smb_xstrdup(homedir);
@@ -188,10 +201,18 @@ int register_vuid(auth_serversupplied_info *server_info, const char *smb_name)
                vuser->nt_user_token = dup_nt_token(server_info->ptok);
        } else {
                DEBUG(1, ("server_info does not contain a user_token - cannot continue\n"));
-               free(vuser);
+               free_server_info(&server_info);
+               SAFE_FREE(vuser->homedir);
+               SAFE_FREE(vuser->unix_homedir);
+               SAFE_FREE(vuser->logon_script);
+
+               SAFE_FREE(vuser);
                return UID_FIELD_INVALID;
        }
 
+       /* use this to keep tabs on all our info from the authentication */
+       vuser->server_info = server_info;
+
        DEBUG(3,("UNIX uid %d is UNIX user %s, and will be vuid %u\n",(int)vuser->uid,vuser->user.unix_name, vuser->vuid));
 
        next_vuid++;

diff --git a/source3/smbd/sesssetup.c b/source3/smbd/sesssetup.c
index bb7d17be56f4a190e143f063c1cabe6b1f5f9435..e408cc88e9a1f919aca34b1021f546f5230dd54b 100644 (file)
--- a/source3/smbd/sesssetup.c
+++ b/source3/smbd/sesssetup.c
@@ -168,6 +168,8 @@ static int reply_spnego_kerberos(connection_struct *conn,
                return ERROR_NT(NT_STATUS_LOGON_FAILURE);
        }
 
+       data_blob_free(&auth_data);
+
        DEBUG(3,("Ticket name is [%s]\n", client));
 
        p = strchr_m(client, '@');
@@ -219,10 +221,10 @@ static int reply_spnego_kerberos(connection_struct *conn,
                return ERROR_NT(ret);
        }
        
+       /* register_vuid keeps the server info */
        sess_vuid = register_vuid(server_info, user);
 
        free(user);
-       free_server_info(&server_info);
 
        if (sess_vuid == -1) {
                return ERROR_NT(NT_STATUS_LOGON_FAILURE);
@@ -263,8 +265,10 @@ static BOOL reply_spnego_ntlmssp(connection_struct *conn, char *outbuf,
 
        if (NT_STATUS_IS_OK(nt_status)) {
                int sess_vuid;
-               sess_vuid = register_vuid(server_info, (*auth_ntlmssp_state)->ntlmssp_state->user /* check this for weird */);
-               
+               /* register_vuid keeps the server info */
+               sess_vuid = register_vuid(server_info, (*auth_ntlmssp_state)->ntlmssp_state->user);
+               (*auth_ntlmssp_state)->server_info = NULL;
+
                if (sess_vuid == -1) {
                        nt_status = NT_STATUS_LOGON_FAILURE;
                } else {
@@ -272,7 +276,7 @@ static BOOL reply_spnego_ntlmssp(connection_struct *conn, char *outbuf,
                        set_message(outbuf,4,0,True);
                        SSVAL(outbuf, smb_vwv3, 0);
                        
-                       if ((*auth_ntlmssp_state)->server_info && (*auth_ntlmssp_state)->server_info->guest) {
+                       if (server_info->guest) {
                                SSVAL(outbuf,smb_vwv2,1);
                        }
                        
@@ -285,7 +289,7 @@ static BOOL reply_spnego_ntlmssp(connection_struct *conn, char *outbuf,
        data_blob_free(&response);
 
        if (!ret || !NT_STATUS_EQUAL(nt_status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
-               auth_ntlmssp_end(&global_ntlmssp_state);
+               auth_ntlmssp_end(auth_ntlmssp_state);
        }
 
        return ret;
@@ -584,13 +588,6 @@ int reply_sesssetup_and_X(connection_struct *conn, char *inbuf,char *outbuf,
                         domain,native_os,native_lanman));
        }
        
-       /* don't allow for weird usernames or domains */
-       alpha_strcpy(user, user, ". _-$", sizeof(user));
-       alpha_strcpy(domain, domain, ". _-@", sizeof(domain));
-       if (strstr(user, "..") || strstr(domain,"..")) {
-               return ERROR_NT(NT_STATUS_LOGON_FAILURE);
-       }
-
        DEBUG(3,("sesssetupX:name=[%s]\\[%s]@[%s]\n", domain, user, get_remote_machine_name()));
 
        if (*user) {
@@ -609,7 +606,7 @@ int reply_sesssetup_and_X(connection_struct *conn, char *inbuf,char *outbuf,
                fstrcpy(sub_user, lp_guestaccount());
        }
 
-       fstrcpy(current_user_info.smb_name,sub_user);
+       sub_set_smb_name(sub_user);
 
        reload_services(True);
        
@@ -692,15 +689,13 @@ int reply_sesssetup_and_X(connection_struct *conn, char *inbuf,char *outbuf,
        /* register the name and uid as being validated, so further connections
           to a uid can get through without a password, on the same VC */
 
+       /* register_vuid keeps the server info */
        sess_vuid = register_vuid(server_info, sub_user);
-
-       free_server_info(&server_info);
   
        if (sess_vuid == -1) {
                return ERROR_NT(NT_STATUS_LOGON_FAILURE);
        }
 
- 
        SSVAL(outbuf,smb_uid,sess_vuid);
        SSVAL(inbuf,smb_uid,sess_vuid);
        

diff --git a/source3/smbd/statcache.c b/source3/smbd/statcache.c
index 93782b9bb01109c75cb2b339b54a1f2c6bb466a3..f4b613428a223803976b2861f90da93e1df36263 100644 (file)
--- a/source3/smbd/statcache.c
+++ b/source3/smbd/statcache.c
@@ -122,8 +122,8 @@ void stat_cache_add( char *full_orig_name, char *orig_translated_path)
       DEBUG(0,("stat_cache_add: Out of memory !\n"));
       return;
     }
-    pstrcpy(scp->names, orig_name);
-    pstrcpy(scp->names+namelen+1, translated_path);
+    safe_strcpy(scp->names, orig_name, namelen);
+    safe_strcpy(scp->names+namelen+1, translated_path, namelen);
     scp->name_len = namelen;
     hash_insert(&stat_cache, (char *)scp, orig_name);
   }