Browse Source

fix: compare output objects and report InheritedFrom correctly

Equals of FileSystemAccessRule2, FileSystemAuditRule2, and
FileSystemSecurity2 cast its argument to the .NET type, which throws for
the wrapper types themselves: -eq and -contains, and in PowerShell 7 also
Select-Object -Unique and Compare-Object, stopped with an
InvalidCastException. GetHashCode of FileSystemSecurity2 read a field
that is never set and threw a NullReferenceException. Two objects are now
equal when they hold the same entry or descriptor, like the .NET types.

InheritedFrom: Win32.GetInheritedFrom returned the sources of the SACL
whenever the descriptor had one, also for the access entries, and the
callers gave the filtered entries of -ExcludeExplicit the sources of the
first entries of the ACL. Get-NTFSAccess -SecurityDescriptor stopped with
an ArgumentOutOfRangeException for a descriptor with audit entries, as
Get-NTFSSecurityDescriptor reads them in an elevated session. The method
now takes the ACL of the entries, and the callers map the sources before
they filter.

The coverage report of rc6 pointed at both; each test fails without its
fix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>
ai/release-5.0.0-rc6
Raimund Andree 4 days ago
parent
commit
1b9edbbd6a
  1. 11
      CHANGELOG.md
  2. 2
      Docs/Cmdlets/Get-NTFSAccess.md
  3. 2
      Docs/Cmdlets/Get-NTFSAudit.md
  4. 2
      NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml
  5. 18
      Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.GetFileSystemAccessRules.cs
  6. 7
      Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.cs
  7. 20
      Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.GetFileSystemAuditRules.cs
  8. 7
      Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.cs
  9. 10
      Security2/FileSystem/FileSystemSecurity2.cs
  10. 41
      Security2/Win32/Lib.cs
  11. 71
      Tests/Access.Tests.ps1
  12. 34
      Tests/Audit.Tests.ps1
  13. 19
      Tests/SecurityDescriptor.Tests.ps1

11
CHANGELOG.md

