Investigating a problem constructing a security descriptor to deny thread-specific access rights to Everyone


A customer had a question about how to set up a security access mask.

How can I deny thread-specific access rights to Everyone?

Specifically, I want to deny the Everyone group the following rights when I create a process:

  • THREAD_SET_INFORMATION
  • THREAD_SET_THREAD_TOKEN
  • THREAD_TERMINATE
  • PROCESS_CREATE_PROCESS
  • PROCESS_SET_SESSION­ID
  • PROCESS_VM_OPERATION
  • PROCESS_VM_WRITE

How do I create the access mask for this? Will this function work?

DWORD GetDeniedMask()
{
  DWORD accessMask = 0;
  GENERIC_MAPPING genmap;
  genmap.GenericRead = WRITE_DAC | WRITE_OWNER;
  genmap.GenericWrite = WRITE_DAC | WRITE_OWNER;
  genmap.GenericExecute = WRITE_DAC | WRITE_OWNER;
  genmap.GenericAll = WRITE_DAC | WRITE_OWNER |
    THREAD_SET_INFORMATION |
    THREAD_SET_THREAD_TOKEN |
    THREAD_TERMINATE |
    PROCESS_CREATE_PROCESS |
    PROCESS_SET_SESSIONID |
    PROCESS_VM_OPERATION |
    PROCESS_VM_WRITE;
  MapGenericMask(&accessMask, &genmap);
  return accessMask;
}

This question is so confused it's hard to say where to start.

Why are you trying to deny these accesses to Everyone? Note that Everyone includes the owner of the process, which means that the owner of the process can't even terminate his own process! Furthermore, many normal operations need accesses like the ones you are denying. You are going to end up with a process that can't do much, not even to itself. For example, the Virtual­Alloc function needs PROCESS_VM_OPERATION access.¹ A process that can't allocate any memory is not going to get very far. Some of these accesses are needed by the process creator in order to do things like set up the initial environment and command line. And anti-malware software is going to block the creation of any process that refuses to let the anti-malware software inspect it!

The customer explained,

A security audit uncovered that our processes granted the rights listed above to Everyone, so we are seeing what we can do to deny those rights to Everyone while still allowing those rights to the creator and people in the right security group.

Is it a security risk to grant the above listed rights to Everyone? If so, how do we deny them to Everyone while still allowing it to the right people? We assume we need to pass custom security attributes as the lpProcess­Attributes and lpThread­Attributes parameters when we call the Create­Process function, but we need help building those security attributes.

Since deny actions override allow actions,² you can't deny something to Everyone, and then grant it to a special subgroup. The deny on Everyone will override the allow on the subgroup.

The way to do this is not to deny Everyone, but rather to stop allowing Everyone. A security principal receives access if there is an applicable allow rule and no applicable deny rule.² So remove the spurious allow rule.

Actually, where is the allow rule for Everyone coming from? The default process security does not grant Everyone those accesses. The customer must be doing something unusual.

