Skip to content

Commit 66be8cd

Browse files
erik-krogherik-krogh
committed
remove more of the implementation into ConditionalBypassQuery.qll
1 parent 442749b commit 66be8cd

File tree

3 files changed

+93
-187
lines changed

3 files changed

+93
-187
lines changed

‎javascript/ql/lib/semmle/javascript/security/dataflow/ConditionalBypassQuery.qll

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,96 @@ class Configuration extends TaintTracking::Configuration {
3030
dst.asExpr().(Comparison).hasOperands(src.asExpr(), any(ConstantExpr c))
3131
}
3232
}
33+
34+
/**
35+
* Holds if the value of `nd` flows into `guard`.
36+
*/
37+
predicate flowsToGuardExpr(DataFlow::Node nd, SensitiveActionGuardConditional guard) {
38+
nd = guard or
39+
flowsToGuardExpr(nd.getASuccessor(), guard)
40+
}
41+
42+
/**
43+
* A comparison that guards a sensitive action, e.g. the comparison in:
44+
* `var ok = x == y; if (ok) login()`.
45+
*/
46+
class SensitiveActionGuardComparison extends Comparison {
47+
SensitiveActionGuardConditional guard;
48+
49+
SensitiveActionGuardComparison() { flowsToGuardExpr(DataFlow::valueNode(this), guard) }
50+
51+
/**
52+
* Gets the guard that uses this comparison.
53+
*/
54+
SensitiveActionGuardConditional getGuard() { result = guard }
55+
}
56+
57+
/**
58+
* An intermediary sink to enable reuse of the taint configuration.
59+
* This sink should not be presented to the client of this query.
60+
*/
61+
class SensitiveActionGuardComparisonOperand extends Sink {
62+
SensitiveActionGuardComparison comparison;
63+
64+
SensitiveActionGuardComparisonOperand() { asExpr() = comparison.getAnOperand() }
65+
66+
override SensitiveAction getAction() { result = comparison.getGuard().getAction() }
67+
}
68+
69+
/**
70+
* Holds if `sink` guards `action`, and `source` taints `sink`.
71+
*
72+
* If flow from `source` taints `sink`, then an attacker can
73+
* control if `action` should be executed or not.
74+
*/
75+
predicate isTaintedGuardForSensitiveAction(
76+
DataFlow::PathNode sink, DataFlow::PathNode source, SensitiveAction action
77+
) {
78+
action = sink.getNode().(Sink).getAction() and
79+
// exclude the intermediary sink
80+
not sink.getNode() instanceof SensitiveActionGuardComparisonOperand and
81+
exists(Configuration cfg |
82+
// ordinary taint tracking to a guard
83+
cfg.hasFlowPath(source, sink)
84+
or
85+
// taint tracking to both operands of a guard comparison
86+
exists(
87+
SensitiveActionGuardComparison cmp, DataFlow::PathNode lSource, DataFlow::PathNode rSource,
88+
DataFlow::PathNode lSink, DataFlow::PathNode rSink
89+
|
90+
sink.getNode() = cmp.getGuard() and
91+
cfg.hasFlowPath(lSource, lSink) and
92+
lSink.getNode() = DataFlow::valueNode(cmp.getLeftOperand()) and
93+
cfg.hasFlowPath(rSource, rSink) and
94+
rSink.getNode() = DataFlow::valueNode(cmp.getRightOperand())
95+
|
96+
source = lSource or
97+
source = rSource
98+
)
99+
)
100+
}
101+
102+
/**
103+
* Holds if `e` effectively guards access to `action` by returning or throwing early.
104+
*
105+
* Example: `if (e) return; action(x)`.
106+
*/
107+
predicate isEarlyAbortGuard(DataFlow::PathNode e, SensitiveAction action) {
108+
exists(IfStmt guard |
109+
// `e` is in the condition of an if-statement ...
110+
e.getNode().(Sink).asExpr().getParentExpr*() = guard.getCondition() and
111+
// ... where the then-branch always throws or returns
112+
exists(Stmt abort |
113+
abort instanceof ThrowStmt or
114+
abort instanceof ReturnStmt
115+
|
116+
abort.nestedIn(guard) and
117+
abort.getBasicBlock().(ReachableBasicBlock).postDominates(guard.getThen().getBasicBlock())
118+
) and
119+
// ... and the else-branch does not exist
120+
not exists(guard.getElse())
121+
|
122+
// ... and `action` is outside the if-statement
123+
not action.asExpr().getEnclosingStmt().nestedIn(guard)
124+
)
125+
}

