Skip to content

BED-7491 #273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lrfalslev wants to merge 6 commits into
base: v4
Choose a base branch
from
Open

BED-7491 #273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a3d2abd
resolve CA1416 windows-only compile time check
lrfalslev Feb 19, 2026
e223096
fix NU1701 and suppress NU1702, nuget dependency warnings
lrfalslev Feb 20, 2026
a2c3f0d
Other Test Fixes
lrfalslev Feb 20, 2026
53d8222
SYSLIB0050 - replace deprecated call
lrfalslev Feb 20, 2026
f90a0c1
fix comment syntax
lrfalslev Feb 20, 2026
a705a68
enable nullable context
lrfalslev Feb 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/CommonLib/Logging/Logging.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#nullable enable
using System.Collections.Concurrent;
using System.Collections.Concurrent;
using Microsoft.Extensions.Logging;

namespace SharpHoundCommonLib
Expand Down
7 changes: 5 additions & 2 deletions src/CommonLib/Ntlm/LdapTransport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@

#nullable enable
Copy link
Contributor Author

@lrfalslev lrfalslev Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.


using Microsoft.Extensions.Logging;
using SharpHoundCommonLib.Enums;
using System;
Expand Down Expand Up @@ -85,4 +86,6 @@ public void Dispose() {
_disposed = true;
}
}
}
}

#nullable disable
14 changes: 9 additions & 5 deletions src/CommonLib/Processors/DCLdapProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using Microsoft.Extensions.Logging;
#nullable enable

