10000 JS: Add experimental variants of common security queries with more sources by erik-krogh · Pull Request #11582 · github/codeql · GitHub
[go: up one dir, main page]
Skip to content

JS: Add experimental variants of common security queries with more sources #11582

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 4, 2023
Merged

JS: Add experimental variants of common security queries with more sources #11582

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f84b21
QL: add getQueryName to QueryDoc
erik-krogh Dec 7, 2022
442749b
JS: add heuristic variants of queries that use RemoteFlowSource
erik-krogh Dec 19, 2022
66be8cd
remove more of the implementation into ConditionalBypassQuery.qll
erik-krogh Dec 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
remove more of the implementation into ConditionalBypassQuery.qll
  • Loading branch information
@erik-krogh
erik-krogh committed Dec 19, 2022
commit 66be8cda0650ac706bb3469b4d88e2c173a32ec0
93 changes: 93 additions & 0 deletions javascript/ql/lib/semmle/javascript/security/dataflow/ConditionalBypassQuery.qll
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,96 @@ class Configuration extends TaintTracking::Configuration {
dst.asExpr().(Comparison).hasOperands(src.asExpr(), any(ConstantExpr c))
}
}

/**
* Holds if the value of `nd` flows into `guard`.
*/
predicate flowsToGuardExpr(DataFlow::Node nd, SensitiveActionGuardConditional guard) {
nd = guard or
flowsToGuardExpr(nd.getASuccessor(), guard)
}

/**
* A comparison that guards a sensitive action, e.g. the comparison in:
* `var ok = x == y; if (ok) login()`.
*/
class SensitiveActionGuardComparison extends Comparison {
SensitiveActionGuardConditional guard;

SensitiveActionGuardComparison() { flowsToGuardExpr(DataFlow::valueNode(this), guard) }

/**
* Gets the guard that uses this comparison.
*/
SensitiveActionGuardConditional getGuard() { result = guard }
}

/**
* An intermediary sink to enable reuse of the taint configuration.
* This sink should not be presented to the client of this query.
*/
class SensitiveActionGuardComparisonOperand extends Sink {
SensitiveActionGuardComparison comparison;

SensitiveActionGuardComparisonOperand() { asExpr() = comparison.getAnOperand() }

override SensitiveAction getAction() { result = comparison.getGuard().getAction() }
}

