Merge pull request #14108 from hvitved/dataflow/more-consistency-checks

Data flow: Add `ArgumentNode` consistency checks
This commit is contained in:
Tom Hvitved
2023-09-13 11:30:51 +02:00
committed by GitHub
39 changed files with 153 additions and 51 deletions

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -139,3 +138,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -32,3 +31,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -10,7 +10,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -192,3 +191,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -53,3 +52,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -16,7 +16,6 @@ uniqueNodeLocation
missingLocation
| Nodes without location: 2 |
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -98,3 +97,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -5,8 +5,6 @@ uniqueNodeLocation
missingLocation
uniqueNodeToString
| cpp11.cpp:50:15:50:16 | (no string representation) | Node should have one toString but has 0. |
missingToString
| Nodes without toString: 1 |
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -54,3 +52,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -72,11 +72,44 @@ private module Input implements InputSig<CsharpDataFlow> {
}
predicate reverseReadExclude(Node n) { n.asExpr() = any(AwaitExpr ae).getExpr() }
predicate missingArgumentCallExclude(ArgumentNode arg) {
// TODO: Remove once object initializers are modeled properly
arg.(Private::PostUpdateNodes::ObjectInitializerNode).getInitializer() instanceof
ObjectInitializer
or
// TODO: Remove once underlying issue is fixed
exists(QualifiableExpr qe |
qe.isConditional() and
qe.getQualifier() = arg.asExpr()
)
}
predicate multipleArgumentCallExclude(ArgumentNode arg, DataFlowCall call) {
isArgumentNode(arg, call, _) and
(
// TODO: Remove once object initializers are modeled properly
arg =
any(Private::PostUpdateNodes::ObjectInitializerNode init |
init.argumentOf(call, _) and
init.getInitializer().getNumberOfChildren() > 1
)
or
exists(ControlFlow::Nodes::ElementNode cfn, ControlFlow::Nodes::Split split |
exists(arg.asExprAtNode(cfn))
|
split = cfn.getASplit() and
not split = call.getControlFlowNode().getASplit()
or
split = call.getControlFlowNode().getASplit() and
not split = cfn.getASplit()
)
or
call instanceof TransitiveCapturedDataFlowCall
or
call.(NonDelegateDataFlowCall).getDispatchCall().isReflection()
)
}
}
import MakeConsistency<CsharpDataFlow, CsharpTaintTracking, Input>
query predicate multipleToString(DataFlow::Node n, string s) {
s = strictconcat(n.toString(), ",") and
strictcount(n.toString()) > 1
}

View File