Here is the code we are using to set the security attributes on the process. [I have converted the long C++ code into equivalent pseudo-C# code for readability. -Raymond]

var acl = new AccessControlList();

// Deny some accesses to AU and WD.
var deniedMask = GetDeniedMask();
acl.AddDenyAce(AuthenticatedUsersSid, deniedMask);
acl.AddDenyAce(WorldSid, deniedMask);

// Grant some accesses to AU and WD.
var worldMask = GetAllowedMask();
acl.AddAllowAce(AuthenticatedUsersSid, worldMask);
acl.AddAllowAce(WorldSid, worldMask);

It's not clear why they denied and granted identical accesses both to Authenticated Users and to Everyone. (aka World). Since Authenticated Users are a subset of Everyone, all the rules for Authenticated Users are redundant.

We need to peel away yet another layer of the onion. What is the custom access mask being granted to Everyone?

Here is the Get­Allowed­Mask function.

DWORD GetAllowedMask()
{
 DWORD accessMask = GENERIC_READ | GENERIC_EXECUTE;
 GENERIC_MAPPING genmap;
 genmap.GenericRead = GENERIC_READ |
                      FILE_GENERIC_READ |
                      SECTION_MAP_READ;
 genmap.GenericWrite = 0;
 genmap.GenericExecute = GENERIC_EXECUTE |
                         FILE_GENERIC_EXECUTE |
                         SECTION_MAP_EXECUTE;
 genmap.GenericAll = GENERIC_READ |
                     GENERIC_EXECUTE ;
 MapGenericMask(&accessMask, &genmap);
 return accessMask;
}

Here we see the same confusion that started the whole thing.

The customer appears not to understand what the Map­Generic­Mask function does, or what it is for.

I will pause now so you can review my earlier discussion of the Map­Generic­Mask function, what it does, and its intended usage pattern.

Welcome back. If you read and understood that article, you'll observe that this customer completely misses the point of the Map­Generic­Mask function. They are using it to calculate information on the client side. But if you're on the client side, you don't need to convert GENERIC_READ to a specific mask. That's the server's job! Just ask for generic access and go home.

Anyway, let's see what happens. The Get­World­Access­Mask function is passing a hard-coded access mask and a hard-coded generic mapping. We can walk through the code ourselves to see what happens.

  • We start with GENERIC_READ | GENERIC_EXECUTE.

  • Since GENERIC_READ is set, we remove it and replace it with genmap.Generic­Read, which is GENERIC_READ | FILE_GENERIC_READ | SECTION_MAP_READ, resulting in GENERIC_READ | FILE_GENERIC_READ | SECTION_MAP_READ | GENERIC_EXECUTE.

Hey, wait a second. We're making things worse! The whole point of Map­Generic­Mask is to get rid of generic mappings, but this genmap structure says, "To get rid of GENERIC_READ, convert it to this other stuff that includes GENERIC_READ."

This is like reading some tips on how to rid your room of outdated clutter, and one of them says, "If you see an old magazine, you can get rid of it by putting a fern next to it." Um, that's not actually getting rid of anything. You just added more stuff.

  • Since GENERIC_WRITE is not set, nothing is done with Generic­Write.

  • Since GENERIC_EXECUTE is set, we remove it and replace it with genmap.Generic­Execute, which is GENERIC_EXECUTE | FILE_GENERIC_EXECUTE | SECTION_MAP_EXECUTE, resulting in GENERIC_READ | FILE_GENERIC_READ | SECTION_MAP_READ | GENERIC_EXECUTE | FILE_GENERIC_EXECUTE | SECTION_MAP_EXECUTE.

  • Since GENERIC_ALL is not set, nothing is done with Generic­All.

  • Finally, the Map­Generic­Mask function removes all generic access bits, because it promises never to return any generic access bits.

The result of all these shenanigans is that we are granting Everyone the follow access mask:

GENERIC_READ 0x80000000
FILE_GENERIC_READ =
STANDARD_RIGHTS_READ |
FILE_READ_DATA |
FILE_READ_ATTRIBUTES |
FILE_READ_EA |
SYNCHRONIZE

0x00120089 =
0x00020000 |
0x00000001 |
0x00000080 |
0x00000008 |
0x00100000
SECTION_MAP_READ 0x00000004
GENERIC_EXECUTE 0x20000000
FILE_GENERIC_EXECUTE =
STANDARD_RIGHTS_EXECUTE |
FILE_READ_ATTRIBUTES |
FILE_EXECUTE |
SYNCHRONIZE
0x001200A0 =
0x00020000 |
0x00000080 |
0x00000020 |
0x00100000

SECTION_MAP_EXECUTE 0x00000008
Grand total 0x001200AD

Actually, this mask makes no sense. It is combining file-specific access masks and section-specific access masks. And then applying them to a process and a thread! (Background reading.)

This is like looking at the menu for a Chinese restaurant and deciding that you want the Number 21 (cashew chicken), then looking at the menu for an Indian restaurant and deciding that you want the Number 18 (saag paneer), then calling a Greek restaurant, ordering the number 21 and 18 for take-out, then calling a Thai restaurant, and ordering the number 21 and 18 for take-out. And then you wonder why the Greek restaurant gave you a moussaka and a pork souvlaki, and the Thai restaurant gave you a phad thai and a yam nua. Where's your cashew chicken and saag paneer?

Let's see what Greek food we ended up ordering by accident.

0x00020000 |
0x00000001 |
0x00000080 |
0x00000008 |
0x00100000

READ_DAC |
PROCESS_TERMINATE |
PROCESS_CREATE_PROCESS |
PROCESS_VM_OPERATION |
SYNCHRONIZE

0x00000004

PROCESS_SET_SESSION­ID
0x00020000 |
0x00000080 |
0x00000020 |
0x00100000

READ_DAC |
PROCESS_CREATE_PROCESS |
PROCESS_VM_WRITE |
SYNCHRONIZE
0x00000008

PROCESS_VM_OPERATION

Well, that explains why the process grants so many weird accesses to Everyone: Because you're granting all these weird accesses to Everyone!

I think we can predict what Thai food we ordered by accident, but let's do the math.

0x00020000 |
0x00000001 |
0x00000080 |
0x00000008 |
0x00100000

READ_DAC |
THREAD_TERMINATE |
THREAD_SET_THREAD_TOKEN |
THREAD_GET_CONTEXT |
SYNCHRONIZE

0x00000004

???? undefined ????
0x00020000 |
0x00000080 |
0x00000020 |
0x00100000

READ_DAC |
THREAD_SET_THREAD_TOKEN |
THREAD_SET_INFORMATION |
SYNCHRONIZE
0x00000008

THREAD_GET_CONTEXT

From reading the confused code, it appears that the customer wants to grant read and execute rights to Everyone, but it's not clear why. In particular, Execute rights don't have intrinsic meaning for most types of objects, aside from files (to see if you can execute them) and memory (to see if you can execute code from them). Consequently, many object types overload Execute to mean something else. For example, our Gizmo object overloads Execute to mean start/stop.

If it's the case that the customer merely wants to grant permission to execute the program to Everyone, then that's done by applying the ACL to the executable file itself.

Assuming the presumption above is true, then the solution to the customer's problem is simple: Delete all the code that tries to create a custom security descriptor and just pass NULL as the security descriptor for the process and thread. This creates the process with default security, which is just fine for what you want.

The customer wrote back,

Thanks. This is code we acquired recently, and the code base is so old that nobody knows exactly what this custom security attribute is trying to do.

¹ But you luck out because Get­Current­Process returns a handle with full access, so the ACLs on the process object don't get a chance to flex their muscles if the process is talking about itself.

² Reality is more complex than this simple statement, but the details are not important to the story. The statements are true enough.

Comments (15)
  1. Henke37 says:

    Rule number one for working with access masks: they have to be for the correct object type.

  2. Joshua says:

    Make the process deny-all for everyone, but start it suspended. Then track down the process's original handle to itself and replace it w/ yours.

  3. Wyatt says:

    My brain hurts.

  4. DWalker says:

    Well, the customer's reply explains a lot!  I have often said that good commenting helps a lot.  

    If the original coder would have added comments saying what he was TRYING to do, or WHY he was setting those masks, then the customer who acquired the code might have had a clue.  Although I predict that the original coder didn't know what he was doing; maybe he cobbled it together from examples he found elsewhere.

    If you don't know a piece of code well enough to write a meaningful comment explaining WHY you are doing what you are doing, then you don't understand the code well enough to include it in your program.

  5. Mark says:

    Your analogies are really good at cracking me up.  I just recovered from the cleanup tip, and then the restaurants came.

  6. Yukkuri says:

    People that slog through impossible mystery code and fix it are my heroes. A salute to whoever this customer was o>

  7. Cesar says:

    @DWalker: if the code is that confused, the comments probably would be too. Wouldn't surprise me if the comments say something completely different than what the code does, and neither  is what the programmer actually wanted to do. (I've seen things like a comment explaining what the string returned by the function means, while the function actually returns a boolean!)

  8. Myria says:

    @Joshua: You don't need to replace a process's handle to itself: a process always has the -1 pseudo-handle that's always opened with full access.

    But why would you want to make a process deny access to everyone running as the same user?  This seems pointless, because if you're the process's owner, you can open the process with WRITE_DAC access using the owner's prerogative, then add access for yourself and/or remove the deny for yourself.

  9. Passed NULL, Gimme my $200 says:

       "the solution … is simple: Delete all the code … and just pass NULL"

    This applies universally.

  10. Joshua says:

    To keep running after logout w/o admin of course.

  11. Hildar says:

    I have to ask, what the hell does PROCESS_SET_SESSION­ID do, and why is that a problem?

  12. S says:

    "To keep running after logout w/o admin of course." And why that? Hopefully, the sesseion manager takes measure (by resetting this) to kill your rogue process on logout anyway.

    Use services or scheduled tasks for this.

  13. Harry Johnston says:

    @Hildar: presumably related to terminal services (aka remote desktop) sessions.  Although I didn't think it was actually possible to move a process between sessions or put it in any session other than the one given by its access token.  The permission might be obsolete, or it might be used internally by Windows.

  14. Darran Rowe says:

    @Harry Johnston:

    After doing a bit of digging, there is stuff defined in the WDK in regards to this. So if it is available from user mode, then it would be in the native API only. Even then, I wouldn't be surprised if you require the run as part of the operating system privilege to be able to do it.

    There is more you can do with processes than is exposed in the Win32 API. The two most obvious ones are the posix support, and the sandbox mode that modern processes use. They are obviously possible because MS has used them before, the posix support was in sfu/sua and IE for Windows 8 and 8.1 used the sandbox for enhanced protected mode (it doesn't seem to do it in 10).

  15. Max says:

    @DWalker: If the code is that old then it's likely been worked on by several people, going through incremental changes and updates and patches and bugfixes over the course of years. The comments don't just have to be made, they have to be maintained, just like the code is. And even if you meticulously document every fix, over time the comments will accumulate to the point where they become unreadable, and most likely no one's going to go back and analyze the code and rewrite/summarize all the comments into something that's still relevant and accurate until it's already become a problem. On projects that big and old, managing code readability isn't as simple as "comment everything".

Comments are closed.

Skip to main content