Merge pull request #19302 from michaelnebel/csharp/missing-access-control

C#: Relax condition for authorize attributes on `cs/web/missing-function-level-access-control`.

csharp/ql/lib/semmle/code/csharp/security/auth/MissingFunctionLevelAccessControlQuery.qll

@@ -81,7 +81,7 @@ predicate hasAuthViaXml(ActionMethod m) {
 
 /** Holds if the given action has an attribute that indications authorization. */
 predicate hasAuthViaAttribute(ActionMethod m) {
-  exists(Attribute attr | attr.getType().getName().toLowerCase().matches("%auth%") |
+  exists(Attribute attr | attr.getType().getABaseType*().getName().toLowerCase().matches("%auth%") |
     attr = m.getOverridee*().getAnAttribute() or
     attr = getAnUnboundBaseType*(m.getDeclaringType()).getAnAttribute()
   )

csharp/ql/src/change-notes/2025-04-15-missing-function-level-access-control.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Improved detection of authorization checks in the `cs/web/missing-function-level-access-control` query. The query now recognizes authorization attributes inherited from base classes and interfaces.

csharp/ql/test/query-tests/Security Features/CWE-285/MissingAccessControl/MVCTests/MissingAccessControl.expected

@@ -1 +1 @@
-| ProfileController.cs:9:25:9:31 | Delete1 | This action is missing an authorization check. |
+| ProfileController.cs:12:25:12:31 | Delete1 | This action is missing an authorization check. |

csharp/ql/test/query-tests/Security Features/CWE-285/MissingAccessControl/MVCTests/MissingAccessControl.qlref

@@ -1 +1,4 @@
-Security Features/CWE-285/MissingAccessControl.ql
+query: Security Features/CWE-285/MissingAccessControl.ql
+postprocess:
+  - utils/test/PrettyPrintModels.ql
+  - utils/test/InlineExpectationsTestQuery.ql

csharp/ql/test/query-tests/Security Features/CWE-285/MissingAccessControl/MVCTests/ProfileController.cs

@@ -1,19 +1,25 @@
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Authorization;
 
-public class ProfileController : Controller {
+public class RequirePermissionAttribute : AuthorizeAttribute { }
+
+public class ProfileController : Controller
+{
     private void doThings() { }
     private bool isAuthorized() { return false; }
 
     // BAD: This is a Delete method, but no auth is specified.
-    public ActionResult Delete1(int id) {
+    public ActionResult Delete1(int id) // $ Alert
+    {
         doThings();
         return View();
     }
 
     // GOOD: isAuthorized is checked.
-    public ActionResult Delete2(int id) {
-        if (!isAuthorized()) {
+    public ActionResult Delete2(int id)
+    {
+        if (!isAuthorized())
+        {
             return null;
         }
         doThings();
@@ -22,35 +28,49 @@ public class ProfileController : Controller {
 
     // GOOD: The Authorize attribute is used.
     [Authorize]
-    public ActionResult Delete3(int id) {
+    public ActionResult Delete3(int id)
+    {
         doThings();
         return View();
     }
 
-}
-
-[Authorize]
-public class AuthBaseController : Controller {
-    protected void doThings() { }
-}
-
-public class SubController : AuthBaseController {
-    // GOOD: The Authorize attribute is used on the base class.
-    public ActionResult Delete4(int id) {
+    // GOOD: The RequirePermission attribute is used (which extends AuthorizeAttribute).
+    [RequirePermission]
+    public ActionResult Delete4(int id)
+    {
         doThings();
         return View();
     }
 }
 
 [Authorize]
-public class AuthBaseGenericController<T> : Controller {
+public class AuthBaseController : Controller
+{
     protected void doThings() { }
 }
 
-public class SubGenericController : AuthBaseGenericController<string> {
+public class SubController : AuthBaseController
+{
     // GOOD: The Authorize attribute is used on the base class.
-    public ActionResult Delete5(int id) {
+    public ActionResult Delete4(int id)
+    {
         doThings();
         return View();
     }
-}
+}
+
+[Authorize]
+public class AuthBaseGenericController<T> : Controller
+{
+    protected void doThings() { }
+}
+
+public class SubGenericController : AuthBaseGenericController<string>
+{
+    // GOOD: The Authorize attribute is used on the base class.
+    public ActionResult Delete5(int id)
+    {
+        doThings();
+        return View();
+    }
+}