‎javascript/ql/src/Security/CWE-807/ConditionalBypass.ql

Lines changed: 0 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -15,99 +15,6 @@ import javascript
1515
import semmle.javascript.security.dataflow.ConditionalBypassQuery
1616
import DataFlow::PathGraph
1717

18-
/**
19-
* Holds if the value of `nd` flows into `guard`.
20-
*/
21-
predicate flowsToGuardExpr(DataFlow::Node nd, SensitiveActionGuardConditional guard) {
22-
nd = guard or
23-
flowsToGuardExpr(nd.getASuccessor(), guard)
24-
}
25-
26-
/**
27-
* A comparison that guards a sensitive action, e.g. the comparison in:
28-
* `var ok = x == y; if (ok) login()`.
29-
*/
30-
class SensitiveActionGuardComparison extends Comparison {
31-
SensitiveActionGuardConditional guard;
32-
33-
SensitiveActionGuardComparison() { flowsToGuardExpr(DataFlow::valueNode(this), guard) }
34-
35-
/**
36-
* Gets the guard that uses this comparison.
37-
*/
38-
SensitiveActionGuardConditional getGuard() { result = guard }
39-
}
40-
41-
/**
42-
* An intermediary sink to enable reuse of the taint configuration.
43-
* This sink should not be presented to the client of this query.
44-
*/
45-
class SensitiveActionGuardComparisonOperand extends Sink {
46-
SensitiveActionGuardComparison comparison;
47-
48-
SensitiveActionGuardComparisonOperand() { asExpr() = comparison.getAnOperand() }
49-
50-
override SensitiveAction getAction() { result = comparison.getGuard().getAction() }
51-
}
52-
53-
/**
54-
* Holds if `sink` guards `action`, and `source` taints `sink`.
55-
*
56-
* If flow from `source` taints `sink`, then an attacker can
57-
* control if `action` should be executed or not.
58-
*/
59-
predicate isTaintedGuardForSensitiveAction(
60-
DataFlow::PathNode sink, DataFlow::PathNode source, SensitiveAction action
61-
) {
62-
action = sink.getNode().(Sink).getAction() and
63-
// exclude the intermediary sink
64-
not sink.getNode() instanceof SensitiveActionGuardComparisonOperand and
65-
exists(Configuration cfg |
66-
// ordinary taint tracking to a guard
67-
cfg.hasFlowPath(source, sink)
68-
or
69-
// taint tracking to both operands of a guard comparison
70-
exists(
71-
SensitiveActionGuardComparison cmp, DataFlow::PathNode lSource, DataFlow::PathNode rSource,
72-
DataFlow::PathNode lSink, DataFlow::PathNode rSink
73-
|
74-
sink.getNode() = cmp.getGuard() and
75-
cfg.hasFlowPath(lSource, lSink) and
76-
lSink.getNode() = DataFlow::valueNode(cmp.getLeftOperand()) and
77-
cfg.hasFlowPath(rSource, rSink) and
78-
rSink.getNode() = DataFlow::valueNode(cmp.getRightOperand())
79-
|
80-
source = lSource or
81-
source = rSource
82-
)
83-
)
84-
}
85-
86-
/**
87-
* Holds if `e` effectively guards access to `action` by returning or throwing early.
88-
*
89-
* Example: `if (e) return; action(x)`.
90-
*/
91-
predicate isEarlyAbortGuard(DataFlow::PathNode e, SensitiveAction action) {
92-
exists(IfStmt guard |
93-
// `e` is in the condition of an if-statement ...
94-
e.getNode().(Sink).asExpr().getParentExpr*() = guard.getCondition() and
95-
// ... where the then-branch always throws or returns
96-
exists(Stmt abort |
97-
abort instanceof ThrowStmt or
98-
abort instanceof ReturnStmt
99-
|
100-
abort.nestedIn(guard) and
101-
abort.getBasicBlock().(ReachableBasicBlock).postDominates(guard.getThen().getBasicBlock())
102-
) and
103-
// ... and the else-branch does not exist
104-
not exists(guard.getElse())
105-
|
106-
// ... and `action` is outside the if-statement
107-
not action.asExpr().getEnclosingStmt().nestedIn(guard)
108-
)
109-
}
110-
11118
from DataFlow::PathNode source, DataFlow::PathNode sink, SensitiveAction action
11219
where
11320
isTaintedGuardForSensitiveAction(sink, source, action) and

