From: Piotr Caban <piotr@codeweavers.com>
Subject: ntdll: Improve parameter validation in RtlAddAce
Message-Id: <5516BA1D.6020609@codeweavers.com>
Date: Sat, 28 Mar 2015 15:26:37 +0100

---
  dlls/advapi32/tests/security.c | 46 
++++++++++++++++++++++++++++++++++++++++++
  dlls/ntdll/sec.c               |  4 +++-
  2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index e7a992c..a3690e8 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -5569,6 +5569,51 @@ static void test_AdjustTokenPrivileges(void)
     CloseHandle(token);
 }
 
+static void test_AddAce(void)
+{
+    static SID const sidWorld = { SID_REVISION, 1, { SECURITY_WORLD_SID_AUTHORITY} , { SECURITY_WORLD_RID } };
+
+    char acl_buf[1024], ace_buf[256];
+    ACCESS_ALLOWED_ACE *ace = (ACCESS_ALLOWED_ACE*)ace_buf;
+    PACL acl = (PACL)acl_buf;
+    BOOL ret;
+
+    memset(ace, 0, sizeof(ace_buf));
+    ace->Header.AceType = ACCESS_ALLOWED_ACE_TYPE;
+    ace->Header.AceSize = sizeof(ACCESS_ALLOWED_ACE)-sizeof(DWORD)+sizeof(SID);
+    memcpy(&ace->SidStart, &sidWorld, sizeof(sidWorld));
+
+    ret = InitializeAcl(acl, sizeof(acl_buf), ACL_REVISION2);
+    ok(ret, "InitializeAcl failed: %d\n", GetLastError());
+
+    ret = AddAce(acl, ACL_REVISION1, MAXDWORD, ace, ace->Header.AceSize);
+    ok(ret, "AddAce failed: %d\n", GetLastError());
+    ret = AddAce(acl, ACL_REVISION2, MAXDWORD, ace, ace->Header.AceSize);
+    ok(ret, "AddAce failed: %d\n", GetLastError());
+    ret = AddAce(acl, ACL_REVISION3, MAXDWORD, ace, ace->Header.AceSize);
+    ok(ret, "AddAce failed: %d\n", GetLastError());
+    ok(acl->AclRevision == ACL_REVISION3, "acl->AclRevision = %d\n", acl->AclRevision);
+    ret = AddAce(acl, ACL_REVISION4, MAXDWORD, ace, ace->Header.AceSize);
+    ok(ret, "AddAce failed: %d\n", GetLastError());
+    ok(acl->AclRevision == ACL_REVISION4, "acl->AclRevision = %d\n", acl->AclRevision);
+    ret = AddAce(acl, ACL_REVISION1, MAXDWORD, ace, ace->Header.AceSize);
+    ok(ret, "AddAce failed: %d\n", GetLastError());
+    ok(acl->AclRevision == ACL_REVISION4, "acl->AclRevision = %d\n", acl->AclRevision);
+    ret = AddAce(acl, ACL_REVISION2, MAXDWORD, ace, ace->Header.AceSize);
+    ok(ret, "AddAce failed: %d\n", GetLastError());
+
+    ret = AddAce(acl, MIN_ACL_REVISION-1, MAXDWORD, ace, ace->Header.AceSize);
+    ok(ret, "AddAce failed: %d\n", GetLastError());
+    /* next test succeededs but corrupts ACL */
+    ret = AddAce(acl, MAX_ACL_REVISION+1, MAXDWORD, ace, ace->Header.AceSize);
+    ok(ret, "AddAce failed: %d\n", GetLastError());
+    ok(acl->AclRevision == MAX_ACL_REVISION+1, "acl->AclRevision = %d\n", acl->AclRevision);
+    SetLastError(0xdeadbeef);
+    ret = AddAce(acl, ACL_REVISION1, MAXDWORD, ace, ace->Header.AceSize);
+    ok(!ret, "AddAce succeeded\n");
+    ok(GetLastError() == ERROR_INVALID_PARAMETER, "GetLastError() = %d\n", GetLastError());
+}
+
 START_TEST(security)
 {
     init();
@@ -5610,4 +5655,5 @@ START_TEST(security)
     test_TokenIntegrityLevel();
     test_default_dacl_owner_sid();
     test_AdjustTokenPrivileges();
+    test_AddAce();
 }
diff --git a/dlls/ntdll/sec.c b/dlls/ntdll/sec.c
index b733f77..0917780 100644
--- a/dlls/ntdll/sec.c
+++ b/dlls/ntdll/sec.c
@@ -1167,7 +1167,7 @@ NTSTATUS WINAPI RtlAddAce(
 	PACE_HEADER	ace,targetace;
 	int		nrofaces;
 
-	if (acl->AclRevision != ACL_REVISION)
+	if (!RtlValidAcl(acl))
 		return STATUS_INVALID_PARAMETER;
 	if (!RtlFirstFreeAce(acl,&targetace))
 		return STATUS_INVALID_PARAMETER;
@@ -1180,6 +1180,8 @@ NTSTATUS WINAPI RtlAddAce(
 		return STATUS_INVALID_PARAMETER;
 	memcpy(targetace,acestart,acelen);
 	acl->AceCount+=nrofaces;
+	if (rev > acl->AclRevision)
+		acl->AclRevision = rev;
 	return STATUS_SUCCESS;
 }