[Bug 69472] New: LookupAccountName() in user/win32/userinfo.c is not used properly

[bu...@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=69472

            Bug ID: 69472
           Summary: LookupAccountName() in user/win32/userinfo.c is not
                    used properly
           Product: APR
           Version: 1.7.5
          Hardware: PC
            Status: NEW
          Severity: normal
          Priority: P2
         Component: APR
          Assignee: bugs@apr.apache.org
          Reporter: mbienst@maku.eu
  Target Milestone: ---

We use log4cxx for our project to log into a logfile in current users home
directory under windows.
To do this, we use the system property ${user.home) from log4cxx. This does
work most of the time, but not for all systems/users.

To expand the property ${user.home}, log4cxx uses the function
'System::getProperty(const LogString& lkey)' in src/main/cpp/system.cpp of the
log4cxx project. It calls
'apr_uid_current(&userid, &groupid, p)' to get the current user id and
retrieves the username with 'apr_uid_name_get(&username, userid, p)'.
To get the homedirectory for that user, it calls
'apr_uid_homepath_get(&dirname, username, p)'. 
This function calls 'apr_uid_get(&uid, &gid, username, p))' to get the uid back
from given username to calculate the string SID to search into Registry for the
ProfileImagePath for that user to get the working directory. But
'apr_uid_get(&uid, &gid, username, p))' does not work correctly under all
circumstances if only the username is given.

For example: If you have a user, in our case the 'testing' user from domain
'info' with the following SID

PS C:> whoami /user

BENUTZERINFORMATIONEN
---------------------

Benutzername SID
============ ==============================================
info\testing S-1-5-21-220523388-1682526488-1957994488-12130


username from 'apr_uid_name_get()' is 'testing' and given to
'apr_uid_homepath_get(&dirname, username, p)'.
'apr_uid_get(&uid, &gid, username, p))' uses the windows function
LookupAccountName() to get the SID for the username. This does not work for the
given user 'testing', and it returns a wrong SID. In our case the string
representation for that user is S-1-5-21-3419135580-3221695306-2012181586 that
does not exist.

So i take a look at the Function Description for LookupAccountName()
(https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-lookupaccountnamea).
According to the description Microsoft says for parameter lpAccountName :"Use a
fully qualified string in the domain_name\user_name format to ensure that
LookupAccountName finds the account in the desired domain."

Ok, so i changed the username given from  apr_uid_name_get() into the following
form:

//*username = apr_pstrdup(p, name);
*username = apr_pstrcat(p, domain, "\\", name, NULL);

So that the resulting username for the above user is 'INFO\testing'. That does
not work at all, because apr_uid_get() returned an error.
so i take a closer look on apr_uid_get()

    if ((pos = strchr(username, '/'))) {
        domain = apr_pstrmemdup(p, username, pos - username);
        username = pos + 1;
    }
    else if ((pos = strchr(username, '\\'))) {
        domain = apr_pstrmemdup(p, username, pos - username);
        username = pos + 1;
    }
    else {
        domain = NULL;
    }
    /* Get nothing on the first pass ... need to size the sid buffer 
     */
    rv = LookupAccountName(domain, username, domain, &sidlen, 
                           anydomain, &domlen, &sidtype);
    if (sidlen) {
        /* Give it back on the second pass
         */
        *uid = apr_palloc(p, sidlen);
        domlen = sizeof(anydomain);
        rv = LookupAccountName(domain, username, *uid, &sidlen, 
                               anydomain, &domlen, &sidtype);
    }

If the given username has a domain, the domain is splitted from username and
also used as first parameter of function LookupAccountName().
Description for the first parameter says:
"A pointer to a null-terminated character string that specifies the name of the
system. This string can be the name of a remote computer. If this string is
NULL, the account name translation begins on the local system. If the name
cannot be resolved on the local system, this function will try to resolve the
name using domain controllers trusted by the local system. Generally, specify a
value for lpSystemName only when the account is in an untrusted domain and the
name of a computer in that domain is known."

In our case, info is a domain and not a remote computer,and the domain should
not be removed from the username.
As a second attempt, i changed the above code to the following that worked
correctly in our case and gave as the correct SID:


    // if ((pos = strchr(username, '/'))) {
        // domain = apr_pstrmemdup(p, username, pos - username);
        // username = pos + 1;
    // }
    // else if ((pos = strchr(username, '\\'))) {
        // domain = apr_pstrmemdup(p, username, pos - username);
        // username = pos + 1;
    // }
    // else {
        domain = NULL;
    // }
    /* Get nothing on the first pass ... need to size the sid buffer 
     */
    rv = LookupAccountName(domain, username, domain, &sidlen, 
                           anydomain, &domlen, &sidtype);
    if (sidlen) {
        /* Give it back on the second pass
         */
        *uid = apr_palloc(p, sidlen);
        domlen = sizeof(anydomain);
        rv = LookupAccountName(domain, username, *uid, &sidlen, 
                               anydomain, &domlen, &sidtype);
    }

In my opinion, the correct call to LookupAccountName should be:

rv = LookupAccountName(NULL, full_qualified_username, domain, &sidlen, 
                           anydomain, &domlen, &sidtype);

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org