Add IDOR query

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

@@ -4,25 +4,46 @@ import csharp
 import semmle.code.csharp.dataflow.flowsources.Remote
 import ActionMethods
 
+/**
+ * Holds if `m` is a method that may require checks
+ * against the current user to modify a particular resource.
+ */
+// We exclude admin methods as it may be expected that an admin user should be able to modify any resource.
+// Other queries check that there are authorization checks in place for admin methods.
 private predicate needsChecks(ActionMethod m) { m.isEdit() and not m.isAdmin() }
 
+private Expr getParentExpr(Expr ex) { result = ex.getParent() }
+
+/**
+ * Holds if `m` has a parameter or access a remote flow source
+ * that may indicate that it's used as the ID for some resource
+ */
 private predicate hasIdParameter(ActionMethod m) {
   exists(RemoteFlowSource src | src.getEnclosingCallable() = m |
-    src.asParameter().getName().toLowerCase().matches("%id")
+    src.asParameter().getName().toLowerCase().matches(["%id", "%idx"])
     or
     exists(StringLiteral idStr |
-      idStr.getValue().toLowerCase().matches("%id") and
-      idStr.getParent*() = src.asExpr()
+      idStr.getValue().toLowerCase().matches(["%id", "%idx"]) and
+      getParentExpr*(src.asExpr()) = getParentExpr*(idStr)
     )
   )
 }
 
+/** Holds if `m` at some point in its call graph may make some kind of check against the current user. */
 private predicate checksUser(ActionMethod m) {
-  exists(Callable c | c.getName().toLowerCase().matches("%user%") | m.calls*(c))
+  exists(Property p | p.getName().toLowerCase().matches(["%user%", "%session%"]) |
+    m.calls*(p.getGetter())
+  )
 }
 
+/**
+ * Holds if `m` is a method that modifies a particular resource based on
+ * and ID provided by user input, but does not check anything based on the current user
+ * to determine if they should modify this resource.
+ */
 predicate hasInsecureDirectObjectReference(ActionMethod m) {
   needsChecks(m) and
   hasIdParameter(m) and
-  not checksUser(m)
+  not checksUser(m) and
+  exists(m.getBody())
 }

csharp/ql/src/Security Features/CWE-939/InsecureDirectObjectReference.ql

@@ -0,0 +1,19 @@
+/**
+ * @name Insecure Direct Object Reference
+ * @description Using user input to control which object is modified without
+ *              proper authorization checks allows an attacker to modify arbitrary objects.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision medium
+ * @id cs/insecure-direct0object-reference
+ * @tags security
+ *       external/cwe/639
+ */
+
+import csharp
+import semmle.code.csharp.security.auth.InsecureDirectObjectRefrerenceQuery
+
+from ActionMethod m
+where hasInsecureDirectObjectReference(m)
+select m,
+  "This method may not verify which users should be able to access resources of the provided ID."