using Microsoft.Extensions.Logging;
using SharpHoundCommonLib.Enums;
using SharpHoundCommonLib.Ntlm;
using SharpHoundCommonLib.OutputTypes;
Expand Down Expand Up @@ -33,7 +35,7 @@ public class DCLdapProcessor {
private readonly string SEC_E_BAD_BINDINGS = "80090346";


public DCLdapProcessor(int connectionTimeoutMs, string dcHostname, ILogger log = null) {
public DCLdapProcessor(int connectionTimeoutMs, string dcHostname, ILogger? log = null) {
_log = log ?? Logging.LogProvider.CreateLogger("DCLdapProcessor");
_scanner = new PortScanner(maxTimeout: connectionTimeoutMs);
_ldapTimeout = connectionTimeoutMs / 1000;
Expand All @@ -43,7 +45,7 @@ public DCLdapProcessor(int connectionTimeoutMs, string dcHostname, ILogger log =
_checkIsChannelBindingDisabledAdaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromMinutes(1), Logging.LogProvider.CreateLogger(nameof(CheckIsChannelBindingDisabled)));
}

public event ComputerStatusDelegate ComputerStatusEvent;
public event ComputerStatusDelegate? ComputerStatusEvent;

public async Task<LdapService> Scan(string computerName, string computerObjectId) {
var hasLdap = await TestLdapPort();
Expand Down Expand Up @@ -173,7 +175,7 @@ public virtual async Task<bool> TestLdapsPort() {
/// <param name="endpoint"></param>
/// <param name="options"></param>
/// <returns></returns>
protected internal virtual async Task<bool> Authenticate(Uri endpoint, LdapAuthOptions options, NtlmAuthenticationHandler ntlmAuth = null, LdapTransport ldapTransport = null, CancellationToken cancellationToken = default) {
protected internal virtual async Task<bool> Authenticate(Uri endpoint, LdapAuthOptions options, NtlmAuthenticationHandler? ntlmAuth = null, LdapTransport? ldapTransport = null, CancellationToken cancellationToken = default) {
var host = endpoint.Host;
var auth = ntlmAuth ?? new NtlmAuthenticationHandler($"LDAP/{host.ToUpper()}") {
Options = options
Expand Down Expand Up @@ -229,4 +231,6 @@ protected internal virtual async Task<bool> Authenticate(Uri endpoint, LdapAuthO
private async Task SendComputerStatus(CSVComputerStatus status) {
if (ComputerStatusEvent is not null) await ComputerStatusEvent.Invoke(status);
}
}
}

#nullable disable
6 changes: 5 additions & 1 deletion src/CommonLib/SMB/NetBIOS/NetBIOSSessionType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System;
#nullable enable

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
Expand Down Expand Up @@ -127,3 +129,5 @@ public override bool Equals(object? obj) =>
!(left == right);
}
}

#nullable disable
2 changes: 1 addition & 1 deletion src/CommonLib/SharpHoundCommonLib.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<DebugType>full</DebugType>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="AntiXSS" Version="4.3.0" />
<PackageReference Include="AntiXSS" Version="4.3.0" PrivateAssets="All"/>
Copy link
Contributor Author

@lrfalslev lrfalslev Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marked as private to avoid transitive nuget restore in test projects.

warning NU1701: Package 'AntiXSS 4.3.0' was restored using '.NETFramework,Version=v4.6.1, .NETFramework,Version=v4.6.2, .NETFramework,Version=v4.7, .NETFramework,Version=v4.7.1, .NETFramework,Version=v4.7.2, .NETFramework,Version=v4.8, .NETFramework,Version=v4.8.1' instead of the project target framework 'net8.0'. This package may not be fully compatible with your project.

<PackageReference Include="Microsoft.Bcl.AsyncInterfaces" Version="8.0.0" />
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="8.0.0" />
</ItemGroup>
Expand Down
6 changes: 6 additions & 0 deletions test/unit/ACLProcessorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Diagnostics.CodeAnalysis;
using System.DirectoryServices;
using System.Linq;
using System.Runtime.Versioning;
using System.Security.AccessControl;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -60,6 +61,7 @@ public void ACLProcessor_IsACLProtected_NullNTSD_ReturnsFalse() {
Assert.False(result);
}

[SupportedOSPlatform("windows")]
Copy link
Contributor Author

@lrfalslev lrfalslev Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolves warning CA1416: This call site is reachable on all platforms.

[WindowsOnlyFact]
public async Task ACLProcessor_TestKnownDataAddMember() {
var mockLdapUtils = new MockLdapUtils();
Expand Down Expand Up @@ -1421,6 +1423,7 @@ public void Test_ACLProcessor_IsACLProtected_NotProtected() {
Assert.False(result);
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public void ACLProcessor_CalculateImplicitACLHash_ValidInput_ReturnsCorrectHash()
{
Expand All @@ -1439,6 +1442,7 @@ public void ACLProcessor_CalculateImplicitACLHash_ValidInput_ReturnsCorrectHash(
Assert.Equal(expectedHash, result);
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public void ACLProcessor_CalculateImplicitACLHash_DifferentInputs_ProducesUniqueHashes()
{
Expand All @@ -1458,6 +1462,7 @@ public void ACLProcessor_CalculateImplicitACLHash_DifferentInputs_ProducesUnique
Assert.NotEqual(protectedResult, adminsdResult);
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public void ACLProcessor_NullAdminSDHolderHash_Returns_Null_Bool()
{
Expand All @@ -1476,6 +1481,7 @@ public void ACLProcessor_NullAdminSDHolderHash_Returns_Null_Bool()
}


[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public void ACLProcessor_AdminSDHolderHash_Returns_Match()
{
Expand Down
17 changes: 17 additions & 0 deletions test/unit/CertAbuseProcessorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Runtime.Versioning;
using System.Security.AccessControl;
using System.Security.Principal;
using System.Threading;
Expand Down Expand Up @@ -226,6 +227,7 @@ public async Task CertAbuseProcessor_ProcessEAPermissions_HandlesFailedLookup()
Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId);
}

[SupportedOSPlatform("windows")]
public static IEnumerable<object[]> ProcessEAPermissionsTestData() {
return new List<object[]>
{
Expand All @@ -234,6 +236,7 @@ public static IEnumerable<object[]> ProcessEAPermissionsTestData() {
};
}

[SupportedOSPlatform("windows")]
[WindowsOnlyTheory]
[MemberData(nameof(ProcessEAPermissionsTestData))]
public async Task CertAbuseProcessor_ProcessEAPermissions_ReturnsEmpty(RawAcl dacl) {
Expand Down Expand Up @@ -335,6 +338,7 @@ public async Task CertAbuseProcessor_ProcessRegistryEnrollmentPermissions_Handle
Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId);
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_ProcessRegistryEnrollmentPermissions_ReturnsEmpty_WhenNoOwnerAndNoRules() {
var mockSecurityDescriptor = new Mock<ActiveDirectorySecurityDescriptor>(null);
Expand Down Expand Up @@ -405,6 +409,7 @@ public async Task CertAbuseProcessor_ProcessCertTemplates_ReturnsResolvedAndUnre
Assert.Contains(invalidCN, results.unresolvedTemplates);
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_GetRegistryPrincipal_ReturnsFalseForFilteredSID() {
var sid = new SecurityIdentifier("S-1-5-3");
Expand All @@ -422,6 +427,7 @@ public async Task CertAbuseProcessor_GetRegistryPrincipal_ReturnsFalseForFiltere
_mockLdapUtils.VerifyNoOtherCalls();
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_GetRegistryPrincipal_CallsResolveIDAndType_ForDomainController() {
var expectedPrincipalType = Label.Group;
Expand Down Expand Up @@ -450,6 +456,7 @@ public async Task CertAbuseProcessor_GetRegistryPrincipal_CallsResolveIDAndType_
_mockLdapUtils.VerifyNoOtherCalls();
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_GetRegistryPrincipal_CallsConvertLocalWellKnownPrincipal_ForNonDomainController() {
var expectedPrincipalType = Label.Group;
Expand Down Expand Up @@ -477,6 +484,7 @@ public async Task CertAbuseProcessor_GetRegistryPrincipal_CallsConvertLocalWellK
_mockLdapUtils.VerifyNoOtherCalls();
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_GetRegistryPrincipal_ResolvesToLocalPrincipal_ForLocalSID() {
var expectedPrincipalType = Label.LocalGroup;
Expand Down Expand Up @@ -504,6 +512,7 @@ public async Task CertAbuseProcessor_GetRegistryPrincipal_ResolvesToLocalPrincip
_mockLdapUtils.VerifyNoOtherCalls();
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_GetRegistryPrincipal_ResolvesToDomainPrincipal() {
var expectedPrincipalType = Label.Group;
Expand Down Expand Up @@ -558,6 +567,7 @@ public void CertAbuseProcessor_OpenSamServer_CallsOpenServer_Success() {
Assert.IsType<SAMServer>(result.Value);
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_GetMachineSid_ReturnsCachedValue() {
Cache.AddMachineSid(TargetDomainSid, TargetDomainSid);
Expand Down Expand Up @@ -586,6 +596,7 @@ public async Task CertAbuseProcessor_GetMachineSid_OpenSAMFailure_ReturnsNull()
Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId);
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_GetMachineSid_GetMachineSidFailure_ReturnsNull() {
var mockSamServer = new Mock<ISAMServer>();
Expand All @@ -608,6 +619,7 @@ public async Task CertAbuseProcessor_GetMachineSid_GetMachineSidFailure_ReturnsN
Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId);
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_GetMachineSid_ReturnsSid() {
var mockSamServer = new Mock<ISAMServer>();
Expand All @@ -629,6 +641,7 @@ public async Task CertAbuseProcessor_GetMachineSid_ReturnsSid() {
Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId);
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_NullOpaque_ReturnsFalse() {
var nullOpaqueAce = new CommonAce(
Expand All @@ -649,6 +662,7 @@ public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_NullOpaque
_mockLdapUtils.VerifyNoOtherCalls();
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_UnresolvedTemplate_ReturnsFalse() {
var emptyOpaqueAce = new CommonAce(
Expand Down Expand Up @@ -676,6 +690,7 @@ public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_Unresolved
Assert.Null(result.restriction);
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_NoTemplate_ReturnsAllTemplates() {
var emptyOpaqueAce = new CommonAce(
Expand Down Expand Up @@ -709,6 +724,7 @@ public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_NoTemplate
Assert.Contains(result.restriction.Targets, t => t.ObjectIdentifier == "S-1-3");
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_WithCanonicalName_ReturnsTemplate() {
var expectedPrincipalType = Label.CertTemplate;
Expand Down Expand Up @@ -742,6 +758,7 @@ public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_WithCanoni
Times.Once);
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_WithCertTemplateOID_ReturnsTemplate() {
var expectedPrincipalType = Label.CertTemplate;
Expand Down
13 changes: 3 additions & 10 deletions test/unit/CommonLibHelperTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using System;
using System.Security.Principal;
using System.Runtime.Versioning;
using System.Text;
using System.Threading.Tasks;
using SharpHoundCommonLib;
Expand Down Expand Up @@ -302,6 +302,7 @@ public void DomainNameToDistinguishedName_DotsBecomeDcComponents()
Assert.Equal("DC=test,DC=local", result);
}

[SupportedOSPlatform("windows")]
[WindowsOnlyTheory]
[InlineData("S-1-5-32-544", "\\01\\02\\00\\00\\00\\00\\00\\05\\20\\00\\00\\00\\20\\02\\00\\00")]
public void ConvertSidToHexSid_ValidSid_MatchesSecurityIdentifierBinaryForm(string sid, string expectedHexSid)
Expand All @@ -311,17 +312,9 @@ public void ConvertSidToHexSid_ValidSid_MatchesSecurityIdentifierBinaryForm(stri

// Assert
Assert.Equal(expectedHexSid, actual);
return;

static string BuildExpectedHexSid(string sid)
{
var securityIdentifier = new SecurityIdentifier(sid);
var sidBytes = new byte[securityIdentifier.BinaryLength];
securityIdentifier.GetBinaryForm(sidBytes, 0);
return $"\\{BitConverter.ToString(sidBytes).Replace('-', '\\')}";
}
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public void ConvertSidToHexSid_InvalidSid_Throws()
{
Expand Down
2 changes: 2 additions & 0 deletions test/unit/CommonLibTest.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
<CollectCoverage>true</CollectCoverage>
<CoverletOutput>..\..\docfx\coverage\</CoverletOutput>
<CoverletOutputFormat>OpenCover</CoverletOutputFormat>
<!-- Suppress cross-targeting warning when referencing net472 SharpHound projects in net8.0 tests -->
<NoWarn>$(NoWarn);NU1702</NoWarn>
Copy link
Contributor Author

@lrfalslev lrfalslev Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning NU1702: ProjectReference 'SharpHoundCommon\src\CommonLib\SharpHoundCommonLib.csproj' was resolved using '.NETFramework,Version=v4.7.2' instead of the project target framework '.NETCoreApp,Version=v8.0'. This project may not be fully compatible with your project.

</PropertyGroup>

<!-- Project references -->
Expand Down
2 changes: 0 additions & 2 deletions test/unit/DCLdapProcessorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ public async Task DCLdapProcessor_CheckIsNtlmSigningRequired_Exception() {

[Fact]
public async Task DCLdapProcessor_Authenticate_InvalidCredentialsException_SEC_E_UNSUPPORTED_FUNCTION() {
var exception = "ErrorTest";
var endpoint = "http://primary.testlab.local/";
var expected = $"LDAP endpoint '{endpoint}' does not support NTLM";

Expand All @@ -153,7 +152,6 @@ public async Task DCLdapProcessor_Authenticate_InvalidCredentialsException_SEC_E

[Fact]
public async Task DCLdapProcessor_Authenticate_InvalidCredentialsException_SEC_E_BAD_BINDINGS() {
var exception = "ErrorTest";
var endpoint = "http://primary.testlab.local/";
var expected = $"Bad bindings with the LDAPS endpoint '{endpoint}'. Server error: {SEC_E_BAD_BINDINGS}";

Expand Down
2 changes: 2 additions & 0 deletions test/unit/DomainTrustProcessorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.DirectoryServices.Protocols;
using System.Linq;
using System.Runtime.Versioning;
using System.Threading;
using System.Threading.Tasks;
using CommonLibTest.Facades;
Expand All @@ -23,6 +24,7 @@ public DomainTrustProcessorTest(ITestOutputHelper testOutputHelper)
_testOutputHelper = testOutputHelper;
}

[SupportedOSPlatform("windows")]
[WindowsOnlyFact]
public async Task DomainTrustProcessor_EnumerateDomainTrusts_HappyPath()
{
Expand Down
6 changes: 3 additions & 3 deletions test/unit/Facades/FacadeHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using System.Reflection;
using System.Runtime.Serialization;
using System.Runtime.CompilerServices;

namespace CommonLibTest.Facades
{
Expand All @@ -9,8 +9,8 @@ public class FacadeHelpers
private const BindingFlags publicInstance = BindingFlags.Public | BindingFlags.Instance;

internal static T GetUninitializedObject<T>()
{
return (T) FormatterServices.GetUninitializedObject(typeof(T));
Copy link
Contributor Author

@lrfalslev lrfalslev Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FormatterServices has been deprecated.

SYSLIB0050: Class 'System.Runtime.Serialization.FormatterServices' is obsolete: 'Formatter-based serialization is obsolete and should not be used.'

{
return (T)RuntimeHelpers.GetUninitializedObject(typeof(T));
}

internal static void SetField<T1, T2>(T1 obj, string propertyName, T2 propertyValue)
Expand Down
Loading