[PATCH v3] sechost: Support decimal or octal access rights in ConvertStringSecurityDescriptorToSecurityDescriptor.

[Dmitry Timoshkov]

v3: Add more tests with leading zero.

Signed-off-by: Dmitry Timoshkov 
---
 dlls/advapi32/tests/security.c | 22 +++++++++++++++++++---
 dlls/sechost/security.c        |  3 +++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index ed91ccc39d3..a47c4b2d529 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -4219,6 +4219,7 @@ static void test_ConvertStringSecurityDescriptor(void)
         BOOL       ret;
         DWORD      GLE;
         DWORD      altGLE;
+        DWORD      ace_Mask;
     } cssd[] =
     {
         { "D:(A;;GA;;;WD)",                  0xdeadbeef,      FALSE, ERROR_UNKNOWN_REVISION },
@@ -4239,9 +4240,11 @@ static void test_ConvertStringSecurityDescriptor(void)
         { "D:(D;;GA;;; WD)",                SDDL_REVISION_1, TRUE },
         { "D:(D;;GA;;;WD )",                SDDL_REVISION_1, TRUE },
         /* test ACE string access rights */
-        { "D:(A;;GA;;;WD)",                  SDDL_REVISION_1, TRUE },
-        { "D:(A;;GRGWGX;;;WD)",              SDDL_REVISION_1, TRUE },
-        { "D:(A;;RCSDWDWO;;;WD)",            SDDL_REVISION_1, TRUE },
+        { "D:(A;;GA;;;WD)",                  SDDL_REVISION_1, TRUE, 0, 0, GENERIC_ALL },
+        { "D:(A;;1;;;WD)",                   SDDL_REVISION_1, TRUE, 0, 0, 1 },
+        { "D:(A;;020000000000;;;WD)",        SDDL_REVISION_1, TRUE, 0, 0, GENERIC_READ },
+        { "D:(A;;GRGWGX;;;WD)",              SDDL_REVISION_1, TRUE, 0, 0, GENERIC_READ | GENERIC_WRITE | GENERIC_EXECUTE },
+        { "D:(A;;RCSDWDWO;;;WD)",            SDDL_REVISION_1, TRUE, 0, 0, READ_CONTROL | DELETE | WRITE_DAC | WRITE_OWNER },
         { "D:(A;;RPWPCCDCLCSWLODTCR;;;WD)",  SDDL_REVISION_1, TRUE },
         { "D:(A;;FAFRFWFX;;;WD)",            SDDL_REVISION_1, TRUE },
         { "D:(A;;KAKRKWKX;;;WD)",            SDDL_REVISION_1, TRUE },
@@ -4275,7 +4278,20 @@ static void test_ConvertStringSecurityDescriptor(void)
                (cssd[i].altGLE && GLE == cssd[i].altGLE),
                "(%02u) Unexpected last error %ld\n", i, GLE);
         if (ret)
+        {
+            if (cssd[i].ace_Mask)
+            {
+                ACCESS_ALLOWED_ACE *ace;
+
+                acl = (ACL *)((char *)pSD + sizeof(SECURITY_DESCRIPTOR_RELATIVE));
+                ok(acl->AclRevision == ACL_REVISION, "(%02u) Got %u\n", i, acl->AclRevision);
+
+                ace = (ACCESS_ALLOWED_ACE *)(acl + 1);
+                ok(ace->Mask == cssd[i].ace_Mask, "(%02u) Expected %08lx, got %08lx\n",
+                   i, cssd[i].ace_Mask, ace->Mask);
+            }
             LocalFree(pSD);
+        }
     }

/* test behaviour with NULL parameters */
diff --git a/dlls/sechost/security.c b/dlls/sechost/security.c
index 6916b3ad662..8a8ca0f051a 100644
--- a/dlls/sechost/security.c
+++ b/dlls/sechost/security.c
@@ -903,6 +903,9 @@ static DWORD parse_ace_right( const WCHAR **string_ptr )
     if (string[0] == '0' && string[1] == 'x')
         return wcstoul( string, (WCHAR **)string_ptr, 16 );

+    if (iswdigit( string[0] ))
+        return wcstoul( string, (WCHAR **)string_ptr, 0 );
+
     for (i = 0; i < ARRAY_SIZE(ace_rights); ++i)
     {
         if (!wcsncmp( string, ace_rights[i].str, 2 ))

[Zebediah Figura]

On 7/6/22 07:30, Dmitry Timoshkov wrote:
> diff --git a/dlls/sechost/security.c b/dlls/sechost/security.c
> index 6916b3ad662..8a8ca0f051a 100644
> --- a/dlls/sechost/security.c
> +++ b/dlls/sechost/security.c
> @@ -903,6 +903,9 @@ static DWORD parse_ace_right( const WCHAR **string_ptr )
>       if (string[0] == '0' && string[1] == 'x')
>           return wcstoul( string, (WCHAR **)string_ptr, 16 );
> - if (iswdigit( string[0] ))
> -
>    return wcstoul( string, (WCHAR **)string_ptr, 0 );
> -
> for (i = 0; i < ARRAY_SIZE(ace_rights); ++i)
> {
>     if (!wcsncmp( string, ace_rights[i].str, 2 ))

Doesn't that mean we can combine this with the hexadecimal case above?

[Dmitry Timoshkov]

Zebediah Figura  wrote:
> On 7/6/22 07:30, Dmitry Timoshkov wrote:
> > diff --git a/dlls/sechost/security.c b/dlls/sechost/security.c
> > index 6916b3ad662..8a8ca0f051a 100644
> > --- a/dlls/sechost/security.c
> > +++ b/dlls/sechost/security.c
> > @@ -903,6 +903,9 @@ static DWORD parse_ace_right( const WCHAR **string_ptr )
> >       if (string[0] == '0' && string[1] == 'x')
> >           return wcstoul( string, (WCHAR **)string_ptr, 16 );
> > - if (iswdigit( string[0] ))
> > -
> >    return wcstoul( string, (WCHAR **)string_ptr, 0 );
> > -
> > for (i = 0; i < ARRAY_SIZE(ace_rights); ++i)
> > {
> >     if (!wcsncmp( string, ace_rights[i].str, 2 ))
>
> Doesn't that mean we can combine this with the hexadecimal case above?

I also had a temptation to merge both cases, however I decided to leave
them separate, preferring a separate patch if really necessary.