@ -316,5 +316,16 @@ The format is based on
`.\`, such as `.gitignore`: the cmdlets dropped its first two characters
and read, changed, or removed the item with the shorter name, such as
`itignore`, when one existed
- Fix comparing the objects of the access, audit, and security descriptor
cmdlets: `-eq` and `-contains`, and in PowerShell 7 also
`Select-Object -Unique` and `Compare-Object`, stopped with an
`InvalidCastException`, and a security descriptor as the key of a
hashtable with a `NullReferenceException`. Two objects are now equal when
they hold the same entry or descriptor, as in .NET
- Fix `InheritedFrom` of `Get-NTFSAccess` and `Get-NTFSAudit`: with
`-ExcludeExplicit`, each inherited entry showed the source of another
entry, and `Get-NTFSAccess -SecurityDescriptor` stopped with an
`ArgumentOutOfRangeException` for a descriptor with audit entries, such
as one that `Get-NTFSSecurityDescriptor` reads in an elevated session
[Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD

2
Docs/Cmdlets/Get-NTFSAccess.md

@ -186,6 +186,8 @@ Entries whose account cannot be translated into a name are returned with their S
Before 5.0.0, after a path whose ACL could not be read, the cmdlet returned the entries of the previous item again.
Before 5.0.0-rc6, with `-ExcludeExplicit`, each inherited entry showed the `InheritedFrom` path of another entry, and for a security descriptor with audit entries, such as one that `Get-NTFSSecurityDescriptor` reads in an elevated session, the cmdlet stopped with an `ArgumentOutOfRangeException`.
For the root of a drive, such as `C:\`, or of a volume, such as `\\?\Volume{GUID}\`, the cmdlets that read and change security use the root folder of the volume, like Explorer, `icacls`, and `Get-Acl`. Before 5.0.0, they read and changed the security descriptor of the drive itself, a device object with other entries.
## RELATED LINKS

2
Docs/Cmdlets/Get-NTFSAudit.md

@ -185,6 +185,8 @@ If reading the audit entries is denied, the cmdlet writes a `ReadSecurityError`
Before 5.0.0, the cmdlet returned no entries and no error without the Security privilege, and after a path whose security descriptor could not be read, it returned the entries of the previous item again. The `InheritanceEnabled` property of the entries also reported whether the access entries were inherited instead of the audit entries.
Before 5.0.0-rc6, with `-ExcludeExplicit`, each inherited entry showed the `InheritedFrom` path of another entry.
## RELATED LINKS
[Add-NTFSAudit](Add-NTFSAudit.md)

2
NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml

@ -4588,6 +4588,7 @@ PS C:\&gt; Disable-Privileges</dev:code>
<maml:para>If the ACL of an item cannot be read because access is denied, the cmdlet tries once more after making the current account the owner of the item, and restores the previous owner afterwards. Reading the owner, which that fallback needs first, requires the same Read Permissions right as reading the ACL, so the cmdlet then writes a non-terminating `ReadSecurityError` and continues with the next item.</maml:para>
<maml:para>Entries whose account cannot be translated into a name are returned with their SID. Use `Get-NTFSOrphanedAccess` to list only those entries.</maml:para>
<maml:para>Before 5.0.0, after a path whose ACL could not be read, the cmdlet returned the entries of the previous item again.</maml:para>
<maml:para>Before 5.0.0-rc6, with `-ExcludeExplicit`, each inherited entry showed the `InheritedFrom` path of another entry, and for a security descriptor with audit entries, such as one that `Get-NTFSSecurityDescriptor` reads in an elevated session, the cmdlet stopped with an `ArgumentOutOfRangeException`.</maml:para>
<maml:para>For the root of a drive, such as `C:`, or of a volume, such as `\?\Volume{GUID}`, the cmdlets that read and change security use the root folder of the volume, like Explorer, `icacls`, and `Get-Acl`. Before 5.0.0, they read and changed the security descriptor of the drive itself, a device object with other entries.</maml:para>
</maml:alert>
</maml:alertSet>
@ -4871,6 +4872,7 @@ PS C:\&gt; Disable-Privileges</dev:code>
<maml:para>Reading the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes the non-terminating error `ReadSecurityError` for each item, which reports "A required privilege is not held by the client". `Get-NTFSSecurityDescriptor` reads a security descriptor without its SACL when the privilege is missing; for such a descriptor, the cmdlet writes a `ReadSecurityError` as well.</maml:para>
<maml:para>If reading the audit entries is denied, the cmdlet writes a `ReadSecurityError` with the category `PermissionDenied`. It doesn't take ownership of the item, because ownership grants no access to the SACL.</maml:para>
<maml:para>Before 5.0.0, the cmdlet returned no entries and no error without the Security privilege, and after a path whose security descriptor could not be read, it returned the entries of the previous item again. The `InheritanceEnabled` property of the entries also reported whether the access entries were inherited instead of the audit entries.</maml:para>
<maml:para>Before 5.0.0-rc6, with `-ExcludeExplicit`, each inherited entry showed the `InheritedFrom` path of another entry.</maml:para>
</maml:alert>
</maml:alertSet>
<command:examples>

18
Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.GetFileSystemAccessRules.cs

@ -21,21 +21,29 @@ namespace Security2
if (getInheritedFrom)
{
inheritedFrom = Win32.GetInheritedFrom(sd.Item, sd.SecurityDescriptor);
inheritedFrom = Win32.GetInheritedFrom(sd.Item, sd.SecurityDescriptor, false);
}
// All entries, so that each entry gets the source at its index in the DACL; before 5.0.0-rc6, the entries of
// -ExcludeExplicit got the sources of other entries. The filter follows.
var aceCounter = 0;
var acl = !sd.IsFile ?
((DirectorySecurity)sd.SecurityDescriptor).GetAccessRules(includeExplicit, includeInherited, typeof(SecurityIdentifier)) :
((FileSecurity)sd.SecurityDescriptor).GetAccessRules(includeExplicit, includeInherited, typeof(SecurityIdentifier));
((DirectorySecurity)sd.SecurityDescriptor).GetAccessRules(true, true, typeof(SecurityIdentifier)) :
((FileSecurity)sd.SecurityDescriptor).GetAccessRules(true, true, typeof(SecurityIdentifier));
foreach (FileSystemAccessRule ace in acl)
{
var source = getInheritedFrom && aceCounter < inheritedFrom.Count ? inheritedFrom[aceCounter] : null;
aceCounter++;
if (ace.IsInherited ? !includeInherited : !includeExplicit)
{
continue;
}
var ace2 = new FileSystemAccessRule2(ace) { FullName = sd.Item.FullName, InheritanceEnabled = !sd.SecurityDescriptor.AreAccessRulesProtected };
if (getInheritedFrom && inheritedFrom.Count > 0)
{
ace2.inheritedFrom = string.IsNullOrEmpty(inheritedFrom[aceCounter]) ? "" : inheritedFrom[aceCounter].Substring(0, inheritedFrom[aceCounter].Length - 1);
aceCounter++;
ace2.inheritedFrom = string.IsNullOrEmpty(source) ? "" : source.Substring(0, source.Length - 1);
}
aceList.Add(ace2);

7
Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.cs

@ -48,10 +48,13 @@ namespace Security2
return new FileSystemAccessRule2(ace);
}
//REQUIRED BECAUSE OF CONVERSION OPERATORS
// Like the entries of .NET, two objects are equal when they wrap the same entry. A cast instead of "as"
// threw an InvalidCastException for any other type, also for -eq in PowerShell.
public override bool Equals(object obj)
{
return fileSystemAccessRule == (FileSystemAccessRule)obj;
var other = obj as FileSystemAccessRule2;
var rule = other != null ? other.fileSystemAccessRule : obj as FileSystemAccessRule;
return rule != null && ReferenceEquals(fileSystemAccessRule, rule);
}
public override int GetHashCode()
{

20
Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.GetFileSystemAuditRules.cs

@ -21,21 +21,29 @@ namespace Security2
if (getInheritedFrom)
{
inheritedFrom = Win32.GetInheritedFrom(sd.Item, sd.SecurityDescriptor);
inheritedFrom = Win32.GetInheritedFrom(sd.Item, sd.SecurityDescriptor, true);
}
// All entries, so that each entry gets the source at its index in the SACL; before 5.0.0-rc6, the entries of
// -ExcludeExplicit got the sources of other entries. The filter follows.
var aceCounter = 0;
var acl = !sd.IsFile ?
((DirectorySecurity)sd.SecurityDescriptor).GetAuditRules(includeExplicit, includeInherited, typeof(SecurityIdentifier)) :
((FileSecurity)sd.SecurityDescriptor).GetAuditRules(includeExplicit, includeInherited, typeof(SecurityIdentifier));
((DirectorySecurity)sd.SecurityDescriptor).GetAuditRules(true, true, typeof(SecurityIdentifier)) :
((FileSecurity)sd.SecurityDescriptor).GetAuditRules(true, true, typeof(SecurityIdentifier));
foreach (FileSystemAuditRule ace in acl)
{
var source = getInheritedFrom && aceCounter < inheritedFrom.Count ? inheritedFrom[aceCounter] : null;
aceCounter++;
if (ace.IsInherited ? !includeInherited : !includeExplicit)
{
continue;
}
var ace2 = new FileSystemAuditRule2(ace) { FullName = sd.Item.FullName, InheritanceEnabled = !sd.SecurityDescriptor.AreAuditRulesProtected };
if (getInheritedFrom)
if (getInheritedFrom && inheritedFrom.Count > 0)
{
ace2.inheritedFrom = string.IsNullOrEmpty(inheritedFrom[aceCounter]) ? "" : inheritedFrom[aceCounter].Substring(0, inheritedFrom[aceCounter].Length - 1);
aceCounter++;
ace2.inheritedFrom = string.IsNullOrEmpty(source) ? "" : source.Substring(0, source.Length - 1);
}
aceList.Add(ace2);

7
Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.cs

@ -48,10 +48,13 @@ namespace Security2
{
return new FileSystemAuditRule2(ace);
}
//REQUIRED BECAUSE OF CONVERSION OPERATORS
// Like the entries of .NET, two objects are equal when they wrap the same entry. A cast instead of "as"
// threw an InvalidCastException for any other type, also for -eq in PowerShell.
public override bool Equals(object obj)
{
return this.fileSystemAuditRule == (FileSystemAuditRule)obj;
var other = obj as FileSystemAuditRule2;
var rule = other != null ? other.fileSystemAuditRule : obj as FileSystemAuditRule;
return rule != null && ReferenceEquals(fileSystemAuditRule, rule);
}
public override int GetHashCode()
{

10
Security2/FileSystem/FileSystemSecurity2.cs

@ -254,14 +254,18 @@ namespace Security2
return new FileSystemSecurity2(new DirectoryInfo(""));
}
//REQUIRED BECAUSE OF CONVERSION OPERATORS
// Like the descriptors of .NET, two objects are equal when they wrap the same descriptor. The descriptor is in
// sd; fileSecurityDescriptor is never set, so its hash code threw a NullReferenceException, and the cast an
// InvalidCastException for any other type.
public override bool Equals(object obj)
{
return this.fileSecurityDescriptor == (FileSecurity)obj;
var other = obj as FileSystemSecurity2;
var descriptor = other != null ? other.sd : obj as FileSystemSecurity;
return descriptor != null && ReferenceEquals(sd, descriptor);
}
public override int GetHashCode()
{
return fileSecurityDescriptor.GetHashCode();
return sd != null ? sd.GetHashCode() : 0;
}
#endregion

41
Security2/Win32/Lib.cs

@ -19,44 +19,21 @@ namespace Security2
IntPtr pErrorSecObj = IntPtr.Zero;
#region GetInheritedFrom
public static List<string> GetInheritedFrom(FileSystemInfo item, ObjectSecurity sd)
// Returns the source of each entry of the DACL, or of the SACL for audit entries, in the order of the ACL. Before
// 5.0.0-rc6, a descriptor with a SACL returned the sources of the audit entries also for the access entries.
public static List<string> GetInheritedFrom(FileSystemInfo item, ObjectSecurity sd, bool auditEntries)
{
var inheritedFrom = new List<string>();
var sdBytes = sd.GetSecurityDescriptorBinaryForm();
byte[] aclBytes = null;
var rawSd = new RawSecurityDescriptor(sdBytes, 0);
var acl = auditEntries ? rawSd.SystemAcl : rawSd.DiscretionaryAcl;
var aceCount = 0;
if (rawSd.SystemAcl != null)
{
aceCount = rawSd.SystemAcl.Count;
aclBytes = new byte[rawSd.SystemAcl.BinaryLength];
rawSd.SystemAcl.GetBinaryForm(aclBytes, 0);
try
{
inheritedFrom = GetInheritedFrom(item.FullName,
aclBytes,
aceCount,
item is DirectoryInfo ? true : false,
SECURITY_INFORMATION.SACL_SECURITY_INFORMATION);
}
catch
{
inheritedFrom = new List<string>();
for (int i = 0; i < aceCount; i++)
{
inheritedFrom.Add("unknown parent");
}
}
}
else if (rawSd.DiscretionaryAcl != null)
if (acl != null)
{
aceCount = rawSd.DiscretionaryAcl.Count;
aclBytes = new byte[rawSd.DiscretionaryAcl.BinaryLength];
rawSd.DiscretionaryAcl.GetBinaryForm(aclBytes, 0);
var aceCount = acl.Count;
var aclBytes = new byte[acl.BinaryLength];
acl.GetBinaryForm(aclBytes, 0);
try
{
@ -64,7 +41,7 @@ namespace Security2
aclBytes,
aceCount,
item is DirectoryInfo ? true : false,
SECURITY_INFORMATION.DACL_SECURITY_INFORMATION);
auditEntries ? SECURITY_INFORMATION.SACL_SECURITY_INFORMATION : SECURITY_INFORMATION.DACL_SECURITY_INFORMATION);
}
catch
{

71
Tests/Access.Tests.ps1

@ -784,3 +784,74 @@ Describe 'Clear-NTFSAccess' {
}
}
}
# Before 5.0.0-rc6, comparing an entry with anything threw an InvalidCastException, so -eq, -contains, and in PowerShell 7
# also Select-Object -Unique and Compare-Object failed for the output of Get-NTFSAccess. Like the entries of .NET, two
# objects are equal when they wrap the same entry.
Describe 'Comparing access entries' {
BeforeAll {
$compareFile = New-TestSandboxItem -Sandbox $sandbox -Name 'Compare'
Add-NTFSAccess -Path $compareFile -Account 'S-1-1-0' -AccessRights ReadData
$entries = @(Get-NTFSAccess -Path $compareFile)
}
It 'Should find an entry equal to itself and not to another entry' {
$entries.Count | Should -BeGreaterThan 1
$entries[0] -eq $entries[0] | Should -BeTrue
$entries[0] -eq $entries[1] | Should -BeFalse
$entries -contains $entries[1] | Should -BeTrue
$entries[0].Equals('S-1-1-0') | Should -BeFalse
$entries[0].Equals($null) | Should -BeFalse
}
It 'Should work with Select-Object -Unique and Compare-Object' {
@(@($entries) + $entries[0] | Select-Object -Unique) | Should -HaveCount $entries.Count
Compare-Object -ReferenceObject $entries -DifferenceObject $entries | Should -BeNullOrEmpty
}
}
# Before 5.0.0-rc6, -ExcludeExplicit gave each inherited entry the source of another entry, and Get-NTFSAccess stopped
# with an ArgumentOutOfRangeException for a security descriptor with audit entries, because it took the sources of the
# audit entries for the access entries.
Describe 'InheritedFrom of access entries' {
BeforeAll {
$inheritedFromFile = New-TestSandboxItem -Sandbox $sandbox -Name 'InheritedFrom'
Add-NTFSAccess -Path $inheritedFromFile -Account 'S-1-1-0' -AccessRights ReadData
$expectedSource = @{}
foreach ($entry in Get-NTFSAccess -Path $inheritedFromFile) {
if ($entry.IsInherited) {
$expectedSource["$($entry.Account.Sid)"] = $entry.InheritedFrom
}
}
}
It 'Should name the folder that each inherited entry comes from' {
$expectedSource.Count | Should -BeGreaterThan 0
$expectedSource.Values | ForEach-Object -Process { $_ | Should -Not -BeNullOrEmpty }
}
It 'Should name the same folders with -ExcludeExplicit' {
$result = @(Get-NTFSAccess -Path $inheritedFromFile -ExcludeExplicit)
$result | Should -HaveCount $expectedSource.Count
foreach ($entry in $result) {
$entry.InheritedFrom | Should -Be $expectedSource["$($entry.Account.Sid)"]
}
}
It 'Should read a security descriptor with audit entries and name the same folders' -Skip:(-not $holdsSecurityPrivilege) {
$file = New-TestSandboxItem -Sandbox $sandbox -Name 'InheritedFromAudit'
Add-NTFSAccess -Path $file -Account 'S-1-1-0' -AccessRights ReadData
Add-NTFSAudit -Path $file -Account 'S-1-1-0' -AccessRights ReadData -InheritanceFlags None -PropagationFlags None
$sd = Get-NTFSSecurityDescriptor -Path $file
@($sd.SecurityDescriptor.GetAuditRules($true, $true, $sidType)) | Should -Not -BeNullOrEmpty
$result = @(Get-NTFSAccess -SecurityDescriptor $sd -ErrorAction Stop)
$result | Should -HaveCount @(Get-NTFSAccess -Path $file).Count
foreach ($entry in @($result | Where-Object -FilterScript { $_.IsInherited })) {
$entry.InheritedFrom | Should -Be $expectedSource["$($entry.Account.Sid)"]
}
}
}

34
Tests/Audit.Tests.ps1

@ -462,3 +462,37 @@ Describe 'Clear-NTFSAudit' {
}
}
}
# Before 5.0.0-rc6, comparing an audit entry with anything threw an InvalidCastException.
Describe 'Comparing audit entries' {
It 'Should find an entry equal to itself and not to a string' -Skip:(-not $canReadAudit) {
$file = New-TestSandboxItem -Sandbox $sandbox -Name 'Compare'
Add-NTFSAudit -Path $file -Account 'S-1-1-0' -AccessRights ReadData -InheritanceFlags None -PropagationFlags None
$entries = @(Get-NTFSAudit -Path $file -ExcludeInherited)
$entries | Should -HaveCount 1
$entries[0] -eq $entries[0] | Should -BeTrue
$entries -contains $entries[0] | Should -BeTrue
$entries[0].Equals('S-1-1-0') | Should -BeFalse
}
}
# Before 5.0.0-rc6, -ExcludeExplicit gave each inherited audit entry the source of another entry.
Describe 'InheritedFrom of audit entries' {
It 'Should name the folder that an inherited entry comes from, also with -ExcludeExplicit' -Skip:(-not $canReadAudit) {
$folder = New-TestSandboxItem -Sandbox $sandbox -Name 'InheritedFrom' -Directory
Add-NTFSAudit -Path $folder -Account 'S-1-1-0' -AccessRights ReadData -InheritanceFlags 'ContainerInherit, ObjectInherit' -PropagationFlags None
$file = Join-Path -Path $folder -ChildPath 'File.txt'
Assert-TestSandboxPath -Sandbox $sandbox -Path $file
Set-Content -LiteralPath $file -Value 'File'
Add-NTFSAudit -Path $file -Account 'S-1-5-32-546' -AccessRights Delete -InheritanceFlags None -PropagationFlags None
$all = @(Get-NTFSAudit -Path $file)
$inherited = @(Get-NTFSAudit -Path $file -ExcludeExplicit)
@($all | Where-Object -FilterScript { $_.IsInherited }) | Should -HaveCount 1
@($all | Where-Object -FilterScript { $_.IsInherited })[0].InheritedFrom | Should -Be $folder
$inherited | Should -HaveCount 1
$inherited[0].InheritedFrom | Should -Be $folder
}
}

19
Tests/SecurityDescriptor.Tests.ps1

@ -293,3 +293,22 @@ Describe 'FileSystemSecurity2.Write with another item' {
(Get-Acl -LiteralPath $target).GetOwner($sidType).Value | Should -Be $targetOwner
}
}
# Before 5.0.0-rc6, comparing a descriptor threw an InvalidCastException, and its hash code a NullReferenceException,
# so -eq and a hashtable with the descriptor as key failed.
Describe 'Comparing security descriptors' {
It 'Should find a descriptor equal to itself and not to another one, and use it as a key' {
$file = New-TestSandboxItem -Sandbox $sandbox -Name 'Compare'
$sd = Get-NTFSSecurityDescriptor -Path $file
$other = Get-NTFSSecurityDescriptor -Path $file
$sd -eq $sd | Should -BeTrue
$sd -eq $other | Should -BeFalse
$sd.Equals('x') | Should -BeFalse
$table = @{}
$table[$sd] = 'first'
$table[$other] = 'second'
$table[$sd] | Should -Be 'first'
$table.Count | Should -Be 2
}
}

Loading…
Cancel
Save