Skip to content

Commit

Permalink
Attempt to improve performance in checksUser
Browse files Browse the repository at this point in the history
  • Loading branch information
joefarebrother committed Sep 20, 2023
1 parent 868836e commit 475fe3a
Showing 1 changed file with 16 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import csharp
import semmle.code.csharp.dataflow.flowsources.Remote
import DataFlow as DF
import TaintTracking as TT
import ActionMethods

Expand All @@ -25,20 +26,30 @@ private predicate hasIdParameter(ActionMethod m) {
exists(StringLiteral idStr, IndexerCall idx |
idStr.getValue().toLowerCase().matches(["%id", "%idx"]) and
TT::localTaint(src, DataFlow::exprNode(idx.getQualifier())) and
idStr = idx.getArgument(0)
DF::localExprFlow(idStr, idx.getArgument(0))
)
)
}

private predicate authorizingCallable(Callable c) {
exists(string name | name = c.getName().toLowerCase() |
name.matches(["%user%", "%session%"]) and
not name.matches("%get%by%") // methods like `getUserById` or `getXByUsername` aren't likely to be referring to the current user
)
}

/** 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, string name | name = c.getName().toLowerCase() |
name.matches(["%user%", "%session%"]) and
not name.matches("%get%by%") and // methods like `getUserById` or `getXByUsername` aren't likely to be referring to the current user
m.calls*(c)
exists(Callable c |
authorizingCallable(c) and
callsPlus(m, c)
)
}

private predicate calls(Callable c1, Callable c2) { c1.calls(c2) }

private predicate callsPlus(Callable c1, Callable c2) = fastTC(calls/2)(c1, c2)

/** Holds if `m`, its containing class, or a parent class has an attribute that extends `AuthorizeAttribute` */
private predicate hasAuthorizeAttribute(ActionMethod m) {
exists(Attribute attr |
Expand Down

0 comments on commit 475fe3a

Please sign in to comment.