Fix ArgumentNullException in SecurityIdentifier.CompareTo(null)

Changed to `return 1` to conform to the interface's described behavior.
dotnet · Apr 15, 2024 · 2346719 · 2346719
1 parent 8a1e0b7
commit 2346719
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 1 deletion.
diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs
@@ -861,7 +861,10 @@ public override IdentityReference Translate(Type targetType)
 
         public int CompareTo(SecurityIdentifier? sid)
         {
-            ArgumentNullException.ThrowIfNull(sid);
+            if (sid is null)
+            {
+                return 1;
+            }
 
             if (this.IdentifierAuthority < sid.IdentifierAuthority)
             {

diff --git a/src/libraries/System.Security.Principal.Windows/tests/WellKnownSidTypeTests.cs b/src/libraries/System.Security.Principal.Windows/tests/WellKnownSidTypeTests.cs
@@ -152,4 +152,15 @@ public void MaxDefinedHasLegacyValue()
         Assert.Equal(WellKnownSidType.WinBuiltinTerminalServerLicenseServersSid, WellKnownSidType.MaxDefined);
 #pragma warning restore 0618
     }
+
+    [ConditionalTheory(nameof(AccountIsDomainJoined))]
+    [InlineData(WellKnownSidType.WorldSid)]
+    public void CompareTo_Null(WellKnownSidType sidType)
+    {
+        using (var identity = WindowsIdentity.GetCurrent())
+        {
+            var si = new SecurityIdentifier(sidType, identity.Owner.AccountDomainSid);
+            Assert.Equal(1, si.CompareTo(null));
+        }
+    }
 }