dotnet / dotnet/runtime

System.Security.AccessControl.*ObjectSecurity does not handle Mandatory Label ACEs (silently removes)

Open
#132,726 2 comments 1 reaction 0 assignees View on GitHub
area-System.Security
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

### Description

For background, in the Windows security model security descriptors have two Access Control Lists: the DACL and the SACL. The DACL is the traditional list used for access rules. Originally the SACL was rare and only used for auditing rules and so the .NET AccessControl APIs don't refer to it as a SACL in most cases and instead simply call it AuditRules. However in Vista, the SACL was used to add a new core security feature: Mandatory Access Control aka Integrity Levels. This is implemented by placing a new kind of ACE (Access Control Entry) into the SACL called the Mandatory Label ACE. This is a very core/important concept for the Windows security model ever since, as integrity level provides the primary separation between elevated and non-elevated user tokens, AppContainer isolation, etc.

The System.Security.AccessControl.*ObjectSecurity classes were never updated to support or handle the Mandatory Label ACEs. This is frankly... weird... given how long this has existed and how universally this has been a problem. However the API does allow you to call ObjectSecurity.SetSecurityDescriptorSddlForm and pass a custom SACL by string (you can actually pass the entire security descriptor or you can pass portions and tell it which portions to update). This would imply that despite the API not being able to handle Mandatory Labels explicitly that this "escape hatch" should be usable to set the integrity level of an object and that it should be stored internally as a CustomAce. However this fails silently in a particularly nasty way - it actually just doesn't add the extra ACE.

While this maybe shouldn't be classed as a CVE-class issue as-is, it could very easily lead to security issues in client programs written to do simple things like read the ACL of a file and write it back out. When doing so, the Mandatory Label is lost, which could result in a higher or lower level of intended access depending on the circumstances. It is confirmed to impact PowerShell commands which are all based on this framework, so Get-Acl silently drops the integrity level. The code definitely shouldn't lose ACEs silently that it doesn't know about since the framework is built to have CustomAce entries stored. The immediate fix would be to stop this silent dropping from happening.

Longer term, the best move is to add explicit API support for Mandatory Label ACEs. If I have some free time I might make such a suggestion.

```cs
using System.IO.Pipes;
using System.Security.AccessControl;
var pipeSecurity = new PipeSecurity();
// Set medium integrity level to allow non-admins to connect to the pipe.
pipeSecurity.SetSecurityDescriptorSddlForm("S:(ML;;NW;;;ME)", AccessControlSections.Audit);
```

The expectation is that since the ACE is unknown a CustomAce should be created (it is) and added to the internal SACL (it isn't). The root cause is in System.Security.AccessControl.CommonAcl.InspectAce, it has this code:

From [here](https://github.com/dotnet/runtime/blob/4194947dde033d2f552e2570b6cd81e45d19cc40/src/libraries/System.Security.AccessControl/src/System/Security/AccessControl/ACL.cs#L921)
```cs
if (isDacl)
{
// [...]
}
else
{
//
// On a SACL, any ACE that does not specify Success or Failure
// flags can be removed
//

if ((ace.AceFlags & AuditFlags) == 0)
{
return false;
}

//
// Qualified ACEs in a SACL must be audit ACEs
//

if (qualifiedAce != null)
{
if (qualifiedAce.AceQualifier != AceQualifier.SystemAudit)
{
return false;
}
}
}
```

### Reproduction Steps

I've summarized this issue into a test you can step through to see how it fails:
```cs
using System;
using System.Diagnostics;
using System.IO;
using System.IO.Pipes;
using System.Security.AccessControl;

[TestClass]
public class SecurityDescriptorTests{
[TestMethod]
public void RawSecurityDescriptorPreservesMandatoryLabelAce() {
string sampleSacl = "S:(ML;;NW;;;ME)";
var descriptor = new RawSecurityDescriptor(sampleSacl);

// These pass, as RawSecurityDescriptor doesn't immediately drop the ACE
Assert.IsNotNull(descriptor.SystemAcl);
Assert.AreNotEqual(0, descriptor.SystemAcl.Count);

var pipeSecurity = new PipeSecurity();
pipeSecurity.SetSecurityDescriptorSddlForm(sampleSacl, AccessControlSections.Audit);
var outcomingSacl = pipeSecurity.GetSecurityDescriptorSddlForm(AccessControlSections.Audit);

// This fails because SetSecurityDescriptorSddlForm calls CommonAcl.RemoveMeaninglessAcesAndFlags
Assert.AreEqual(sampleSacl, outcomingSacl);
}
}
```

### Expected behavior

The Mandatory Label ACE should be stored as a CustomAce (since there is no dedicated class for mandatory labels right now) rather than being rejected.

### Actual behavior

The Mandatory Label ACE is simply dropped from the SACL.

### Regression?

This appears to have been a problem in the .NET framework since Vista introduced Mandatory Label ACEs.

### Known Workarounds

The only option is to fallback to native Win32 APIs as all of the AccessControl namespace classes appear to round-trip through RawSecurityDescriptor which has this problem.

### Configuration

.NET 10
Windows 11 25H2

### Other information

_No response_

Contributor guide

Open the contributing guide

Research direction

Start in src/libraries/System.Security.AccessControl/src/System/Security/AccessControl/ACL.cs at CommonAcl.InspectAce and trace RemoveMeaninglessAcesAndFlags for SACL entries without audit flags. Use the RawSecurityDescriptorPreservesMandatoryLabelAce reproduction with S:(ML;;NW;;;ME). Done means the CustomAce remains in the SACL and the input SDDL round-trips unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
operating-systems, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.