@@ -2032,7 +2032,7 @@ abstract class PostUpdateNode extends Node {
abstract Node getPreUpdateNode();
}
private module PostUpdateNodes {
module PostUpdateNodes {
class ObjectCreationNode extends PostUpdateNode, ExprNode, TExprNode {
private ObjectCreation oc;

View File

@@ -50,6 +50,9 @@ class DispatchCall extends Internal::TDispatchCall {
RuntimeCallable getADynamicTargetInCallContext(DispatchCall ctx) {
result = Internal::getADynamicTargetInCallContext(this, ctx)
}
/** Holds if this call uses reflection. */
predicate isReflection() { this instanceof Internal::TDispatchReflectionCall }
}
/** Internal implementation details. */

View File

@@ -47,6 +47,10 @@ private module Input implements InputSig<PythonDataFlow> {
predicate identityLocalStepExclude(Node n) {
not exists(n.getLocation().getFile().getRelativePath())
}
predicate multipleArgumentCallExclude(ArgumentNode arg, DataFlowCall call) {
isArgumentNode(arg, call, _)
}
}
module Consistency = MakeConsistency<PythonDataFlow, PythonTaintTracking, Input>;

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -8,7 +8,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -28,3 +27,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -6,7 +6,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -39,3 +38,5 @@ identityLocalStep
| test.py:678:9:678:12 | ControlFlowNode for SINK | Node steps to itself |
| test.py:686:9:686:12 | ControlFlowNode for SINK | Node steps to itself |
| test.py:692:5:692:8 | ControlFlowNode for SINK | Node steps to itself |
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -26,3 +25,5 @@ uniqueContentApprox
identityLocalStep
| test_collections.py:20:9:20:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
| test_unpacking.py:31:9:31:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -39,3 +38,5 @@ identityLocalStep
| test_collections.py:246:9:246:15 | ControlFlowNode for my_dict | Node steps to itself |
| test_collections.py:246:22:246:33 | ControlFlowNode for tainted_dict | Node steps to itself |
| test_for.py:24:9:24:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -4,7 +4,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -24,3 +23,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -10,7 +10,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -33,3 +32,5 @@ identityLocalStep
| test_collections.py:36:10:36:15 | ControlFlowNode for SOURCE | Node steps to itself |
| test_collections.py:45:19:45:21 | ControlFlowNode for mod | Node steps to itself |
| test_collections.py:52:13:52:21 | ControlFlowNode for mod_local | Node steps to itself |
missingArgumentCall
multipleArgumentCall

View File

@@ -35,7 +35,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -55,3 +54,5 @@ uniqueParameterNodeAtPosition
uniqueParameterNodePosition
uniqueContentApprox
identityLocalStep
missingArgumentCall
multipleArgumentCall

View File

@@ -8,7 +8,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -30,3 +29,5 @@ uniqueContentApprox
identityLocalStep
| test_captured.py:7:22:7:22 | ControlFlowNode for p | Node steps to itself |
| test_captured.py:14:26:14:27 | ControlFlowNode for pp | Node steps to itself |
missingArgumentCall
multipleArgumentCall

View File

@@ -39,7 +39,6 @@ uniqueType
uniqueNodeLocation
missingLocation
uniqueNodeToString
missingToString
parameterCallable
localFlowIsLocal
readStepIsLocal
@@ -125,3 +124,5 @@ identityLocalStep
| testapp/tests.py:16:9:16:18 | ControlFlowNode for test_names | Node steps to itself |
| testapp/tests.py:25:13:25:14 | ControlFlowNode for re | Node steps to itself |
| testapp/tests.py:31:9:31:18 | ControlFlowNode for test_names | Node steps to itself |
missingArgumentCall
multipleArgumentCall

View File

@@ -33,11 +33,17 @@ private module Input implements InputSig<RubyDataFlow> {
n.asExpr() = arg
)
}
predicate multipleArgumentCallExclude(ArgumentNode arg, DataFlowCall call) {
// An argument such as `x` in `if not x then ...` has two successors (and hence
// two calls); one for each Boolean outcome of `x`.
exists(CfgNodes::ExprCfgNode n |
arg.argumentOf(call, _) and
n = call.asCall() and
arg.asExpr().getASuccessor(any(SuccessorTypes::ConditionalSuccessor c)).getASuccessor*() = n and
n.getASplit() instanceof Split::ConditionalCompletionSplit
)
}
}
import MakeConsistency<RubyDataFlow, RubyTaintTracking, Input>
query predicate multipleToString(DataFlow::Node n, string s) {
s = strictconcat(n.toString(), ",") and
strictcount(n.toString()) > 1
}

View File

@@ -7,7 +7,6 @@ private import codeql.ruby.ast.internal.Constant
private import codeql.ruby.ast.internal.Literal
private import ControlFlowGraph
private import internal.ControlFlowGraphImpl as CfgImpl
private import internal.Splitting
/** An entry node for a given scope. */
class EntryNode extends CfgNode, CfgImpl::EntryNode {

View File

@@ -4,7 +4,7 @@ private import codeql.ruby.AST
private import codeql.ruby.controlflow.BasicBlocks
private import SuccessorTypes
private import internal.ControlFlowGraphImpl as CfgImpl
private import internal.Splitting
private import internal.Splitting as Splitting
private import internal.Completion
/**
@@ -293,3 +293,10 @@ module SuccessorTypes {
final override string toString() { result = "exit" }
}
}
class Split = Splitting::Split;
/** Provides different kinds of control flow graph splittings. */
module Split {
class ConditionalCompletionSplit = Splitting::ConditionalCompletionSplit;
}

View File

@@ -115,6 +115,8 @@ private module ConditionalCompletionSplitting {
}
}
class ConditionalCompletionSplit = ConditionalCompletionSplitting::ConditionalCompletionSplit;
module EnsureSplitting {
/**
* The type of a split `ensure` node.

View File

@@ -58,6 +58,16 @@ signature module InputSig<DF::InputSig DataFlowLang> {
/** Holds if `n` should be excluded from the consistency test `identityLocalStep`. */
default predicate identityLocalStepExclude(DataFlowLang::Node n) { none() }
/** Holds if `arg` should be excluded from the consistency test `missingArgumentCall`. */
default predicate missingArgumentCallExclude(DataFlowLang::ArgumentNode arg) { none() }
/** Holds if `(arg, call)` should be excluded from the consistency test `multipleArgumentCall`. */
default predicate multipleArgumentCallExclude(
DataFlowLang::ArgumentNode arg, DataFlowLang::DataFlowCall call
) {
none()
}
}
module MakeConsistency<
@@ -147,13 +157,6 @@ module MakeConsistency<
)
}
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."
@@ -287,4 +290,20 @@ module MakeConsistency<
not Input::identityLocalStepExclude(n) and
msg = "Node steps to itself"
}
query predicate missingArgumentCall(ArgumentNode arg, string msg) {
not Input::missingArgumentCallExclude(arg) and
not isArgumentNode(arg, _, _) and
msg = "Missing call for argument node."
}
query predicate multipleArgumentCall(ArgumentNode arg, DataFlowCall call, string msg) {
isArgumentNode(arg, call, _) and
not Input::multipleArgumentCallExclude(arg, call) and
strictcount(DataFlowCall call0 |
isArgumentNode(arg, call0, _) and
not Input::multipleArgumentCallExclude(arg, call0)
) > 1 and
msg = "Multiple calls for argument node."
}
}