Merge branch 'main' into rb/dataflow-query-refactor

This commit is contained in:
Alex Ford
2023-09-07 14:57:38 +01:00
committed by GitHub
930 changed files with 20415 additions and 13248 deletions

View File

@@ -1,3 +1,10 @@
## 0.7.3
### Minor Analysis Improvements
* Flow between positional arguments and splat parameters (`*args`) is now tracked more precisely.
* Flow between splat arguments (`*args`) and positional parameters is now tracked more precisely.
## 0.7.2
No user-facing changes.

View File

@@ -1,4 +0,0 @@
---
category: minorAnalysis
---
* Flow between splat arguments (`*args`) and positional parameters is now tracked more precisely.

View File

@@ -1,4 +0,0 @@
---
category: minorAnalysis
---
* Flow between positional arguments and splat parameters (`*args`) is now tracked more precisely.

View File

@@ -0,0 +1,6 @@
## 0.7.3
### Minor Analysis Improvements
* Flow between positional arguments and splat parameters (`*args`) is now tracked more precisely.
* Flow between splat arguments (`*args`) and positional parameters is now tracked more precisely.

View File

@@ -1,2 +1,2 @@
---
lastReleaseVersion: 0.7.2
lastReleaseVersion: 0.7.3

View File

@@ -1250,3 +1250,40 @@ module LdapExecution {
abstract DataFlow::Node getQuery();
}
}
/**
* A data-flow node that collects methods binding a LDAP connection.
*
* Extend this class to refine existing API models. If you want to model new APIs,
* extend `LdapBind::Range` instead.
*/
class LdapBind extends DataFlow::Node instanceof LdapBind::Range {
/** Gets the argument containing the binding host */
DataFlow::Node getHost() { result = super.getHost() }
/** Gets the argument containing the binding expression. */
DataFlow::Node getPassword() { result = super.getPassword() }
/** Holds if the binding process use SSL. */
predicate usesSsl() { super.usesSsl() }
}
/** Provides classes for modeling LDAP bind-related APIs. */
module LdapBind {
/**
* A data-flow node that collects methods binding a LDAP connection.
*
* Extend this class to model new APIs. If you want to refine existing API models,
* extend `LdapBind` instead.
*/
abstract class Range extends DataFlow::Node {
/** Gets the argument containing the binding host. */
abstract DataFlow::Node getHost();
/** Gets the argument containing the binding expression. */
abstract DataFlow::Node getPassword();
/** Holds if the binding process use SSL. */
abstract predicate usesSsl();
}
}

View File

