Set-Acl: Do not fail on untranslatable SID (PowerShell#21096)

xtqqczze · Jul 16, 2024 · c107c9c · c107c9c
1 parent 2e93c18
commit c107c9c
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 4 deletions.
diff --git a/src/System.Management.Automation/namespaces/FileSystemSecurity.cs b/src/System.Management.Automation/namespaces/FileSystemSecurity.cs
@@ -168,27 +168,29 @@ public sealed partial class FileSystemProvider : NavigationCmdletProvider, ICont
                 {
                     // Get the security descriptor of the destination path
                     ObjectSecurity existingDescriptor = new FileInfo(path).GetAccessControl();
-                    Type ntAccountType = typeof(System.Security.Principal.NTAccount);
+                    // Use SecurityIdentifier to avoid having the below comparison steps
+                    // fail when dealing with an untranslatable SID in the SD
+                    Type identityType = typeof(System.Security.Principal.SecurityIdentifier);
 
                     AccessControlSections sections = AccessControlSections.All;
 
                     // If they didn't modify any audit information, don't try to set
                     // the audit section.
-                    int auditRuleCount = sd.GetAuditRules(true, true, ntAccountType).Count;
+                    int auditRuleCount = sd.GetAuditRules(true, true, identityType).Count;
                     if ((auditRuleCount == 0) &&
                         (sd.AreAuditRulesProtected == existingDescriptor.AreAccessRulesProtected))
                     {
                         sections &= ~AccessControlSections.Audit;
                     }
 
                     // If they didn't modify the owner, don't try to set that section.
-                    if (sd.GetOwner(ntAccountType) == existingDescriptor.GetOwner(ntAccountType))
+                    if (sd.GetOwner(identityType) == existingDescriptor.GetOwner(identityType))
                     {
                         sections &= ~AccessControlSections.Owner;
                     }
 
                     // If they didn't modify the group, don't try to set that section.
-                    if (sd.GetGroup(ntAccountType) == existingDescriptor.GetGroup(ntAccountType))
+                    if (sd.GetGroup(identityType) == existingDescriptor.GetGroup(identityType))
                     {
                         sections &= ~AccessControlSections.Group;
                     }

diff --git a/test/powershell/Modules/Microsoft.PowerShell.Security/AclCmdlets.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Security/AclCmdlets.Tests.ps1
@@ -77,6 +77,31 @@ Describe "Acl cmdlets are available and operate properly" -Tag CI {
             $newrule | Should -Not -BeNullOrEmpty
         }
 
+        It "Can edit SD that contains an orphaned SID" {
+            $badSid = [System.Security.Principal.SecurityIdentifier]::new("S-1-5-1234-5678")
+            $currentUserSid = [System.Security.Principal.WindowsIdentity]::GetCurrent().User
+
+            $testFilePath = "TestDrive:\pwsh-acl-test.txt"
+            $testFile = New-Item -Path $testFilePath -ItemType File -Value 'foo' -Force
+
+            # We should be able to set an SD entry to an untranslatable SID
+            $fileSecurity = $testFilePath | Get-Acl
+            $fileSecurity.SetGroup($badSid)
+            Set-Acl -Path $testFile -AclObject $fileSecurity
+
+            # We should be able to get the SD with an untranslatable SID
+            $setSD = Get-Acl -Path $testFile
+            $setSD.GetGroup([System.Security.Principal.SecurityIdentifier]) | Should -Be $badSid
+
+            # We should be able to set it back to a known SID
+            $setSD.SetGroup($currentUserSid)
+            Set-Acl -Path $testFile -AclObject $setSD
+
+            $actual = Get-Acl -Path $testFile
+            $actualGroup = $actual.GetGroup([System.Security.Principal.SecurityIdentifier])
+            $actualGroup | Should -Be $currentUserSid
+        }
+
         AfterAll {
             $PSDefaultParameterValues.Remove("It:Skip")
         }