hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-22 11:46:32 +01:00
Code Issues Packages Projects Releases Wiki Activity

JS: Port ConditionalBypass

Browse Source
This commit is contained in:
Asger F
2023-10-05 09:13:00 +02:00
parent 2296a273c4
commit f14303acea
3 changed files with 130 additions and 114 deletions
Download Patch File Download Diff File Expand all files Collapse all files

90
javascript/ql/lib/semmle/javascript/security/dataflow/ConditionalBypassQuery.qll
View File

@@ -13,7 +13,28 @@ import ConditionalBypassCustomizations::ConditionalBypass
/** /**
* A taint tracking configuration for bypass of sensitive action guards. * A taint tracking configuration for bypass of sensitive action guards.
*/ */
class Configuration extends TaintTracking::Configuration { module ConditionalBypassConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof Source }
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node dst) {
// comparing a tainted expression against a constant gives a tainted result
dst.asExpr().(Comparison).hasOperands(src.asExpr(), any(ConstantExpr c))
}
}
/**
* Taint tracking flow for bypass of sensitive action guards.
*/
module ConditionalBypassFlow = TaintTracking::Global<ConditionalBypassConfig>;
/**
* DEPRECATED. Use the `ConditionalBypassFlow` module instead.
*/
deprecated class Configuration extends TaintTracking::Configuration {
Configuration() { this = "ConditionalBypass" } Configuration() { this = "ConditionalBypass" }
override predicate isSource(DataFlow::Node source) { source instanceof Source } override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -26,8 +47,7 @@ class Configuration extends TaintTracking::Configuration {
} }
override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node dst) { override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node dst) {
// comparing a tainted expression against a constant gives a tainted result ConditionalBypassConfig::isAdditionalFlowStep(src, dst)
dst.asExpr().(Comparison).hasOperands(src.asExpr(), any(ConstantExpr c))
} }
} }
@@ -72,7 +92,67 @@ class SensitiveActionGuardComparisonOperand extends Sink {
* If flow from `source` taints `sink`, then an attacker can * If flow from `source` taints `sink`, then an attacker can
* control if `action` should be executed or not. * control if `action` should be executed or not.
*/ */
predicate isTaintedGuardForSensitiveAction( predicate isTaintedGuardNodeForSensitiveAction(
ConditionalBypassFlow::PathNode sink, ConditionalBypassFlow::PathNode source,
SensitiveAction action
) {
action = sink.getNode().(Sink).getAction() and
// exclude the intermediary sink
not sink.getNode() instanceof SensitiveActionGuardComparisonOperand and
(
// ordinary taint tracking to a guard
ConditionalBypassFlow::flowPath(source, sink)
or
// taint tracking to both operands of a guard comparison
exists(
SensitiveActionGuardComparison cmp, ConditionalBypassFlow::PathNode lSource,
ConditionalBypassFlow::PathNode rSource, ConditionalBypassFlow::PathNode lSink,
ConditionalBypassFlow::PathNode rSink
|
sink.getNode() = cmp.getGuard() and
ConditionalBypassFlow::flowPath(lSource, lSink) and
lSink.getNode() = DataFlow::valueNode(cmp.getLeftOperand()) and
ConditionalBypassFlow::flowPath(rSource, rSink) and
rSink.getNode() = DataFlow::valueNode(cmp.getRightOperand())
|
source = lSource or
source = rSource
)
)
}
/**
* Holds if `e` effectively guards access to `action` by returning or throwing early.
*
* Example: `if (e) return; action(x)`.
*/
predicate isEarlyAbortGuardNode(ConditionalBypassFlow::PathNode e, SensitiveAction action) {
exists(IfStmt guard |
// `e` is in the condition of an if-statement ...
e.getNode().(Sink).asExpr().getParentExpr*() = guard.getCondition() and
// ... where the then-branch always throws or returns
exists(Stmt abort |
abort instanceof ThrowStmt or
abort instanceof ReturnStmt
|
abort.nestedIn(guard) and
abort.getBasicBlock().(ReachableBasicBlock).postDominates(guard.getThen().getBasicBlock())
) and
// ... and the else-branch does not exist
not exists(guard.getElse())
|
// ... and `action` is outside the if-statement
not action.asExpr().getEnclosingStmt().nestedIn(guard)
)
}
/**
* Holds if `sink` guards `action`, and `source` taints `sink`.
*
* If flow from `source` taints `sink`, then an attacker can
* control if `action` should be executed or not.
*/
deprecated predicate isTaintedGuardForSensitiveAction(
DataFlow::PathNode sink, DataFlow::PathNode source, SensitiveAction action DataFlow::PathNode sink, DataFlow::PathNode source, SensitiveAction action
) { ) {
action = sink.getNode().(Sink).getAction() and action = sink.getNode().(Sink).getAction() and
@@ -104,7 +184,7 @@ predicate isTaintedGuardForSensitiveAction(
* *
* Example: `if (e) return; action(x)`. * Example: `if (e) return; action(x)`.
*/ */
predicate isEarlyAbortGuard(DataFlow::PathNode e, SensitiveAction action) { deprecated predicate isEarlyAbortGuard(DataFlow::PathNode e, SensitiveAction action) {
exists(IfStmt guard | exists(IfStmt guard |
// `e` is in the condition of an if-statement ... // `e` is in the condition of an if-statement ...
e.getNode().(Sink).asExpr().getParentExpr*() = guard.getCondition() and e.getNode().(Sink).asExpr().getParentExpr*() = guard.getCondition() and

10
javascript/ql/src/Security/CWE-807/ConditionalBypass.ql
View File

@@ -13,11 +13,13 @@
import javascript import javascript
import semmle.javascript.security.dataflow.ConditionalBypassQuery import semmle.javascript.security.dataflow.ConditionalBypassQuery
import DataFlow::PathGraph import ConditionalBypassFlow::PathGraph
from DataFlow::PathNode source, DataFlow::PathNode sink, SensitiveAction action from
ConditionalBypassFlow::PathNode source, ConditionalBypassFlow::PathNode sink,
SensitiveAction action
where where
isTaintedGuardForSensitiveAction(sink, source, action) and isTaintedGuardNodeForSensitiveAction(sink, source, action) and
not isEarlyAbortGuard(sink, action) not isEarlyAbortGuardNode(sink, action)
select sink.getNode(), source, sink, "This condition guards a sensitive $@, but a $@ controls it.", select sink.getNode(), source, sink, "This condition guards a sensitive $@, but a $@ controls it.",
action, "action", source.getNode(), "user-provided value" action, "action", source.getNode(), "user-provided value"

144
javascript/ql/test/query-tests/Security/CWE-807/ConditionalBypass.expected
View File

@@ -1,122 +1,56 @@
nodes
| example_bypass.js:6:9:6:19 | req.cookies |
| example_bypass.js:6:9:6:19 | req.cookies |
| example_bypass.js:6:9:6:34 | req.coo ... nUserId |
| example_bypass.js:6:9:6:34 | req.coo ... nUserId |
| example_bypass.js:6:40:6:56 | req.params.userId |
| example_bypass.js:6:40:6:56 | req.params.userId |
| example_bypass.js:6:40:6:56 | req.params.userId |
| example_bypass.js:17:46:17:62 | req.params.userId |
| example_bypass.js:17:46:17:62 | req.params.userId |
| example_bypass.js:17:46:17:62 | req.params.userId |
| tst.js:9:8:9:26 | req.params.shutDown |
| tst.js:9:8:9:26 | req.params.shutDown |
| tst.js:9:8:9:26 | req.params.shutDown |
| tst.js:13:9:13:19 | req.cookies |
| tst.js:13:9:13:19 | req.cookies |
| tst.js:13:9:13:30 | req.coo ... inThing |
| tst.js:13:9:13:30 | req.coo ... inThing |
| tst.js:27:9:27:37 | v3 |
| tst.js:27:14:27:37 | id(req. ... okieId) |
| tst.js:27:17:27:27 | req.cookies |
| tst.js:27:17:27:27 | req.cookies |
| tst.js:27:17:27:36 | req.cookies.cookieId |
| tst.js:28:9:28:10 | v3 |
| tst.js:28:9:28:10 | v3 |
| tst.js:33:13:33:23 | req.cookies |
| tst.js:33:13:33:23 | req.cookies |
| tst.js:33:13:33:32 | req.cookies.cookieId |
| tst.js:33:13:33:32 | req.cookies.cookieId |
| tst.js:38:9:38:19 | req.cookies |
| tst.js:38:9:38:19 | req.cookies |
| tst.js:38:9:38:28 | req.cookies.cookieId |
| tst.js:38:9:38:28 | req.cookies.cookieId |
| tst.js:44:8:44:23 | req.params.login |
| tst.js:44:8:44:23 | req.params.login |
| tst.js:44:8:44:23 | req.params.login |
| tst.js:57:8:57:23 | req.params.login |
| tst.js:57:8:57:23 | req.params.login |
| tst.js:57:8:57:23 | req.params.login |
| tst.js:61:9:61:19 | req.cookies |
| tst.js:61:9:61:19 | req.cookies |
| tst.js:61:9:61:28 | req.cookies.cookieId |
| tst.js:61:9:61:28 | req.cookies.cookieId |
| tst.js:61:34:61:53 | req.params.requestId |
| tst.js:61:34:61:53 | req.params.requestId |
| tst.js:61:34:61:53 | req.params.requestId |
| tst.js:65:14:65:24 | req.cookies |
| tst.js:65:14:65:24 | req.cookies |
| tst.js:65:14:65:33 | req.cookies.cookieId |
| tst.js:65:14:65:33 | req.cookies.cookieId |
| tst.js:65:39:65:58 | req.params.requestId |
| tst.js:65:39:65:58 | req.params.requestId |
| tst.js:65:39:65:58 | req.params.requestId |
| tst.js:78:9:78:19 | req.cookies |
| tst.js:78:9:78:19 | req.cookies |
| tst.js:78:9:78:28 | req.cookies.cookieId |
| tst.js:78:9:78:28 | req.cookies.cookieId |
| tst.js:78:9:78:41 | req.coo ... secret" |
| tst.js:78:9:78:41 | req.coo ... secret" |
| tst.js:91:10:91:17 | req.body |
| tst.js:91:10:91:17 | req.body |
| tst.js:91:10:91:17 | req.body |
| tst.js:98:13:98:32 | req.query.vulnerable |
| tst.js:98:13:98:32 | req.query.vulnerable |
| tst.js:98:13:98:32 | req.query.vulnerable |
| tst.js:105:13:105:32 | req.query.vulnerable |
| tst.js:105:13:105:32 | req.query.vulnerable |
| tst.js:105:13:105:32 | req.query.vulnerable |
| tst.js:113:13:113:32 | req.query.vulnerable |
| tst.js:113:13:113:32 | req.query.vulnerable |
| tst.js:113:13:113:32 | req.query.vulnerable |
edges edges
| example_bypass.js:6:9:6:19 | req.cookies | example_bypass.js:6:9:6:34 | req.coo ... nUserId | | example_bypass.js:6:9:6:19 | req.cookies | example_bypass.js:6:9:6:34 | req.coo ... nUserId |
| example_bypass.js:6:9:6:19 | req.cookies | example_bypass.js:6:9:6:34 | req.coo ... nUserId |
| example_bypass.js:6:9:6:19 | req.cookies | example_bypass.js:6:9:6:34 | req.coo ... nUserId |
| example_bypass.js:6:9:6:19 | req.cookies | example_bypass.js:6:9:6:34 | req.coo ... nUserId |
| example_bypass.js:6:40:6:56 | req.params.userId | example_bypass.js:6:40:6:56 | req.params.userId |
| example_bypass.js:17:46:17:62 | req.params.userId | example_bypass.js:17:46:17:62 | req.params.userId |
| tst.js:9:8:9:26 | req.params.shutDown | tst.js:9:8:9:26 | req.params.shutDown |
| tst.js:13:9:13:19 | req.cookies | tst.js:13:9:13:30 | req.coo ... inThing | | tst.js:13:9:13:19 | req.cookies | tst.js:13:9:13:30 | req.coo ... inThing |
| tst.js:13:9:13:19 | req.cookies | tst.js:13:9:13:30 | req.coo ... inThing | | tst.js:24:17:24:17 | v | tst.js:25:16:25:16 | v |
| tst.js:13:9:13:19 | req.cookies | tst.js:13:9:13:30 | req.coo ... inThing |
| tst.js:13:9:13:19 | req.cookies | tst.js:13:9:13:30 | req.coo ... inThing |
| tst.js:27:9:27:37 | v3 | tst.js:28:9:28:10 | v3 |
| tst.js:27:9:27:37 | v3 | tst.js:28:9:28:10 | v3 | | tst.js:27:9:27:37 | v3 | tst.js:28:9:28:10 | v3 |
| tst.js:27:14:27:37 | id(req. ... okieId) | tst.js:27:9:27:37 | v3 | | tst.js:27:14:27:37 | id(req. ... okieId) | tst.js:27:9:27:37 | v3 |
| tst.js:27:17:27:27 | req.cookies | tst.js:27:17:27:36 | req.cookies.cookieId | | tst.js:27:17:27:27 | req.cookies | tst.js:27:17:27:36 | req.cookies.cookieId |
| tst.js:27:17:27:27 | req.cookies | tst.js:27:17:27:36 | req.cookies.cookieId | | tst.js:27:17:27:36 | req.cookies.cookieId | tst.js:24:17:24:17 | v |
| tst.js:27:17:27:36 | req.cookies.cookieId | tst.js:27:14:27:37 | id(req. ... okieId) | | tst.js:27:17:27:36 | req.cookies.cookieId | tst.js:27:14:27:37 | id(req. ... okieId) |
| tst.js:33:13:33:23 | req.cookies | tst.js:33:13:33:32 | req.cookies.cookieId | | tst.js:33:13:33:23 | req.cookies | tst.js:33:13:33:32 | req.cookies.cookieId |
| tst.js:33:13:33:23 | req.cookies | tst.js:33:13:33:32 | req.cookies.cookieId |
| tst.js:33:13:33:23 | req.cookies | tst.js:33:13:33:32 | req.cookies.cookieId |
| tst.js:33:13:33:23 | req.cookies | tst.js:33:13:33:32 | req.cookies.cookieId |
| tst.js:38:9:38:19 | req.cookies | tst.js:38:9:38:28 | req.cookies.cookieId | | tst.js:38:9:38:19 | req.cookies | tst.js:38:9:38:28 | req.cookies.cookieId |
| tst.js:38:9:38:19 | req.cookies | tst.js:38:9:38:28 | req.cookies.cookieId |
| tst.js:38:9:38:19 | req.cookies | tst.js:38:9:38:28 | req.cookies.cookieId |
| tst.js:38:9:38:19 | req.cookies | tst.js:38:9:38:28 | req.cookies.cookieId |
| tst.js:44:8:44:23 | req.params.login | tst.js:44:8:44:23 | req.params.login |
| tst.js:57:8:57:23 | req.params.login | tst.js:57:8:57:23 | req.params.login |
| tst.js:61:9:61:19 | req.cookies | tst.js:61:9:61:28 | req.cookies.cookieId | | tst.js:61:9:61:19 | req.cookies | tst.js:61:9:61:28 | req.cookies.cookieId |
| tst.js:61:9:61:19 | req.cookies | tst.js:61:9:61:28 | req.cookies.cookieId |
| tst.js:61:9:61:19 | req.cookies | tst.js:61:9:61:28 | req.cookies.cookieId |
| tst.js:61:9:61:19 | req.cookies | tst.js:61:9:61:28 | req.cookies.cookieId |
| tst.js:61:34:61:53 | req.params.requestId | tst.js:61:34:61:53 | req.params.requestId |
| tst.js:65:14:65:24 | req.cookies | tst.js:65:14:65:33 | req.cookies.cookieId | | tst.js:65:14:65:24 | req.cookies | tst.js:65:14:65:33 | req.cookies.cookieId |
| tst.js:65:14:65:24 | req.cookies | tst.js:65:14:65:33 | req.cookies.cookieId |
| tst.js:65:14:65:24 | req.cookies | tst.js:65:14:65:33 | req.cookies.cookieId |
| tst.js:65:14:65:24 | req.cookies | tst.js:65:14:65:33 | req.cookies.cookieId |
| tst.js:65:39:65:58 | req.params.requestId | tst.js:65:39:65:58 | req.params.requestId |
| tst.js:78:9:78:19 | req.cookies | tst.js:78:9:78:28 | req.cookies.cookieId |
| tst.js:78:9:78:19 | req.cookies | tst.js:78:9:78:28 | req.cookies.cookieId |
| tst.js:78:9:78:19 | req.cookies | tst.js:78:9:78:28 | req.cookies.cookieId | | tst.js:78:9:78:19 | req.cookies | tst.js:78:9:78:28 | req.cookies.cookieId |
| tst.js:78:9:78:19 | req.cookies | tst.js:78:9:78:28 | req.cookies.cookieId | | tst.js:78:9:78:19 | req.cookies | tst.js:78:9:78:28 | req.cookies.cookieId |
| tst.js:78:9:78:28 | req.cookies.cookieId | tst.js:78:9:78:41 | req.coo ... secret" | | tst.js:78:9:78:28 | req.cookies.cookieId | tst.js:78:9:78:41 | req.coo ... secret" |
| tst.js:78:9:78:28 | req.cookies.cookieId | tst.js:78:9:78:41 | req.coo ... secret" | nodes
| tst.js:91:10:91:17 | req.body | tst.js:91:10:91:17 | req.body | | example_bypass.js:6:9:6:19 | req.cookies | semmle.label | req.cookies |
| tst.js:98:13:98:32 | req.query.vulnerable | tst.js:98:13:98:32 | req.query.vulnerable | | example_bypass.js:6:9:6:34 | req.coo ... nUserId | semmle.label | req.coo ... nUserId |
| tst.js:105:13:105:32 | req.query.vulnerable | tst.js:105:13:105:32 | req.query.vulnerable | | example_bypass.js:6:40:6:56 | req.params.userId | semmle.label | req.params.userId |
| tst.js:113:13:113:32 | req.query.vulnerable | tst.js:113:13:113:32 | req.query.vulnerable | | example_bypass.js:17:46:17:62 | req.params.userId | semmle.label | req.params.userId |
| tst.js:9:8:9:26 | req.params.shutDown | semmle.label | req.params.shutDown |
| tst.js:13:9:13:19 | req.cookies | semmle.label | req.cookies |
| tst.js:13:9:13:30 | req.coo ... inThing | semmle.label | req.coo ... inThing |
| tst.js:24:17:24:17 | v | semmle.label | v |
| tst.js:25:16:25:16 | v | semmle.label | v |
| tst.js:27:9:27:37 | v3 | semmle.label | v3 |
| tst.js:27:14:27:37 | id(req. ... okieId) | semmle.label | id(req. ... okieId) |
| tst.js:27:17:27:27 | req.cookies | semmle.label | req.cookies |
| tst.js:27:17:27:36 | req.cookies.cookieId | semmle.label | req.cookies.cookieId |
| tst.js:28:9:28:10 | v3 | semmle.label | v3 |
| tst.js:33:13:33:23 | req.cookies | semmle.label | req.cookies |
| tst.js:33:13:33:32 | req.cookies.cookieId | semmle.label | req.cookies.cookieId |
| tst.js:38:9:38:19 | req.cookies | semmle.label | req.cookies |
| tst.js:38:9:38:28 | req.cookies.cookieId | semmle.label | req.cookies.cookieId |
| tst.js:44:8:44:23 | req.params.login | semmle.label | req.params.login |
| tst.js:57:8:57:23 | req.params.login | semmle.label | req.params.login |
| tst.js:61:9:61:19 | req.cookies | semmle.label | req.cookies |
| tst.js:61:9:61:28 | req.cookies.cookieId | semmle.label | req.cookies.cookieId |
| tst.js:61:34:61:53 | req.params.requestId | semmle.label | req.params.requestId |
| tst.js:65:14:65:24 | req.cookies | semmle.label | req.cookies |
| tst.js:65:14:65:33 | req.cookies.cookieId | semmle.label | req.cookies.cookieId |
| tst.js:65:39:65:58 | req.params.requestId | semmle.label | req.params.requestId |
| tst.js:78:9:78:19 | req.cookies | semmle.label | req.cookies |
| tst.js:78:9:78:28 | req.cookies.cookieId | semmle.label | req.cookies.cookieId |
| tst.js:78:9:78:28 | req.cookies.cookieId | semmle.label | req.cookies.cookieId |
| tst.js:78:9:78:41 | req.coo ... secret" | semmle.label | req.coo ... secret" |
| tst.js:91:10:91:17 | req.body | semmle.label | req.body |
| tst.js:98:13:98:32 | req.query.vulnerable | semmle.label | req.query.vulnerable |
| tst.js:105:13:105:32 | req.query.vulnerable | semmle.label | req.query.vulnerable |
| tst.js:113:13:113:32 | req.query.vulnerable | semmle.label | req.query.vulnerable |
subpaths
| tst.js:27:17:27:36 | req.cookies.cookieId | tst.js:24:17:24:17 | v | tst.js:25:16:25:16 | v | tst.js:27:14:27:37 | id(req. ... okieId) |
#select #select
| tst.js:9:8:9:26 | req.params.shutDown | tst.js:9:8:9:26 | req.params.shutDown | tst.js:9:8:9:26 | req.params.shutDown | This condition guards a sensitive $@, but a $@ controls it. | tst.js:10:9:10:22 | process.exit() | action | tst.js:9:8:9:26 | req.params.shutDown | user-provided value | | tst.js:9:8:9:26 | req.params.shutDown | tst.js:9:8:9:26 | req.params.shutDown | tst.js:9:8:9:26 | req.params.shutDown | This condition guards a sensitive $@, but a $@ controls it. | tst.js:10:9:10:22 | process.exit() | action | tst.js:9:8:9:26 | req.params.shutDown | user-provided value |
| tst.js:13:9:13:30 | req.coo ... inThing | tst.js:13:9:13:19 | req.cookies | tst.js:13:9:13:30 | req.coo ... inThing | This condition guards a sensitive $@, but a $@ controls it. | tst.js:14:9:14:17 | o.login() | action | tst.js:13:9:13:19 | req.cookies | user-provided value | | tst.js:13:9:13:30 | req.coo ... inThing | tst.js:13:9:13:19 | req.cookies | tst.js:13:9:13:30 | req.coo ... inThing | This condition guards a sensitive $@, but a $@ controls it. | tst.js:14:9:14:17 | o.login() | action | tst.js:13:9:13:19 | req.cookies | user-provided value |