[prev in list] [next in list] [prev in thread] [next in thread] 

List:       ros-dev
Subject:    Re: [ros-dev] [ros-diffs] [hpoussin] 28693: Implement
From:       Jeremy Drake <reactos () jdrake ! com>
Date:       2007-08-31 15:47:43
Message-ID: Pine.BSO.4.64.0708310843180.27513 () resin ! csoft ! net
[Download RAW message or body]

Umm, this implementation of IsUserAnAdmin is not quite correct.  I just
had to deal with a function almost identical to this in real life, which
gave the wrong answer on Vista due to the filtered token containing the
Administrators group with the "for deny only" attribute.  You need to
either check the attribute portion of the SID_AND_ATTRIBUTE struct as
well, or use something like CheckTokenMembership (assuming that is
implemented properly), or construct a security descriptor and use
AccessCheck (again, assuming that is implemented correctly (I would hope
so...)).

Of course, this is probably just overly anal on ReactOS, as you don't have
the whole Vista filtered token/elevation mess (yet), but probably best to
be technically correct...


On Fri, 31 Aug 2007, hpoussin@svn.reactos.org wrote:

> Author: hpoussin
> Date: Fri Aug 31 13:49:48 2007
> New Revision: 28693
> 
> URL: http://svn.reactos.org/svn/reactos?rev=28693&view=rev
> Log:
> Implement IsUserAnAdmin(), by copying implementation of IsUserAdmin() in setupapi \
> (was implemented by Eric Kohl) Redirect setupapi.IsUserAdmin to \
> shell32.IsUserAnAdmin 
> Modified:
> trunk/reactos/dll/win32/setupapi/cfgmgr.c
> trunk/reactos/dll/win32/setupapi/misc.c
> trunk/reactos/dll/win32/setupapi/setupapi.spec
> trunk/reactos/dll/win32/shell32/shellord.c
> 
> Modified: trunk/reactos/dll/win32/setupapi/cfgmgr.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/setupapi/cfgmgr.c?rev=28693&r1=28692&r2=28693&view=diff
>  ==============================================================================
> --- trunk/reactos/dll/win32/setupapi/cfgmgr.c (original)
> +++ trunk/reactos/dll/win32/setupapi/cfgmgr.c Fri Aug 31 13:49:48 2007
> @@ -135,7 +135,7 @@
> if (!PnpGetLocalHandles(&BindingHandle, NULL))
> return CR_FAILURE;
> 
> -    bAdmin = IsUserAdmin();
> +    bAdmin = IsUserAnAdmin();
> 
> for (i = 0; i < 30; i++)
> {
> @@ -182,7 +182,7 @@
> FIXME("%p %p %lu %lx %p\n",
> plcLogConf, dnDevInst, Priority, ulFlags, hMachine);
> 
> -    if (!IsUserAdmin())
> +    if (!IsUserAnAdmin())
> return CR_ACCESS_DENIED;
> 
> if (plcLogConf == NULL)
> @@ -298,7 +298,7 @@
> 
> TRACE("%p %s %lx %p\n", dnDevInst, debugstr_w(pszID), ulFlags, hMachine);
> 
> -    if (!IsUserAdmin())
> +    if (!IsUserAnAdmin())
> return CR_ACCESS_DENIED;
> 
> if (dnDevInst == 0)
> @@ -490,7 +490,7 @@
> FIXME("%p %s %p %lx %p\n",
> pdnDevInst, debugstr_w(pDeviceID), dnParent, ulFlags, hMachine);
> 
> -    if (!IsUserAdmin())
> +    if (!IsUserAnAdmin())
> return CR_ACCESS_DENIED;
> 
> if (pdnDevInst == NULL)
> @@ -637,7 +637,7 @@
> 
> FIXME("%p %lx %p\n", dnDevInst, ulFlags, hMachine);
> 
> -    if (!IsUserAdmin())
> +    if (!IsUserAnAdmin())
> return CR_ACCESS_DENIED;
> 
> if (dnDevInst == 0)
> @@ -725,7 +725,7 @@
> 
> FIXME("%p %lx %p\n", dnDevInst, ulFlags, hMachine);
> 
> -    if (!IsUserAdmin())
> +    if (!IsUserAnAdmin())
> return CR_ACCESS_DENIED;
> 
> if (dnDevInst == 0)
> @@ -961,7 +961,7 @@
> 
> TRACE("%lx %lx %lx\n", lcLogConfToBeFreed, ulFlags, hMachine);
> 
> -    if (!IsUserAdmin())
> +    if (!IsUserAnAdmin())
> return CR_ACCESS_DENIED;
> 
> pLogConfInfo = (PLOG_CONF_INFO)lcLogConfToBeFreed;
> @@ -2707,7 +2707,7 @@
> FIXME("%lx %lx %lx %lx\n",
> dnFromDevInst, dnToDevInst, ulFlags, hMachine);
> 
> -    if (!IsUserAdmin())
> +    if (!IsUserAnAdmin())
> return CR_ACCESS_DENIED;
> 
> if (dnFromDevInst == 0 || dnToDevInst == 0)
> @@ -3051,7 +3051,7 @@
> 
> TRACE("%lx %lx\n", ulFlags, hMachine);
> 
> -    if (!IsUserAdmin())
> +    if (!IsUserAnAdmin())
> return CR_ACCESS_DENIED;
> 
> if (ulFlags & ~CM_DETECT_BITS)
> @@ -3525,7 +3525,7 @@
> 
> FIXME("%lx %lx %lx\n", dnDevInst, ulFlags, hMachine);
> 
> -    if (!IsUserAdmin())
> +    if (!IsUserAnAdmin())
> return CR_ACCESS_DENIED;
> 
> if (dnDevInst == 0)
> 
> Modified: trunk/reactos/dll/win32/setupapi/misc.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/setupapi/misc.c?rev=28693&r1=28692&r2=28693&view=diff
>  ==============================================================================
> --- trunk/reactos/dll/win32/setupapi/misc.c (original)
> +++ trunk/reactos/dll/win32/setupapi/misc.c Fri Aug 31 13:49:48 2007
> @@ -246,84 +246,6 @@
> MyFree(*lpData);
> 
> return lError;
> -}
> -
> -
> -/**************************************************************************
> - * IsUserAdmin [SETUPAPI.@]
> - *
> - * Checks whether the current user is a member of the Administrators group.
> - *
> - * PARAMS
> - *     None
> - *
> - * RETURNS
> - *     Success: TRUE
> - *     Failure: FALSE
> - */
> -BOOL WINAPI IsUserAdmin(VOID)
> -{
> -    SID_IDENTIFIER_AUTHORITY Authority = {SECURITY_NT_AUTHORITY};
> -    HANDLE hToken;
> -    DWORD dwSize;
> -    PTOKEN_GROUPS lpGroups;
> -    PSID lpSid;
> -    DWORD i;
> -    BOOL bResult = FALSE;
> -
> -    TRACE("\n");
> -
> -    if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &hToken))
> -    {
> -        return FALSE;
> -    }
> -
> -    if (!GetTokenInformation(hToken, TokenGroups, NULL, 0, &dwSize))
> -    {
> -        if (GetLastError() != ERROR_INSUFFICIENT_BUFFER)
> -        {
> -            CloseHandle(hToken);
> -            return FALSE;
> -        }
> -    }
> -
> -    lpGroups = MyMalloc(dwSize);
> -    if (lpGroups == NULL)
> -    {
> -        CloseHandle(hToken);
> -        return FALSE;
> -    }
> -
> -    if (!GetTokenInformation(hToken, TokenGroups, lpGroups, dwSize, &dwSize))
> -    {
> -        MyFree(lpGroups);
> -        CloseHandle(hToken);
> -        return FALSE;
> -    }
> -
> -    CloseHandle(hToken);
> -
> -    if (!AllocateAndInitializeSid(&Authority, 2, SECURITY_BUILTIN_DOMAIN_RID,
> -                                  DOMAIN_ALIAS_RID_ADMINS, 0, 0, 0, 0, 0, 0,
> -                                  &lpSid))
> -    {
> -        MyFree(lpGroups);
> -        return FALSE;
> -    }
> -
> -    for (i = 0; i < lpGroups->GroupCount; i++)
> -    {
> -        if (EqualSid(lpSid, lpGroups->Groups[i].Sid))
> -        {
> -            bResult = TRUE;
> -            break;
> -        }
> -    }
> -
> -    FreeSid(lpSid);
> -    MyFree(lpGroups);
> -
> -    return bResult;
> }
> 
> 
> 
> Modified: trunk/reactos/dll/win32/setupapi/setupapi.spec
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/setupapi/setupapi.spec?rev=28693&r1=28692&r2=28693&view=diff
>  ==============================================================================
> --- trunk/reactos/dll/win32/setupapi/setupapi.spec (original)
> +++ trunk/reactos/dll/win32/setupapi/setupapi.spec Fri Aug 31 13:49:48 2007
> @@ -216,7 +216,7 @@
> @ stdcall InstallHinfSectionW(long long wstr long)
> @ stub InstallStop
> @ stub InstallStopEx
> -@ stdcall IsUserAdmin()
> +@ stdcall IsUserAdmin=shell32.IsUserAnAdmin()
> @ stub LookUpStringInTable
> @ stub MemoryInitialize
> @ stdcall MultiByteToUnicode(str long)
> 
> Modified: trunk/reactos/dll/win32/shell32/shellord.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/shell32/shellord.c?rev=28693&r1=28692&r2=28693&view=diff
>  ==============================================================================
> --- trunk/reactos/dll/win32/shell32/shellord.c (original)
> +++ trunk/reactos/dll/win32/shell32/shellord.c Fri Aug 31 13:49:48 2007
> @@ -1227,10 +1227,78 @@
> /*************************************************************************
> * IsUserAnAdmin					[SHELL32.680] NT 4.0
> *
> + * Checks whether the current user is a member of the Administrators group.
> + *
> + * PARAMS
> + *     None
> + *
> + * RETURNS
> + *     Success: TRUE
> + *     Failure: FALSE
> */
> BOOL WINAPI IsUserAnAdmin(VOID)
> -{	FIXME("stub\n");
> -	return TRUE;
> +{
> +    SID_IDENTIFIER_AUTHORITY Authority = {SECURITY_NT_AUTHORITY};
> +    HANDLE hToken;
> +    DWORD dwSize;
> +    PTOKEN_GROUPS lpGroups;
> +    PSID lpSid;
> +    DWORD i;
> +    BOOL bResult = FALSE;
> +
> +    TRACE("\n");
> +
> +    if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &hToken))
> +    {
> +        return FALSE;
> +    }
> +
> +    if (!GetTokenInformation(hToken, TokenGroups, NULL, 0, &dwSize))
> +    {
> +        if (GetLastError() != ERROR_INSUFFICIENT_BUFFER)
> +        {
> +            CloseHandle(hToken);
> +            return FALSE;
> +        }
> +    }
> +
> +    lpGroups = HeapAlloc(GetProcessHeap(), 0, dwSize);
> +    if (lpGroups == NULL)
> +    {
> +        CloseHandle(hToken);
> +        return FALSE;
> +    }
> +
> +    if (!GetTokenInformation(hToken, TokenGroups, lpGroups, dwSize, &dwSize))
> +    {
> +        HeapFree(GetProcessHeap(), 0, lpGroups);
> +        CloseHandle(hToken);
> +        return FALSE;
> +    }
> +
> +    CloseHandle(hToken);
> +
> +    if (!AllocateAndInitializeSid(&Authority, 2, SECURITY_BUILTIN_DOMAIN_RID,
> +                                  DOMAIN_ALIAS_RID_ADMINS, 0, 0, 0, 0, 0, 0,
> +                                  &lpSid))
> +    {
> +        HeapFree(GetProcessHeap(), 0, lpGroups);
> +        return FALSE;
> +    }
> +
> +    for (i = 0; i < lpGroups->GroupCount; i++)
> +    {
> +        if (EqualSid(lpSid, lpGroups->Groups[i].Sid))
> +        {
> +            bResult = TRUE;
> +            break;
> +        }
> +    }
> +
> +    FreeSid(lpSid);
> +    HeapFree(GetProcessHeap(), 0, lpGroups);
> +
> +    return bResult;
> }
> 
> /*************************************************************************
> 
> 
> 
> 

-- 
Fortune's Real-Life Courtroom Quote #19:

Q:  Doctor, how many autopsies have you performed on dead people?
A:  All my autopsies have been performed on dead people.
_______________________________________________
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic