samba - Feb 2015 - Can login with a bogus username which ends with a "/" or a "\"

We have noticed that if a username, that ends in a "\" or a
"/", tries to login, then the workspace becomes the user name ( up to
the "/" or "\" ) and then username is empty, allowing a
bogus user to authenticate and calls cli_session_setup_guest() to log in
anonymously. This is done in cli_session_setup():
                /* allow for workgroups as part of the username */
                if ((p=strchr_m(user2,'\\')) ||
(p=strchr_m(user2,'/')) ||
                    (p=strchr_m(user2,*lp_winbind_separator()))) {
                                *p = 0;
                                user = p+1;
                                if (!strupper_m(user2)) {
                                                return
NT_STATUS_INVALID_PARAMETER;
                                }
                                workgroup = user2;
                }

I'm guessing that this was intended for a "WORKSPACE/USERNAME"
construct and not for just "USERNAME/". We use smbclient to
authenticate users, for access to services on our machine, so letting bogus
users logon, is not a good thing.

In popt_common_credentials_callback() I added the code:

                case 'U':
                                {
                                                char *lp;
                                                char *puser = SMB_STRDUP(arg);

                                                if
((lp=strchr_m(puser,'%'))) {
                                                                size_t len;
                                                                *lp = 0;

                                                                len =
strlen(puser)-1;                                                                
// +++ added code
                                                                if (
(*(puser+len) == '\\') || (*(puser+len) == '/') )            //
+++ added code
                                                                               
*(puser+len) = 0;                                                             //
+++ added code

                                                               
set_cmdline_auth_info_username(auth_info,
                                                                                
puser);
                                                               
set_cmdline_auth_info_password(auth_info,
                                                                                
lp+1);
                                                                len =
strlen(lp+1);
                                                               
memset(strchr_m(arg,'%')+1,'X',len);
                                                } else {
                                                               
set_cmdline_auth_info_username(auth_info,
                                                                                
puser);
                                                }
                                                SAFE_FREE(puser);
                                }
                                break;

Are there use cases which we aren't thinking of, or does this modification
make sense. Please let us know.

Regards,
Mike

Re-submitting question, hoping for a yay or nay on the change as to whether my
logic is correct or not ...

Thank you in advance,
- Mike
?                      ?

-----Original Message-----
From: samba-bounces at lists.samba.org [mailto:samba-bounces at lists.samba.org]
On Behalf Of Tompkins, Michael
Sent: Monday, February 02, 2015 11:19 AM
To: samba at lists.samba.org
Cc: USA Xerox Samba
Subject: [Samba] Can login with a bogus username which ends with a "/"
or a "\"

We have noticed that if a username, that ends in a "\" or a
"/", tries to login, then the workspace becomes the user name ( up to
the "/" or "\" ) and then username is empty, allowing a
bogus user to authenticate and calls cli_session_setup_guest() to log in
anonymously. This is done in cli_session_setup():
                /* allow for workgroups as part of the username */
                if ((p=strchr_m(user2,'\\')) ||
(p=strchr_m(user2,'/')) ||
                    (p=strchr_m(user2,*lp_winbind_separator()))) {
                                *p = 0;
                                user = p+1;
                                if (!strupper_m(user2)) {
                                                return
NT_STATUS_INVALID_PARAMETER;
                                }
                                workgroup = user2;
                }

I'm guessing that this was intended for a "WORKSPACE/USERNAME"
construct and not for just "USERNAME/". We use smbclient to
authenticate users, for access to services on our machine, so letting bogus
users logon, is not a good thing.

In popt_common_credentials_callback() I added the code:

                case 'U':
                                {
                                                char *lp;
                                                char *puser = SMB_STRDUP(arg);

                                                if
((lp=strchr_m(puser,'%'))) {
                                                                size_t len;
                                                                *lp = 0;

                                                                len =
strlen(puser)-1;                                                                
// +++ added code
                                                                if (
(*(puser+len) == '\\') || (*(puser+len) == '/') )            //
+++ added code
                                                                               
*(puser+len) = 0;                                                             //
+++ added code

                                                               
set_cmdline_auth_info_username(auth_info,
                                                                                
puser);
                                                               
set_cmdline_auth_info_password(auth_info,
                                                                                
lp+1);
                                                                len =
strlen(lp+1);
                                                               
memset(strchr_m(arg,'%')+1,'X',len);
                                                } else {
                                                               
set_cmdline_auth_info_username(auth_info,
                                                                                
puser);
                                                }
                                                SAFE_FREE(puser);
                                }
                                break;

Are there use cases which we aren't thinking of, or does this modification
make sense. Please let us know.

Regards,
Mike


-- 
To unsubscribe from this list go to the following URL and read the
instructions:  https://lists.samba.org/mailman/options/samba