@@ -73,16 +73,14 @@ module SuccessorTypes {
}
/**
* A conditional control flow successor. Either a Boolean successor (`BooleanSuccessor`),
* an emptiness successor (`EmptinessSuccessor`), or a matching successor
* (`MatchingSuccessor`)
* A conditional control flow successor. Either a Boolean successor (`BooleanSuccessor`)
* or a matching successor (`MatchingSuccessor`)
*/
class ConditionalSuccessor extends SuccessorType {
boolean value;
ConditionalSuccessor() {
this = CfgImpl::TBooleanSuccessor(value) or
this = CfgImpl::TEmptinessSuccessor(value) or
this = CfgImpl::TMatchingSuccessor(value)
}
@@ -109,41 +107,6 @@ module SuccessorTypes {
*/
class BooleanSuccessor extends ConditionalSuccessor, CfgImpl::TBooleanSuccessor { }
/**
* An emptiness control flow successor.
*
* For example, this program fragment:
*
* ```rb
* for arg in args do
* puts arg
* end
* puts "done";
* ```
*
* has a control flow graph containing emptiness successors:
*
* ```
* args
* |
* for------<-----
* / \ \
* / \ |
* / \ |
* / \ |
* empty non-empty |
* | \ |
* puts "done" \ |
* arg |
* | |
* puts arg |
* \___/
* ```
*/
class EmptinessSuccessor extends ConditionalSuccessor, CfgImpl::TEmptinessSuccessor {
override string toString() { if value = true then result = "empty" else result = "non-empty" }
}
/**
* A matching control flow successor.
*

View File

@@ -1513,7 +1513,6 @@ private module Cached {
newtype TSuccessorType =
TSuccessorSuccessor() or
TBooleanSuccessor(boolean b) { b in [false, true] } or
TEmptinessSuccessor(boolean isEmpty) { isEmpty in [false, true] } or
TMatchingSuccessor(boolean isMatch) { isMatch in [false, true] } or
TReturnSuccessor() or
TBreakSuccessor() or

View File

@@ -1,299 +0,0 @@
/**
* Provides consistency queries for checking invariants in the language-specific
* data-flow classes and predicates.
*/
private import DataFlowImplSpecific::Private
private import DataFlowImplSpecific::Public
private import tainttracking1.TaintTrackingParameter::Private
private import tainttracking1.TaintTrackingParameter::Public
module Consistency {
private newtype TConsistencyConfiguration = MkConsistencyConfiguration()
/** A class for configuring the consistency queries. */
class ConsistencyConfiguration extends TConsistencyConfiguration {
string toString() { none() }
/** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
predicate uniqueEnclosingCallableExclude(Node n) { none() }
/** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */
predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() }
/** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
predicate uniqueNodeLocationExclude(Node n) { none() }
/** Holds if `n` should be excluded from the consistency test `missingLocation`. */
predicate missingLocationExclude(Node n) { none() }
/** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
predicate postWithInFlowExclude(Node n) { none() }
/** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
/** Holds if `n` should be excluded from the consistency test `reverseRead`. */
predicate reverseReadExclude(Node n) { none() }
/** Holds if `n` should be excluded from the consistency test `postHasUniquePre`. */
predicate postHasUniquePreExclude(PostUpdateNode n) { none() }
/** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */
predicate uniquePostUpdateExclude(Node n) { none() }
/** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */
predicate viableImplInCallContextTooLargeExclude(
DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
) {
none()
}
/** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodeAtPosition`. */
predicate uniqueParameterNodeAtPositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) {
none()
}
/** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodePosition`. */
predicate uniqueParameterNodePositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) {
none()
}
/** Holds if `n` should be excluded from the consistency test `identityLocalStep`. */
predicate identityLocalStepExclude(Node n) { none() }
}
private class RelevantNode extends Node {
RelevantNode() {
this instanceof ArgumentNode or
this instanceof ParameterNode or
this instanceof ReturnNode or
this = getAnOutNode(_, _) or
simpleLocalFlowStep(this, _) or
simpleLocalFlowStep(_, this) or
jumpStep(this, _) or
jumpStep(_, this) or
storeStep(this, _, _) or
storeStep(_, _, this) or
readStep(this, _, _) or
readStep(_, _, this) or
defaultAdditionalTaintStep(this, _) or
defaultAdditionalTaintStep(_, this)
}
}
query predicate uniqueEnclosingCallable(Node n, string msg) {
exists(int c |
n instanceof RelevantNode and
c = count(nodeGetEnclosingCallable(n)) and
c != 1 and
not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and
msg = "Node should have one enclosing callable but has " + c + "."
)
}
query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) {
exists(int c |
c = count(call.getEnclosingCallable()) and
c != 1 and
not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and
msg = "Call should have one enclosing callable but has " + c + "."
)
}
query predicate uniqueType(Node n, string msg) {
exists(int c |
n instanceof RelevantNode and
c = count(getNodeType(n)) and
c != 1 and
msg = "Node should have one type but has " + c + "."
)
}
query predicate uniqueNodeLocation(Node n, string msg) {
exists(int c |
c =
count(string filepath, int startline, int startcolumn, int endline, int endcolumn |
n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
) and
c != 1 and
not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and
msg = "Node should have one location but has " + c + "."
)
}
query predicate missingLocation(string msg) {
exists(int c |
c =
strictcount(Node n |
not n.hasLocationInfo(_, _, _, _, _) and
not any(ConsistencyConfiguration conf).missingLocationExclude(n)
) and
msg = "Nodes without location: " + c
)
}
query predicate uniqueNodeToString(Node n, string msg) {
exists(int c |
c = count(n.toString()) and
c != 1 and
msg = "Node should have one toString but has " + c + "."
)
}
query predicate missingToString(string msg) {
exists(int c |
c = strictcount(Node n | not exists(n.toString())) and
msg = "Nodes without toString: " + c
)
}
query predicate parameterCallable(ParameterNode p, string msg) {
exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and
msg = "Callable mismatch for parameter."
}
query predicate localFlowIsLocal(Node n1, Node n2, string msg) {
simpleLocalFlowStep(n1, n2) and
nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
msg = "Local flow step does not preserve enclosing callable."
}
query predicate readStepIsLocal(Node n1, Node n2, string msg) {
readStep(n1, _, n2) and
nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
msg = "Read step does not preserve enclosing callable."
}
query predicate storeStepIsLocal(Node n1, Node n2, string msg) {
storeStep(n1, _, n2) and
nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
msg = "Store step does not preserve enclosing callable."
}
private DataFlowType typeRepr() { result = getNodeType(_) }
query predicate compatibleTypesReflexive(DataFlowType t, string msg) {
t = typeRepr() and
not compatibleTypes(t, t) and
msg = "Type compatibility predicate is not reflexive."
}
query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) {
isUnreachableInCall(n, call) and
exists(DataFlowCallable c |
c = nodeGetEnclosingCallable(n) and
not viableCallable(call) = c
) and
msg = "Call context for isUnreachableInCall is inconsistent with call graph."
}
query predicate localCallNodes(DataFlowCall call, Node n, string msg) {
(
n = getAnOutNode(call, _) and
msg = "OutNode and call does not share enclosing callable."
or
n.(ArgumentNode).argumentOf(call, _) and
msg = "ArgumentNode and call does not share enclosing callable."
) and
nodeGetEnclosingCallable(n) != call.getEnclosingCallable()
}
// This predicate helps the compiler forget that in some languages
// it is impossible for a result of `getPreUpdateNode` to be an
// instance of `PostUpdateNode`.
private Node getPre(PostUpdateNode n) {
result = n.getPreUpdateNode()
or
none()
}
query predicate postIsNotPre(PostUpdateNode n, string msg) {
getPre(n) = n and
msg = "PostUpdateNode should not equal its pre-update node."
}
query predicate postHasUniquePre(PostUpdateNode n, string msg) {
not any(ConsistencyConfiguration conf).postHasUniquePreExclude(n) and
exists(int c |
c = count(n.getPreUpdateNode()) and
c != 1 and
msg = "PostUpdateNode should have one pre-update node but has " + c + "."
)
}
query predicate uniquePostUpdate(Node n, string msg) {
not any(ConsistencyConfiguration conf).uniquePostUpdateExclude(n) and
1 < strictcount(PostUpdateNode post | post.getPreUpdateNode() = n) and
msg = "Node has multiple PostUpdateNodes."
}
query predicate postIsInSameCallable(PostUpdateNode n, string msg) {
nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and
msg = "PostUpdateNode does not share callable with its pre-update node."
}
private predicate hasPost(Node n) { exists(PostUpdateNode post | post.getPreUpdateNode() = n) }
query predicate reverseRead(Node n, string msg) {
exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and
not any(ConsistencyConfiguration conf).reverseReadExclude(n) and
msg = "Origin of readStep is missing a PostUpdateNode."
}
query predicate argHasPostUpdate(ArgumentNode n, string msg) {
not hasPost(n) and
not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and
msg = "ArgumentNode is missing PostUpdateNode."
}
// This predicate helps the compiler forget that in some languages
// it is impossible for a `PostUpdateNode` to be the target of
// `simpleLocalFlowStep`.
private predicate isPostUpdateNode(Node n) { n instanceof PostUpdateNode or none() }
query predicate postWithInFlow(Node n, string msg) {
isPostUpdateNode(n) and
not clearsContent(n, _) and
simpleLocalFlowStep(_, n) and
not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
msg = "PostUpdateNode should not be the target of local flow."
}
query predicate viableImplInCallContextTooLarge(
DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
) {
callable = viableImplInCallContext(call, ctx) and
not callable = viableCallable(call) and
not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable)
}
query predicate uniqueParameterNodeAtPosition(
DataFlowCallable c, ParameterPosition pos, Node p, string msg
) {
not any(ConsistencyConfiguration conf).uniqueParameterNodeAtPositionExclude(c, pos, p) and
isParameterNode(p, c, pos) and
not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and
msg = "Parameters with overlapping positions."
}
query predicate uniqueParameterNodePosition(
DataFlowCallable c, ParameterPosition pos, Node p, string msg
) {
not any(ConsistencyConfiguration conf).uniqueParameterNodePositionExclude(c, pos, p) and
isParameterNode(p, c, pos) and
not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and
msg = "Parameter node with multiple positions."
}
query predicate uniqueContentApprox(Content c, string msg) {
not exists(unique(ContentApprox approx | approx = getContentApprox(c))) and
msg = "Non-unique content approximation."
}
query predicate identityLocalStep(Node n, string msg) {
simpleLocalFlowStep(n, n) and
not any(ConsistencyConfiguration c).identityLocalStepExclude(n) and
msg = "Node steps to itself"
}
}