/**
* 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.
*/
predicate isTaintedGuardForSensitiveAction(
DataFlow::PathNode sink, DataFlow::PathNode source, SensitiveAction action
) {
action = sink.getNode().(Sink).getAction() and
// exclude the intermediary sink
not sink.getNode() instanceof SensitiveActionGuardComparisonOperand and
exists(Configuration cfg |
// ordinary taint tracking to a guard
cfg.hasFlowPath(source, sink)
or
// taint tracking to both operands of a guard comparison
exists(
SensitiveActionGuardComparison cmp, DataFlow::PathNode lSource, DataFlow::PathNode rSource,
DataFlow::PathNode lSink, DataFlow::PathNode rSink
|
sink.getNode() = cmp.getGuard() and
cfg.hasFlowPath(lSource, lSink) and
lSink.getNode() = DataFlow::valueNode(cmp.getLeftOperand()) and
cfg.hasFlowPath(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 isEarlyAbortGuard(DataFlow::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)
)
}
93 changes: 0 additions & 93 deletions javascript/ql/src/Security/CWE-807/ConditionalBypass.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,99 +15,6 @@ import javascript
import semmle.javascript.security.dataflow.ConditionalBypassQuery
import DataFlow::PathGraph

/**
* Holds if the value of `nd` flows into `guard`.
*/
predicate flowsToGuardExpr(DataFlow::Node nd, SensitiveActionGuardConditional guard) {
nd = guard or
flowsToGuardExpr(nd.getASuccessor(), guard)
}

/**
* A comparison that guards a sensitive action, e.g. the comparison in:
* `var ok = x == y; if (ok) login()`.
*/
class SensitiveActionGuardComparison extends Comparison {
SensitiveActionGuardConditional guard;

SensitiveActionGuardComparison() { flowsToGuardExpr(DataFlow::valueNode(this), guard) }

/**
* Gets the guard that uses this comparison.
*/
SensitiveActionGuardConditional getGuard() { result = guard }
}

/**
* An intermediary sink to enable reuse of the taint configuration.
* This sink should not be presented to the client of this query.
*/
class SensitiveActionGuardComparisonOperand extends Sink {
SensitiveActionGuardComparison comparison;

SensitiveActionGuardComparisonOperand() { asExpr() = comparison.getAnOperand() }

override SensitiveAction getAction() { result = comparison.getGuard().getAction() }
}

/**
* 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.
*/
predicate isTaintedGuardForSensitiveAction(
DataFlow::PathNode sink, DataFlow::PathNode source, SensitiveAction action
) {
action = sink.getNode().(Sink).getAction() and
// exclude the intermediary sink
not sink.getNode() instanceof SensitiveActionGuardComparisonOperand and
exists(Configuration cfg |
// ordinary taint tracking to a guard
cfg.hasFlowPath(source, sink)
or
// taint tracking to both operands of a guard comparison
exists(
SensitiveActionGuardComparison cmp, DataFlow::PathNode lSource, DataFlow::PathNode rSource,
DataFlow::PathNode lSink, DataFlow::PathNode rSink
|
sink.getNode() = cmp.getGuard() and
cfg.hasFlowPath(lSource, lSink) and
lSink.getNode() = DataFlow::valueNode(cmp.getLeftOperand()) and
cfg.hasFlowPath(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 isEarlyAbortGuard(DataFlow::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)
)
}

from DataFlow::PathNode source, DataFlow::PathNode sink, SensitiveAction action
where
isTaintedGuardForSensitiveAction(sink, source, action) and
Expand Down
94 changes: 0 additions & 94 deletions javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-807/ConditionalBypass.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,100 +15,6 @@
import javascript
import semmle.javascript.security.dataflow.ConditionalBypassQuery
import DataFlow::PathGraph

/**
* Holds if the value of `nd` flows into `guard`.
*/
predicate flowsToGuardExpr(DataFlow::Node nd, SensitiveActionGuardConditional guard) {
nd = guard or
flowsToGuardExpr(nd.getASuccessor(), guard)
}

/**
* A comparison that guards a sensitive action, e.g. the comparison in:
* `var ok = x == y; if (ok) login()`.
*/
class SensitiveActionGuardComparison extends Comparison {
SensitiveActionGuardConditional guard;

SensitiveActionGuardComparison() { flowsToGuardExpr(DataFlow::valueNode(this), guard) }

/**
* Gets the guard that uses this comparison.
*/
SensitiveActionGuardConditional getGuard() { result = guard }
}

/**
* An intermediary sink to enable reuse of the taint configuration.
* This sink should not be presented to the client of th 6D40 is query.
*/
class SensitiveActionGuardComparisonOperand extends Sink {
SensitiveActionGuardComparison comparison;

SensitiveActionGuardComparisonOperand() { asExpr() = comparison.getAnOperand() }

override SensitiveAction getAction() { result = comparison.getGuard().getAction() }
}

/**
* 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.
*/
predicate isTaintedGuardForSensitiveAction(
DataFlow::PathNode sink, DataFlow::PathNode source, SensitiveAction action
) {
action = sink.getNode().(Sink).getAction() and
// exclude the intermediary sink
not sink.getNode() instanceof SensitiveActionGuardComparisonOperand and
exists(Configuration cfg |
// ordinary taint tracking to a guard
cfg.hasFlowPath(source, sink)
or
// taint tracking to both operands of a guard comparison
exists(
SensitiveActionGuardComparison cmp, DataFlow::PathNode lSource, DataFlow::PathNode rSource,
DataFlow::PathNode lSink, DataFlow::PathNode rSink
|
sink.getNode() = cmp.getGuard() and
cfg.hasFlowPath(lSource, lSink) and
lSink.getNode() = DataFlow::valueNode(cmp.getLeftOperand()) and
cfg.hasFlowPath(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 isEarlyAbortGuard(DataFlow::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)
)
}

import semmle.javascript.heuristics.AdditionalSources

from DataFlow::PathNode source, DataFlow::PathNode sink, SensitiveAction action
Expand Down
0