Review publicly visible event handlers

Rule ND3104 Security

Review publicly visible event handlers (methods with the standard (Object,EventArgs) signature) running security critical actions: malicious code can subscribe them to an event to trigger the action on firing. Even with a security check, the handler can run from a chain of trusted callers and cannot detect malicious registration.

Flagged Code
public class AdminPanel {
   public void OnDeleteAllUsers(object sender, EventArgs e) {   // flagged: public handler
      _userStore.DeleteAll();
   }
}
Fixed Code
public class AdminPanel {
   private void OnDeleteAllUsers(object sender, EventArgs e) {   // non-public handler
      if (!_currentUser.IsAdministrator) return;
      _userStore.DeleteAll();
   }
}

How to fix

Make matched handlers non-public; preferably don't run security critical actions from event handlers. Suppress the issue if no such action is involved.

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>Review publicly visible event handlers</Name>
// <Id>ND3104:ReviewPubliclyVisibleEventHandlers</Id>
warnif count > 0 
from m in Application.Methods
where 
  m.IsPubliclyVisible
  && m.Name.EndsWith("(Object,EventArgs)")
select new { 
  m,
  Debt = m.NbLinesOfCode.Linear(3, 2, 300, 12).ToMinutes().ToDebt(),
  Severity = Severity.Low
}

//<Expl>
// The *public {0}* is an event handler that might perform a security critical action. Review it!
//</Expl>

//<Desc>
// Review publicly visible event handlers (methods with the standard
// *(Object,EventArgs)* signature) running security critical actions:
// malicious code can subscribe them to an event to trigger the action on
// firing. Even with a security check, the handler can run from a chain of
// trusted callers and cannot detect malicious registration.
//
// **Flagged Code**
// ```csharp
// public class AdminPanel {
//    public void OnDeleteAllUsers(object sender, EventArgs e) {   // flagged: public handler
//       _userStore.DeleteAll();
//    }
// }
// ```
//
// **Fixed Code**
// ```csharp
// public class AdminPanel {
//    private void OnDeleteAllUsers(object sender, EventArgs e) {   // non-public handler
//       if (!_currentUser.IsAdministrator) return;
//       _userStore.DeleteAll();
//    }
// }
// ```
//</Desc>

//<HowToFix>
// Make matched handlers non-public; preferably don't run security critical
// actions from event handlers. Suppress the issue if no such action is
// involved.
//</HowToFix>

Open in the interactive Rules Explorer · All default rules

Security: all 39 items