View File

@@ -558,9 +558,7 @@ import Cached
/** Holds if `n` should be hidden from path explanations. */
predicate nodeIsHidden(Node n) {
exists(SsaImpl::DefinitionExt def | def = n.(SsaDefinitionExtNode).getDefinitionExt() |
not def instanceof Ssa::WriteDefinition
)
n.(SsaDefinitionExtNode).isHidden()
or
n = LocalFlow::getParameterDefNode(_)
or
@@ -593,6 +591,13 @@ class SsaDefinitionExtNode extends NodeImpl, TSsaDefinitionExtNode {
/** Gets the underlying variable. */
Variable getVariable() { result = def.getSourceVariable() }
/** Holds if this node should be hidden from path explanations. */
predicate isHidden() {
not def instanceof Ssa::WriteDefinition
or
isDesugarNode(def.(Ssa::WriteDefinition).getWriteAccess().getExpr())
}
override CfgScope getCfgScope() { result = def.getBasicBlock().getScope() }
override Location getLocationImpl() { result = def.getLocation() }
@@ -1593,7 +1598,11 @@ class CastNode extends Node {
*/
predicate neverSkipInPathGraph(Node n) {
// ensure that all variable assignments are included in the path graph
n.(SsaDefinitionExtNode).getDefinitionExt() instanceof Ssa::WriteDefinition
n =
any(SsaDefinitionExtNode def |
def.getDefinitionExt() instanceof Ssa::WriteDefinition and
not def.isHidden()
)
}
class DataFlowExpr = CfgNodes::ExprCfgNode;

View File

@@ -43,6 +43,17 @@ module NetLdap {
/** A call that establishes a LDAP Connection */
private class NetLdapConnection extends DataFlow::CallNode {
NetLdapConnection() { this in [ldap().getAnInstantiation(), ldap().getAMethodCall("open")] }
predicate usesSsl() {
getValue(this, "encryption").getConstantValue().isStringlikeValue("simple_tls")
}
DataFlow::Node getAuthValue(string arg) {
result =
this.getKeywordArgument("auth")
.(DataFlow::HashLiteralNode)
.getElementFromKey(any(Ast::ConstantValue cv | cv.isStringlikeValue(arg)))
}
}
/** A call that constructs a LDAP query */
@@ -67,4 +78,29 @@ module NetLdap {
override DataFlow::Node getQuery() { result = this.getKeywordArgument(_) }
}
/** A call considered as a LDAP bind. */
private class NetLdapBind extends LdapBind::Range, DataFlow::CallNode {
private NetLdapConnection l;
NetLdapBind() { this = l.getAMethodCall("bind") }
override DataFlow::Node getHost() { result = getValue(l, "host") }
override DataFlow::Node getPassword() {
result = l.getAuthValue("password") or
result = l.getAMethodCall("auth").getArgument(1)
}
override predicate usesSsl() { l.usesSsl() }
}
/** LDAP Attribute value */
DataFlow::Node getValue(NetLdapConnection l, string attr) {
result =
[
l.getKeywordArgument(attr), l.getAMethodCall(attr).getArgument(0),
l.getAMethodCall(attr).getKeywordArgument(attr)
]
}
}

View File

@@ -0,0 +1,49 @@
/**
* Provides default sources, sinks and sanitizers for detecting
* improper LDAP authentication, as well as extension points for adding your own
*/
private import codeql.ruby.Concepts
private import codeql.ruby.DataFlow
private import codeql.ruby.dataflow.BarrierGuards
private import codeql.ruby.dataflow.RemoteFlowSources
/**
* Provides default sources, sinks and sanitizers for detecting
* improper LDAP authentication, as well as extension points for adding your own
*/
module ImproperLdapAuth {
/** A data flow source for improper LDAP authentication vulnerabilities */
abstract class Source extends DataFlow::Node { }
/** A data flow sink for improper LDAP authentication vulnerabilities */
abstract class Sink extends DataFlow::Node { }
/** A sanitizer for improper LDAP authentication vulnerabilities. */
abstract class Sanitizer extends DataFlow::Node { }
/**
* A source of remote user input, considered as a flow source.
*/
private class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
/**
* An LDAP query execution considered as a flow sink.
*/
private class LdapBindAsSink extends Sink {
LdapBindAsSink() { this = any(LdapBind l).getPassword() }
}
/**
* A comparison with a constant string, considered as a sanitizer-guard.
*/
private class StringConstCompareAsSanitizerGuard extends Sanitizer, StringConstCompareBarrier { }
/**
* An inclusion check against an array of constant strings, considered as a
* sanitizer-guard.
*/
private class StringConstArrayInclusionCallAsSanitizer extends Sanitizer,
StringConstArrayInclusionCallBarrier
{ }
}

View File

@@ -0,0 +1,21 @@
/**
* Provides default sources, sinks and sanitizers for detecting
* improper LDAP authentication, as well as extension points for adding your own
*/
private import codeql.ruby.DataFlow
private import codeql.ruby.TaintTracking
private import ImproperLdapAuthCustomizations::ImproperLdapAuth
/**
* A taint-tracking configuration for detecting improper LDAP authentication vulnerabilities.
*/
class Configuration extends TaintTracking::Configuration {
Configuration() { this = "ImproperLdapAuth" }
override predicate isSource(DataFlow::Node source) { source instanceof Source }
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
}

View File

@@ -1,5 +1,5 @@
name: codeql/ruby-all
version: 0.7.3-dev
version: 0.7.4-dev
groups: ruby
extractor: ruby
dbscheme: ruby.dbscheme