[NTOSKRNL]

- Check the SeTakeOwnership privilege only if WRITE_OWNER access is desired.
- Move the check for token ownership from SepAccessCheck because this check grants access rights rather than checking them.

svn path=/trunk/; revision=46683

reactos/ntoskrnl/se/semgr.c

@@ -314,6 +314,31 @@ SepSidInToken(PACCESS_TOKEN _Token,
     return FALSE;
 }
 
+static BOOLEAN
+SepTokenIsOwner(PACCESS_TOKEN Token,
+                PSECURITY_DESCRIPTOR SecurityDescriptor)
+{
+    NTSTATUS Status;
+    PSID Sid = NULL;
+    BOOLEAN Defaulted;
+
+    Status = RtlGetOwnerSecurityDescriptor(SecurityDescriptor,
+                                           &Sid,
+                                           &Defaulted);
+    if (!NT_SUCCESS(Status))
+    {
+        DPRINT1("RtlGetOwnerSecurityDescriptor() failed (Status %lx)\n", Status);
+        return FALSE;
+    }
+
+    if (Sid == NULL)
+    {
+        DPRINT1("Owner Sid is NULL\n");
+        return FALSE;
+    }
+
+    return SepSidInToken(Token, Sid);
+}
 
 VOID NTAPI
 SeQuerySecurityAccessMask(IN SECURITY_INFORMATION SecurityInformation,
@@ -438,22 +463,25 @@ SepAccessCheck(IN PSECURITY_DESCRIPTOR SecurityDescriptor,
     CurrentAccess = PreviouslyGrantedAccess;
 
     /* RULE 2: Check token for 'take ownership' privilege */
-    Privilege.Luid = SeTakeOwnershipPrivilege;
+    if (DesiredAccess & WRITE_OWNER)
-    Privilege.Attributes = SE_PRIVILEGE_ENABLED;
-
-    if (SepPrivilegeCheck(Token,
-                          &Privilege,
-                          1,
-                          PRIVILEGE_SET_ALL_NECESSARY,
-                          AccessMode))
     {
-        CurrentAccess |= WRITE_OWNER;
+        Privilege.Luid = SeTakeOwnershipPrivilege;
-        if ((DesiredAccess & ~VALID_INHERIT_FLAGS) == 
+        Privilege.Attributes = SE_PRIVILEGE_ENABLED;
-            (CurrentAccess & ~VALID_INHERIT_FLAGS))
+
+        if (SepPrivilegeCheck(Token,
+                              &Privilege,
+                              1,
+                              PRIVILEGE_SET_ALL_NECESSARY,
+                              AccessMode))
         {
-            *GrantedAccess = CurrentAccess;
+            CurrentAccess |= WRITE_OWNER;
-            *AccessStatus = STATUS_SUCCESS;
+            if ((DesiredAccess & ~VALID_INHERIT_FLAGS) == 
-            return TRUE;
+                (CurrentAccess & ~VALID_INHERIT_FLAGS))
+            {
+                *GrantedAccess = CurrentAccess;
+                *AccessStatus = STATUS_SUCCESS;
+                return TRUE;
+            }
         }
     }
 
@@ -465,29 +493,6 @@ SepAccessCheck(IN PSECURITY_DESCRIPTOR SecurityDescriptor,
         return FALSE;
     }
 
-    /* RULE 3: Check whether the token is the owner */
-    Status = RtlGetOwnerSecurityDescriptor(SecurityDescriptor,
-                                           &Sid,
-                                           &Defaulted);
-    if (!NT_SUCCESS(Status))
-    {
-        DPRINT1("RtlGetOwnerSecurityDescriptor() failed (Status %lx)\n", Status);
-        *AccessStatus = Status;
-        return FALSE;
-    }
-
-    if (Sid && SepSidInToken(Token, Sid))
-    {
-        CurrentAccess |= (READ_CONTROL | WRITE_DAC);
-        if ((DesiredAccess & ~VALID_INHERIT_FLAGS) == 
-            (CurrentAccess & ~VALID_INHERIT_FLAGS))
-        {
-            *GrantedAccess = CurrentAccess;
-            *AccessStatus = STATUS_SUCCESS;
-            return TRUE;
-        }
-    }
-
     /* Fail if DACL is absent */
     if (Present == FALSE)
     {
@@ -649,16 +654,43 @@ SeAccessCheck(IN PSECURITY_DESCRIPTOR SecurityDescriptor,
     if (!SubjectContextLocked)
         SeLockSubjectContext(SubjectSecurityContext);
 
-    /* Call the internal function */
+    /* Check if the token is the owner and grant WRITE_DAC and READ_CONTROL rights */
-    ret = SepAccessCheck(SecurityDescriptor,
+    if (DesiredAccess & (WRITE_DAC | READ_CONTROL | MAXIMUM_ALLOWED))
-                         SubjectSecurityContext,
+    {
-                         DesiredAccess,
+         PACCESS_TOKEN Token = SubjectSecurityContext->ClientToken ?
-                         PreviouslyGrantedAccess,
+             SubjectSecurityContext->ClientToken : SubjectSecurityContext->PrimaryToken;
-                         Privileges,
+
-                         GenericMapping,
+        if (SepTokenIsOwner(Token,
-                         AccessMode,
+                            SecurityDescriptor))
-                         GrantedAccess,
+        {
-                         AccessStatus);
+            if (DesiredAccess & MAXIMUM_ALLOWED)
+                PreviouslyGrantedAccess |= (WRITE_DAC | READ_CONTROL);
+            else
+                PreviouslyGrantedAccess |= (DesiredAccess & (WRITE_DAC | READ_CONTROL));
+
+            DesiredAccess &= ~(WRITE_DAC | READ_CONTROL);
+        }
+    }
+
+    if (DesiredAccess == 0)
+    {
+        *GrantedAccess = PreviouslyGrantedAccess;
+        *AccessStatus = STATUS_SUCCESS;
+        ret = TRUE;
+    }
+    else
+    {
+        /* Call the internal function */
+        ret = SepAccessCheck(SecurityDescriptor,
+                             SubjectSecurityContext,
+                             DesiredAccess,
+                             PreviouslyGrantedAccess,
+                             Privileges,
+                             GenericMapping,
+                             AccessMode,
+                             GrantedAccess,
+                             AccessStatus);
+    }
 
     /* Release the lock if needed */
     if (!SubjectContextLocked)
@@ -686,6 +718,7 @@ NtAccessCheck(IN PSECURITY_DESCRIPTOR SecurityDescriptor,
     PSECURITY_DESCRIPTOR CapturedSecurityDescriptor = NULL;
     SECURITY_SUBJECT_CONTEXT SubjectSecurityContext;
     KPROCESSOR_MODE PreviousMode = ExGetPreviousMode();
+    ACCESS_MASK PreviouslyGrantedAccess = 0;
     PTOKEN Token;
     NTSTATUS Status;
     PAGED_CODE();
@@ -801,16 +834,38 @@ NtAccessCheck(IN PSECURITY_DESCRIPTOR SecurityDescriptor,
     SubjectSecurityContext.ProcessAuditId = NULL;
     SeLockSubjectContext(&SubjectSecurityContext);
 
-    /* Now perform the access check */
+    /* Check if the token is the owner and grant WRITE_DAC and READ_CONTROL rights */
-    SepAccessCheck(SecurityDescriptor, // FIXME: use CapturedSecurityDescriptor
+    if (DesiredAccess & (WRITE_DAC | READ_CONTROL | MAXIMUM_ALLOWED))
-                   &SubjectSecurityContext,
+    {
-                   DesiredAccess,
+        if (SepTokenIsOwner(Token, SecurityDescriptor)) // FIXME: use CapturedSecurityDescriptor
-                   0,
+        {
-                   &PrivilegeSet, //FIXME
+            if (DesiredAccess & MAXIMUM_ALLOWED)
-                   GenericMapping,
+                PreviouslyGrantedAccess |= (WRITE_DAC | READ_CONTROL);
-                   PreviousMode,
+            else
-                   GrantedAccess,
+                PreviouslyGrantedAccess |= (DesiredAccess & (WRITE_DAC | READ_CONTROL));
-                   AccessStatus);
+
+            DesiredAccess &= ~(WRITE_DAC | READ_CONTROL);
+        }
+    }
+
+    if (DesiredAccess == 0)
+    {
+        *GrantedAccess = PreviouslyGrantedAccess;
+        *AccessStatus = STATUS_SUCCESS;
+    }
+    else
+    {
+        /* Now perform the access check */
+        SepAccessCheck(SecurityDescriptor, // FIXME: use CapturedSecurityDescriptor
+                       &SubjectSecurityContext,
+                       DesiredAccess,
+                       PreviouslyGrantedAccess,
+                       &PrivilegeSet, //FIXME
+                       GenericMapping,
+                       PreviousMode,
+                       GrantedAccess,
+                       AccessStatus);
+    }
 
     /* Unlock subject context */
     SeUnlockSubjectContext(&SubjectSecurityContext);