From 3f146b8e52842825ba7b147940df40e641d36421 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Wed, 21 Oct 2026 12:38:34 +0000 Subject: [PATCH] fix(security2): let the readers handle a descriptor that has no item 3.1 made a FileSystemSecurity2 without an item reachable from a .NET caller (the conversions from FileSecurity and DirectorySecurity), but six sibling call sites still dereferenced the item without checking it first, so a caller who converted a FileSecurity or DirectorySecurity and then read its entries, owner, inheritance, or effective access met a NullReferenceException instead of a usable result. GetFileSystemAccessRules and GetFileSystemAuditRules (the FileSystemSecurity2 overload) now give each returned entry the same null FullName that the descriptor itself reports; asking for the source of an inherited entry (getInheritedFrom) needs the path of the item, so it now throws one clear InvalidOperationException instead, naming the reason, when the descriptor has none. FileSystemOwner and FileSystemInheritanceInfo give a null FullName (and Name) the same way; FileSystemOwner.ToString, which must never return null, gives an empty string instead, since there is no other representation to fall back to. The FileSystemAccessRule2(rule, item) constructor, which EffectiveAccess.GetEffectiveAccess uses to build its result, now accepts a null item the same way, so effective access can be calculated for such a descriptor and the result names no path. Every existing behavior for an item that is not null is unchanged; only a .NET caller that reads such a descriptor can notice the difference, and only for the better: a usable result or a clear error instead of an unrelated NullReferenceException. Public signatures are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- ...temAccessRule2.GetFileSystemAccessRules.cs | 13 +- .../FileSystemAccessRule2.cs | 4 +- ...ystemAuditRule2.GetFileSystemAuditRules.cs | 13 +- .../FileSystem/FileSystemInheritanceInfo.cs | 6 +- Security2/FileSystem/FileSystemOwner.cs | 8 +- Tests/ObjectApis.Tests.ps1 | 115 ++++++++++++++++++ 6 files changed, 152 insertions(+), 7 deletions(-) diff --git a/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.GetFileSystemAccessRules.cs b/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.GetFileSystemAccessRules.cs index 8946af0..6e4f65f 100644 --- a/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.GetFileSystemAccessRules.cs +++ b/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.GetFileSystemAccessRules.cs @@ -1,4 +1,5 @@ using Alphaleonis.Win32.Filesystem; +using System; using System.Collections.Generic; using System.Security.AccessControl; using System.Security.Principal; @@ -21,6 +22,15 @@ namespace Security2 if (getInheritedFrom) { + // The source of an inherited entry is found from the path of the item that the descriptor was read + // from; a descriptor that a caller built directly, such as the conversion from FileSecurity, has none. + if (sd.Item == null) + { + throw new InvalidOperationException( + "Cannot find the source of an inherited entry: this security descriptor has no item, so there " + + "is no path to search from. Pass getInheritedFrom as false, or read the entries from an item."); + } + inheritedFrom = Win32.GetInheritedFrom(sd.Item, sd.SecurityDescriptor, false); } @@ -40,7 +50,8 @@ namespace Security2 continue; } - var ace2 = new FileSystemAccessRule2(ace) { FullName = sd.Item.FullName, InheritanceEnabled = !sd.SecurityDescriptor.AreAccessRulesProtected }; + // sd.FullName, not sd.Item.FullName: null without an item instead of a NullReferenceException. + var ace2 = new FileSystemAccessRule2(ace) { FullName = sd.FullName, InheritanceEnabled = !sd.SecurityDescriptor.AreAccessRulesProtected }; if (getInheritedFrom && inheritedFrom.Count > 0) { // Windows names a folder with a trailing backslash; the text for an unknown parent has none. diff --git a/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.cs b/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.cs index 4b1a200..fdeb63e 100644 --- a/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.cs +++ b/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.cs @@ -31,7 +31,9 @@ namespace Security2 public FileSystemAccessRule2(FileSystemAccessRule fileSystemAccessRule, FileSystemInfo item) { this.fileSystemAccessRule = fileSystemAccessRule; - this.fullName = item.FullName; + // Null without an item, such as when EffectiveAccess.GetEffectiveAccess builds this from a + // FileSystemSecurity2 that has none, instead of a NullReferenceException. + this.fullName = item != null ? item.FullName : null; } public FileSystemAccessRule2(FileSystemAccessRule fileSystemAccessRule, string path) diff --git a/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.GetFileSystemAuditRules.cs b/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.GetFileSystemAuditRules.cs index fa18d6c..2d3d27c 100644 --- a/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.GetFileSystemAuditRules.cs +++ b/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.GetFileSystemAuditRules.cs @@ -1,4 +1,5 @@ using Alphaleonis.Win32.Filesystem; +using System; using System.Collections.Generic; using System.Security.AccessControl; using System.Security.Principal; @@ -21,6 +22,15 @@ namespace Security2 if (getInheritedFrom) { + // The source of an inherited entry is found from the path of the item that the descriptor was read + // from; a descriptor that a caller built directly, such as the conversion from FileSecurity, has none. + if (sd.Item == null) + { + throw new InvalidOperationException( + "Cannot find the source of an inherited entry: this security descriptor has no item, so there " + + "is no path to search from. Pass getInheritedFrom as false, or read the entries from an item."); + } + inheritedFrom = Win32.GetInheritedFrom(sd.Item, sd.SecurityDescriptor, true); } @@ -40,7 +50,8 @@ namespace Security2 continue; } - var ace2 = new FileSystemAuditRule2(ace) { FullName = sd.Item.FullName, InheritanceEnabled = !sd.SecurityDescriptor.AreAuditRulesProtected }; + // sd.FullName, not sd.Item.FullName: null without an item instead of a NullReferenceException. + var ace2 = new FileSystemAuditRule2(ace) { FullName = sd.FullName, InheritanceEnabled = !sd.SecurityDescriptor.AreAuditRulesProtected }; if (getInheritedFrom && inheritedFrom.Count > 0) { // Windows names a folder with a trailing backslash; the text for an unknown parent has none. diff --git a/Security2/FileSystem/FileSystemInheritanceInfo.cs b/Security2/FileSystem/FileSystemInheritanceInfo.cs index 5fc9f9c..3a37496 100644 --- a/Security2/FileSystem/FileSystemInheritanceInfo.cs +++ b/Security2/FileSystem/FileSystemInheritanceInfo.cs @@ -35,9 +35,11 @@ namespace Security2 set { auditInheritanceEnabled = value; } } - public string FullName { get { return Item.FullName; } } + // Null without an item, such as for a descriptor that a conversion from FileSecurity built directly, instead + // of a NullReferenceException. + public string FullName { get { return item != null ? item.FullName : null; } } - public string Name { get { return Path.GetFileName(item.FullName); } } + public string Name { get { return item != null ? Path.GetFileName(item.FullName) : null; } } private FileSystemInheritanceInfo(FileSystemInfo item, bool? accessInheritanceEnabled, bool? auditInheritanceEnabled) { diff --git a/Security2/FileSystem/FileSystemOwner.cs b/Security2/FileSystem/FileSystemOwner.cs index 103482b..a2dfea2 100644 --- a/Security2/FileSystem/FileSystemOwner.cs +++ b/Security2/FileSystem/FileSystemOwner.cs @@ -27,7 +27,9 @@ namespace Security2 public string FullName { - get { return item.FullName; } + // Null without an item, such as for a descriptor that a conversion from FileSecurity built directly, + // instead of a NullReferenceException. + get { return item != null ? item.FullName : null; } } private FileSystemOwner(FileSystemInfo item, IdentityReference2 owner) @@ -62,7 +64,9 @@ namespace Security2 public override string ToString() { - return item.FullName; + // Empty, not null, without an item: ToString must never return null, and there is no other + // representation of this object to fall back to. + return item != null ? item.FullName : string.Empty; } } } diff --git a/Tests/ObjectApis.Tests.ps1 b/Tests/ObjectApis.Tests.ps1 index 76f65ce..9f13b07 100644 --- a/Tests/ObjectApis.Tests.ps1 +++ b/Tests/ObjectApis.Tests.ps1 @@ -830,6 +830,18 @@ Describe 'The FileSystemSecurity2 conversions from FileSecurity and DirectorySec $raw.AddAccessRule($rule) $raw } + + # PowerShell silently turns an exception that a property getter throws into $null, without even recording a + # non-terminating error, so a plain "$object.Property" cannot tell a getter that throws from one that + # legitimately returns null. The getter is invoked through reflection instead, so that it throws normally. + function Invoke-PropertyGetter { + [Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseShouldProcessForStateChangingFunctions', '', Justification = 'Test helper that only reads a property through reflection.' + )] + param ([object] $InputObject, [string] $PropertyName) + + $InputObject.GetType().GetProperty($PropertyName).GetGetMethod().Invoke($InputObject, $null) + } } It 'Should wrap the descriptor that was passed, report IsFile as , and round-trip the same instance' -ForEach @( @@ -901,6 +913,109 @@ Describe 'The FileSystemSecurity2 conversions from FileSecurity and DirectorySec @((Get-Acl -LiteralPath $path).GetAccessRules($true, $false, [System.Security.Principal.SecurityIdentifier]) | Where-Object -FilterScript { $_.IdentityReference.Value -eq 'S-1-1-0' }) | Should -HaveCount 1 } + + # 3.1 makes a FileSystemSecurity2 without an item reachable from a .NET caller; the readers below must not meet a + # NullReferenceException from it. Built in memory, so no privilege is needed to add the audit rule. + It 'Should return the entries of a without an item, with a null FullName' -ForEach @( + @{ Kind = 'FileSecurity'; RuleType = 'access' } + @{ Kind = 'DirectorySecurity'; RuleType = 'access' } + @{ Kind = 'FileSecurity'; RuleType = 'audit' } + @{ Kind = 'DirectorySecurity'; RuleType = 'audit' } + ) { + if ($RuleType -eq 'access') { + $sd = [Security2.FileSystemSecurity2] (New-RawFileSystemSecurity -Kind $Kind) + $found = @([Security2.FileSystemAccessRule2]::GetFileSystemAccessRules($sd, $true, $true, $false)) + } + else { + $raw = New-RawFileSystemSecurity -Kind $Kind + $raw.AddAuditRule((New-Object -TypeName 'System.Security.AccessControl.FileSystemAuditRule' -ArgumentList ( + $sid, [System.Security.AccessControl.FileSystemRights]::ReadData, + [System.Security.AccessControl.AuditFlags]::Success + ))) + $sd = [Security2.FileSystemSecurity2] $raw + $found = @([Security2.FileSystemAuditRule2]::GetFileSystemAuditRules($sd, $true, $true, $false)) + } + + $found | Should -HaveCount 1 + $found[0].FullName | Should -Be $null + $found[0].Account.Sid | Should -BeExactly 'S-1-1-0' + } + + It 'Should throw an InvalidOperationException, not a NullReferenceException, for the source of an inherited entry of a descriptor without an item' -ForEach @( + @{ RuleType = 'access' } + @{ RuleType = 'audit' } + ) { + if ($RuleType -eq 'access') { + $sd = [Security2.FileSystemSecurity2] (New-RawFileSystemSecurity -Kind 'FileSecurity') + $failure = { [Security2.FileSystemAccessRule2]::GetFileSystemAccessRules($sd, $true, $true, $true) } | + Should -Throw -PassThru + } + else { + $raw = New-RawFileSystemSecurity -Kind 'FileSecurity' + $raw.AddAuditRule((New-Object -TypeName 'System.Security.AccessControl.FileSystemAuditRule' -ArgumentList ( + $sid, [System.Security.AccessControl.FileSystemRights]::ReadData, + [System.Security.AccessControl.AuditFlags]::Success + ))) + $sd = [Security2.FileSystemSecurity2] $raw + $failure = { [Security2.FileSystemAuditRule2]::GetFileSystemAuditRules($sd, $true, $true, $true) } | + Should -Throw -PassThru + } + + $failure.Exception.GetBaseException() | Should -BeOfType [System.InvalidOperationException] + $failure.Exception.GetBaseException().Message | Should -BeLike '*item*' + } + + It 'Should return an owner object with a null FullName and an empty ToString for a without an item' -ForEach @( + @{ Kind = 'FileSecurity' } + @{ Kind = 'DirectorySecurity' } + ) { + $raw = New-RawFileSystemSecurity -Kind $Kind + $raw.SetOwner($sid) + $sd = [Security2.FileSystemSecurity2] $raw + + $owner = [Security2.FileSystemOwner]::GetOwner($sd) + + $owner.Owner.Sid | Should -BeExactly 'S-1-1-0' + { Invoke-PropertyGetter -InputObject $owner -PropertyName 'FullName' } | Should -Not -Throw + (Invoke-PropertyGetter -InputObject $owner -PropertyName 'FullName') | Should -Be $null + $owner.ToString() | Should -BeExactly '' + } + + It 'Should return inheritance info with a null FullName and Name for a without an item' -ForEach @( + @{ Kind = 'FileSecurity' } + @{ Kind = 'DirectorySecurity' } + ) { + $sd = [Security2.FileSystemSecurity2] (New-RawFileSystemSecurity -Kind $Kind) + + $info = [Security2.FileSystemInheritanceInfo]::GetFileSystemInheritanceInfo($sd) + + { Invoke-PropertyGetter -InputObject $info -PropertyName 'FullName' } | Should -Not -Throw + { Invoke-PropertyGetter -InputObject $info -PropertyName 'Name' } | Should -Not -Throw + (Invoke-PropertyGetter -InputObject $info -PropertyName 'FullName') | Should -Be $null + (Invoke-PropertyGetter -InputObject $info -PropertyName 'Name') | Should -Be $null + $info.AccessInheritanceEnabled | Should -BeTrue + } + + # The Authz access check needs a descriptor with an owner and a group, like any real file has; the minimal + # descriptor of the other tests has neither, and the call fails with "The parameter is incorrect" for either. + It 'Should return the effective access of a without an item, with a null FullName on the result' -ForEach @( + @{ Kind = 'FileSecurity' } + @{ Kind = 'DirectorySecurity' } + ) { + $raw = New-RawFileSystemSecurity -Kind $Kind + $raw.SetOwner($sid) + $raw.SetGroup($sid) + $sd = [Security2.FileSystemSecurity2] $raw + + $result = [Security2.EffectiveAccess]::GetEffectiveAccess($sd, $identity, $null) + + if ($result.OperationFailed) { + Set-ItResult -Skipped -Because "the Authz call could not run in this configuration: $($result.AuthzException.Message)" + return + } + + $result.Ace.FullName | Should -Be $null + } } # Before 5.0.0, the account list of both methods was built but its filter result was never assigned anywhere, so a