[Samba] Can login with a bogus username which ends with a "/" or a "\"

[Tompkins, Michael]

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