Skip to content

Commit 475fe3a

Browse files
Attempt to improve performance in checksUser
1 parent 868836e commit 475fe3a

File tree

1 file changed

+16
-5
lines changed

1 file changed

+16
-5
lines changed

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import csharp
44
import semmle.code.csharp.dataflow.flowsources.Remote
5+
import DataFlow as DF
56
import TaintTracking as TT
67
import ActionMethods
78

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

34+
private predicate authorizingCallable(Callable c) {
35+
exists(string name | name = c.getName().toLowerCase() |
36+
name.matches(["%user%", "%session%"]) and
37+
not name.matches("%get%by%") // methods like `getUserById` or `getXByUsername` aren't likely to be referring to the current user
38+
)
39+
}
40+
3341
/** Holds if `m` at some point in its call graph may make some kind of check against the current user. */
3442
private predicate checksUser(ActionMethod m) {
35-
exists(Callable c, string name | name = c.getName().toLowerCase() |
36-
name.matches(["%user%", "%session%"]) and
37-
not name.matches("%get%by%") and // methods like `getUserById` or `getXByUsername` aren't likely to be referring to the current user
38-
m.calls*(c)
43+
exists(Callable c |
44+
authorizingCallable(c) and
45+
callsPlus(m, c)
3946
)
4047
}
4148

49+
private predicate calls(Callable c1, Callable c2) { c1.calls(c2) }
50+
51+
private predicate callsPlus(Callable c1, Callable c2) = fastTC(calls/2)(c1, c2)
52+
4253
/** Holds if `m`, its containing class, or a parent class has an attribute that extends `AuthorizeAttribute` */
4354
private predicate hasAuthorizeAttribute(ActionMethod m) {
4455
exists(Attribute attr |

0 commit comments

Comments
 (0)