‎javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-807/ConditionalBypass.ql

Lines changed: 0 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -15,100 +15,6 @@
1515
import javascript
1616
import semmle.javascript.security.dataflow.ConditionalBypassQuery
1717
import DataFlow::PathGraph
18-
19-
/**
20-
* Holds if the value of `nd` flows into `guard`.
21-
*/
22-
predicate flowsToGuardExpr(DataFlow::Node nd, SensitiveActionGuardConditional guard) {
23-
nd = guard or
24-
flowsToGuardExpr(nd.getASuccessor(), guard)
25-
}
26-
27-
/**
28-
* A comparison that guards a sensitive action, e.g. the comparison in:
29-
* `var ok = x == y; if (ok) login()`.
30-
*/
31-
class SensitiveActionGuardComparison extends Comparison {
32-
SensitiveActionGuardConditional guard;
33-
34-
SensitiveActionGuardComparison() { flowsToGuardExpr(DataFlow::valueNode(this), guard) }
35-
36-
/**
37-
* Gets the guard that uses this comparison.
38-
*/
39-
SensitiveActionGuardConditional getGuard() { result = guard }
40-
}
41-
42-
/**
43-
* An intermediary sink to enable reuse of the taint configuration.
44-
* This sink should not be presented to the client of this query.
45-
*/
46-
class SensitiveActionGuardComparisonOperand extends Sink {
47-
SensitiveActionGuardComparison comparison;
48-
49-
SensitiveActionGuardComparisonOperand() { asExpr() = comparison.getAnOperand() }
50-
51-
override SensitiveAction getAction() { result = comparison.getGuard().getAction() }
52-
}
53-
54-
/**
55-
* Holds if `sink` guards `action`, and `source` taints `sink`.
56-
*
57-
* If flow from `source` taints `sink`, then an attacker can
58-
* control if `action` should be executed or not.
59-
*/
60-
predicate isTaintedGuardForSensitiveAction(
61-
DataFlow::PathNode sink, DataFlow::PathNode source, SensitiveAction action
62-
) {
63-
action = sink.getNode().(Sink).getAction() and
64-
// exclude the intermediary sink
65-
not sink.getNode() instanceof SensitiveActionGuardComparisonOperand and
66-
exists(Configuration cfg |
67-
// ordinary taint tracking to a guard
68-
cfg.hasFlowPath(source, sink)
69-
or
70-
// taint tracking to both operands of a guard comparison
71-
exists(
72-
SensitiveActionGuardComparison cmp, DataFlow::PathNode lSource, DataFlow::PathNode rSource,
73-
DataFlow::PathNode lSink, DataFlow::PathNode rSink
74-
|
75-
sink.getNode() = cmp.getGuard() and
76-
cfg.hasFlowPath(lSource, lSink) and
77-
lSink.getNode() = DataFlow::valueNode(cmp.getLeftOperand()) and
78-
cfg.hasFlowPath(rSource, rSink) and
79-
rSink.getNode() = DataFlow::valueNode(cmp.getRightOperand())
80-
|
81-
source = lSource or
82-
source = rSource
83-
)
84-
)
85-
}
86-
87-
/**
88-
* Holds if `e` effectively guards access to `action` by returning or throwing early.
89-
*
90-
* Example: `if (e) return; action(x)`.
91-
*/
92-
predicate isEarlyAbortGuard(DataFlow::PathNode e, SensitiveAction action) {
93-
exists(IfStmt guard |
94-
// `e` is in the condition of an if-statement ...
95-
e.getNode().(Sink).asExpr().getParentExpr*() = guard.getCondition() and
96-
// ... where the then-branch always throws or returns
97-
exists(Stmt abort |
98-
abort instanceof ThrowStmt or
99-
abort instanceof ReturnStmt
100-
|
101-
abort.nestedIn(guard) and
102-
abort.getBasicBlock().(ReachableBasicBlock).postDominates(guard.getThen().getBasicBlock())
103-
) and
104-
// ... and the else-branch does not exist
105-
not exists(guard.getElse())
106-
|
107-
// ... and `action` is outside the if-statement
108-
not action.asExpr().getEnclosingStmt().nestedIn(guard)
109-
)
110-
}
111-
11218
import semmle.javascript.heuristics.AdditionalSources
11319

11420
from DataFlow::PathNode source, DataFlow::PathNode sink, SensitiveAction action

0 commit comments

Comments
 (0)