Seal methods that satisfy non-public interfaces

Rule ND3106 Security

Matches public virtual methods of non-sealed public classes that override a method of an internal interface. The non-public interface indicates a process meant to remain private: a malicious subclass can override the method behavior, which private implementation will then invoke — a security vulnerability.

Flagged Code
internal interface ILicenseChecker { bool IsLicensed(); }

public class Product : ILicenseChecker {
   public virtual bool IsLicensed() => _license.IsValid;   // flagged: a subclass can override it
}
Fixed Code
internal interface ILicenseChecker { bool IsLicensed(); }

public sealed class Product : ILicenseChecker {
   public bool IsLicensed() => _license.IsValid;   // non-virtual method, sealed class
}

How to fix

Either seal the parent class, make it non-public, implement the method without virtual, or make the method non-public. Suppress the issue if no security threat exists.

CQLinq source

This rule is written in CQLinq, C# LINQ over the NDepend code model. It ships with every new NDepend project: you can read it, adapt it, or use it as a starting point for your own custom rules.

// <Name>Seal methods that satisfy non-public interfaces</Name>
// <Id>ND3106:SealMethodsThatSatisfyNonPublicInterfaces</Id>

warnif count > 0
from m in Application.Methods
where 
  !m.IsFinal && // If the method is not marked as virtual the flag final is set
   m.IsPubliclyVisible &&
  !m.ParentType.IsSealed &&
   m.ParentType.IsClass
let overridenInterface = 
   m.OverriddensBase.FirstOrDefault(mb => mb.ParentType.IsInterface && !mb.ParentType.IsPubliclyVisible)
where overridenInterface != null
select new { 
  m,
  overridenInterface = overridenInterface.ParentType,
  Debt = 30.ToMinutes().ToDebt(),
  Severity = Severity.High
 }

//<Expl>
// The *public {0}* overrides a method defined 
// in the non-public *{1}*.
// This situation might be a security vulnerability because 
// it allows the creation of a malicious subclass that can 
// override the method behavior.
//</Expl>

//<Desc>
// Matches public virtual methods of non-sealed public classes that override
// a method of an internal interface. The non-public interface indicates a
// process meant to remain private: a malicious subclass can override the
// method behavior, which private implementation will then invoke — a
// security vulnerability.
//
// **Flagged Code**
// ```csharp
// internal interface ILicenseChecker { bool IsLicensed(); }
//
// public class Product : ILicenseChecker {
//    public virtual bool IsLicensed() => _license.IsValid;   // flagged: a subclass can override it
// }
// ```
//
// **Fixed Code**
// ```csharp
// internal interface ILicenseChecker { bool IsLicensed(); }
//
// public sealed class Product : ILicenseChecker {
//    public bool IsLicensed() => _license.IsValid;   // non-virtual method, sealed class
// }
// ```
//</Desc>

//<HowToFix>
// Either seal the parent class, make it non-public, implement the method
// without *virtual*, or make the method non-public. Suppress the issue if
// no security threat exists.
//</HowToFix>

Open in the interactive Rules Explorer · All default rules

Security: all 39 items