[PATCH V2] advapi32/tests: Add additional tests for NtAccessCheck

[Zhiyi Zhang]

On 4/24/19 11:06 PM, Vijay Kiran Kamuju wrote:
> Add todo_wine for failing tests
>
> From: Vijay Kiran Kamuju <infyquest at gmail.com>
> Signed-off-by: Vijay Kiran Kamuju <infyquest at gmail.com>
> ---
>  dlls/advapi32/tests/security.c | 35 +++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
> index d9cae64da8b..7534e007915 100644
> --- a/dlls/advapi32/tests/security.c
> +++ b/dlls/advapi32/tests/security.c
> @@ -1343,11 +1343,13 @@ static void test_AccessCheck(void)
>  
>      if(pNtAccessCheck)
>      {
> +       DWORD ntPrivSetLen = sizeof(PRIVILEGE_SET);
> +
>         /* Generic access mask - no privilegeset buffer */
>         SetLastError(0xdeadbeef);
>         Access = ntAccessStatus = 0x1abe11ed;
>         ntret = pNtAccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping,
> -                              NULL, &PrivSetLen, &Access, &ntAccessStatus);
> +                              NULL, &ntPrivSetLen, &Access, &ntAccessStatus);
>         err = GetLastError();
>         ok(ntret == STATUS_ACCESS_VIOLATION,
>            "NtAccessCheck should have failed with STATUS_ACCESS_VIOLATION, got %x\n", ntret);
> @@ -1355,6 +1357,7 @@ static void test_AccessCheck(void)
>            "NtAccessCheck shouldn't set last error, got %d\n", err);
>         ok(Access == 0x1abe11ed && ntAccessStatus == 0x1abe11ed,
>            "Access and/or AccessStatus were changed!\n");
> +       ok(ntPrivSetLen == sizeof(PRIVILEGE_SET), "PrivSetLen returns %d\n", ntPrivSetLen);
>  
>        /* Generic access mask - no returnlength */
>        SetLastError(0xdeadbeef);
> @@ -1381,6 +1384,36 @@ static void test_AccessCheck(void)
>           "NtAccessCheck shouldn't set last error, got %d\n", err);
>        ok(Access == 0x1abe11ed && ntAccessStatus == 0x1abe11ed,
>           "Access and/or AccessStatus were changed!\n");
> +
> +      /* Generic access mask - zero returnlength */
> +      SetLastError(0xdeadbeef);
> +      Access = ntAccessStatus = 0x1abe11ed;
> +	  ntPrivSetLen = 0;
Formating.

> +      ntret = pNtAccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping,
> +                             PrivSet, &ntPrivSetLen, &Access, &ntAccessStatus);
> +      err = GetLastError();
> +      ok(ntret == STATUS_GENERIC_NOT_MAPPED,
> +         "NtAccessCheck should have failed with STATUS_GENERIC_NOT_MAPPED, got %x\n", ntret);
> +      ok(err == 0xdeadbeef,
> +         "NtAccessCheck shouldn't set last error, got %d\n", err);
> +      ok(Access == 0x1abe11ed && ntAccessStatus == 0x1abe11ed,
> +         "Access and/or AccessStatus were changed!\n");
> +      todo_wine ok(ntPrivSetLen == 0, "PrivSetLen returns %d\n", ntPrivSetLen);
> +
> +      /* Generic access mask - insufficient returnlength */
> +      SetLastError(0xdeadbeef);
> +      Access = ntAccessStatus = 0x1abe11ed;
> +	  ntPrivSetLen = sizeof(PRIVILEGE_SET)-1;
> +      ntret = pNtAccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping,
> +                             PrivSet, &ntPrivSetLen, &Access, &ntAccessStatus);
> +      err = GetLastError();
> +      ok(ntret == STATUS_GENERIC_NOT_MAPPED,
This doesn't look right. You are testing the return length. Make sure other parameters are valid.
This way we know the error is indeed from the invalid length.

And I think we should move NtAccessCheck test into ntdll tests if it is not too much trouble.
> +         "NtAccessCheck should have failed with STATUS_GENERIC_NOT_MAPPED, got %x\n", ntret);
> +      ok(err == 0xdeadbeef,
> +         "NtAccessCheck shouldn't set last error, got %d\n", err);
> +      ok(Access == 0x1abe11ed && ntAccessStatus == 0x1abe11ed,
> +         "Access and/or AccessStatus were changed!\n");
> +      todo_wine ok(ntPrivSetLen == sizeof(PRIVILEGE_SET)-1, "PrivSetLen returns %d\n", ntPrivSetLen);
>      }
>      else
>         win_skip("NtAccessCheck unavailable. Skipping.\n");