Author: ekohl Date: Mon Sep 6 15:26:12 2010 New Revision: 48710
URL: http://svn.reactos.org/svn/reactos?rev=48710&view=rev Log: Improvements to NtAdjustPrivilegesToken part 2: - Check for invalid parameter combinations. - Count privileges that will be changed before changing them. - Return required buffer size. - Fail if the provided buffer is too small. See issue #5497 for more details.
Modified: trunk/reactos/ntoskrnl/se/token.c
Modified: trunk/reactos/ntoskrnl/se/token.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/se/token.c?rev=487... ============================================================================== --- trunk/reactos/ntoskrnl/se/token.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/se/token.c [iso-8859-1] Mon Sep 6 15:26:12 2010 @@ -1958,22 +1958,21 @@ ULONG CapturedCount = 0; ULONG CapturedLength = 0; ULONG NewStateSize = 0; - ULONG PrivilegeCount; + ULONG ChangeCount; PTOKEN Token; ULONG i; ULONG j; ULONG k; ULONG Count; -#if 0 - ULONG a; - ULONG b; - ULONG c; -#endif NTSTATUS Status;
PAGED_CODE();
DPRINT ("NtAdjustPrivilegesToken() called\n"); + + /* Fail, if we do not disable all privileges but NewState is NULL */ + if (DisableAllPrivileges == FALSE && NewState == NULL) + return STATUS_INVALID_PARAMETER;
PreviousMode = KeGetPreviousMode (); if (PreviousMode != KernelMode) @@ -2067,21 +2066,77 @@ return Status; }
- -#if 0 - SepAdjustPrivileges(Token, - 0, - PreviousMode, - PrivilegeCount, - Privileges, - PreviousState, - &a, - &b, - &c); -#endif - - PrivilegeCount = (BufferLength - FIELD_OFFSET(TOKEN_PRIVILEGES, Privileges)) / - sizeof(LUID_AND_ATTRIBUTES); + /* Count the privileges that need to be changed */ + ChangeCount = 0; + for (i = 0; i < Token->PrivilegeCount; i++) + { + if (DisableAllPrivileges) + { + if (Token->Privileges[i].Attributes & SE_PRIVILEGE_ENABLED) + { + DPRINT("Attribute enabled\n"); + + ChangeCount++; + } + } + else + { + for (j = 0; j < CapturedCount; j++) + { + if (Token->Privileges[i].Luid.LowPart == CapturedPrivileges[j].Luid.LowPart && + Token->Privileges[i].Luid.HighPart == CapturedPrivileges[j].Luid.HighPart) + { + DPRINT("Found privilege\n"); + + if ((Token->Privileges[i].Attributes & SE_PRIVILEGE_ENABLED) != + (CapturedPrivileges[j].Attributes & SE_PRIVILEGE_ENABLED)) + { + DPRINT("Attributes differ\n"); + DPRINT("Current attributes %lx New attributes %lx\n", + Token->Privileges[i].Attributes, + CapturedPrivileges[j].Attributes); + + ChangeCount++; + } + } + } + } + } + + /* + * Return the required buffer size and + * check if the available buffer is large enough + */ + if (PreviousState != NULL) + { + ULONG RequiredLength = (ULONG)sizeof(TOKEN_PRIVILEGES) + + ((ChangeCount - ANYSIZE_ARRAY) * (ULONG)sizeof(LUID_AND_ATTRIBUTES)); + + /* Try to return the required buffer length */ + _SEH2_TRY + { + *ReturnLength = RequiredLength; + } + _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) + { + /* Return the exception code */ + _SEH2_YIELD(return _SEH2_GetExceptionCode()); + } + _SEH2_END; + + /* Fail, if the buffer length is smaller than the required length */ + if (BufferLength < RequiredLength) + { + ObDereferenceObject(Token); + if (CapturedPrivileges != NULL) + SeReleaseLuidAndAttributesArray(CapturedPrivileges, + PreviousMode, + TRUE); + + return STATUS_BUFFER_TOO_SMALL; + } + } +
if (PreviousState != NULL) PreviousState->PrivilegeCount = 0; @@ -2091,27 +2146,16 @@ { for (i = 0; i < Token->PrivilegeCount; i++) { - if (Token->Privileges[i].Attributes != 0) - { - DPRINT ("Attributes differ\n"); + if (Token->Privileges[i].Attributes & SE_PRIVILEGE_ENABLED) + { + DPRINT ("Attributes enabled\n");
/* Save current privilege */ if (PreviousState != NULL) { - if (k < PrivilegeCount) - { - PreviousState->PrivilegeCount++; - PreviousState->Privileges[k].Luid = Token->Privileges[i].Luid; - PreviousState->Privileges[k].Attributes = Token->Privileges[i].Attributes; - } - else - { - /* - * FIXME: Should revert all the changes, calculate how - * much space would be needed, set ResultLength - * accordingly and fail. - */ - } + PreviousState->PrivilegeCount++; + PreviousState->Privileges[k].Luid = Token->Privileges[i].Luid; + PreviousState->Privileges[k].Attributes = Token->Privileges[i].Attributes;
k++; } @@ -2139,27 +2183,16 @@ (CapturedPrivileges[j].Attributes & SE_PRIVILEGE_ENABLED)) { DPRINT ("Attributes differ\n"); - DPRINT ("Current attributes %lx desired attributes %lx\n", + DPRINT ("Current attributes %lx New attributes %lx\n", Token->Privileges[i].Attributes, CapturedPrivileges[j].Attributes);
/* Save current privilege */ if (PreviousState != NULL) { - if (k < PrivilegeCount) - { - PreviousState->PrivilegeCount++; - PreviousState->Privileges[k].Luid = Token->Privileges[i].Luid; - PreviousState->Privileges[k].Attributes = Token->Privileges[i].Attributes; - } - else - { - /* - * FIXME: Should revert all the changes, calculate how - * much space would be needed, set ResultLength - * accordingly and fail. - */ - } + PreviousState->PrivilegeCount++; + PreviousState->Privileges[k].Luid = Token->Privileges[i].Luid; + PreviousState->Privileges[k].Attributes = Token->Privileges[i].Attributes;
k++; } @@ -2177,14 +2210,9 @@ } }
- Status = Count < CapturedCount ? STATUS_NOT_ALL_ASSIGNED : STATUS_SUCCESS; - } - - if (ReturnLength != NULL) - { - *ReturnLength = sizeof(TOKEN_PRIVILEGES) + - (sizeof(LUID_AND_ATTRIBUTES) * (k - 1)); - } + Status = (Count < CapturedCount) ? STATUS_NOT_ALL_ASSIGNED : STATUS_SUCCESS; + } +
/* Dereference the token */ ObDereferenceObject (Token);