System.Security.AccessControl.ObjectSecurity.GetSecurityDescriptorBinaryForm() is not thread safe

[vitaly-cyberhaven]

Describe the bug

GetSecurityDescriptorBinaryForm grabs a read lock on the object, suggesting it is thread safe. However, _securityDescriptor.GetBinaryForm may in some cases canonicalize the security descriptor, writing to it. Concurrent calls to GetSecurityDescriptorBinaryForm can therefore corrupt the object. The method should grab a write lock instead. Other methods may be affected as well if they eventually canonicalize the descriptor.

To Reproduce

Run this test case:

[TestMethod]
public void TestPipeSecurity()
{
    using (var start = new SemaphoreSlim(0)) {
        while (true) {
            var localSystem = new SecurityIdentifier(WellKnownSidType.LocalSystemSid, null);
            var localService = new SecurityIdentifier(WellKnownSidType.LocalServiceSid, null);
            var users = new SecurityIdentifier(WellKnownSidType.AuthenticatedUserSid, null);

            var sec = new PipeSecurity();
            sec.AddAccessRule(new PipeAccessRule(localSystem, PipeAccessRights.CreateNewInstance, AccessControlType.Allow));
            sec.AddAccessRule(new PipeAccessRule(localService, PipeAccessRights.CreateNewInstance, AccessControlType.Allow));
            sec.AddAccessRule(new PipeAccessRule(users, PipeAccessRights.CreateNewInstance, AccessControlType.Allow));

            byte[] b1 = null, b2 = null;

            // If this is called outside of the threads, the bug does not occur.
            // sec.GetSecurityDescriptorBinaryForm();

            ThreadStart thrFcn1 = () => {
                start.Wait();
                b1 = sec.GetSecurityDescriptorBinaryForm();
            };

            ThreadStart thrFcn2 = () => {
                start.Wait();
                b2 = sec.GetSecurityDescriptorBinaryForm();
            };

            var thr1 = new Thread(thrFcn1);
            var thr2 = new Thread(thrFcn2);
            thr1.Start();
            thr2.Start();
            start.Release(2);
            thr1.Join();
            thr2.Join();

            Assert.IsTrue(Enumerable.SequenceEqual(b1, b2));
        }
    }
}

Further technical details

• Include the output of dotnet --info

.NET SDK:
 Version:           8.0.403
 Commit:            c64aa40a71
 Workload version:  8.0.400-manifests.e99c892e
 MSBuild version:   17.11.9+a69bbaaf5

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.26100
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\8.0.403\

.NET workloads installed:
Configured to use loose manifests when installing new manifests.
There are no installed workloads to display.

Host:
  Version:      8.0.10
  Architecture: x64
  Commit:       81cabf2857

.NET SDKs installed:
  8.0.403 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.10 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.32 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.35 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.10 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.32 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.35 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.10 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download

• The IDE (VS / VS Code/ VS4Mac) you're running on, and its version

Microsoft Visual Studio Professional 2022 (64-bit) - Current
Version 17.11.5

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.