hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 19:26:02 +02:00
Code Issues Packages Projects Releases Wiki Activity

C#: Simpler data-flow modelling of parameters

Browse Source
This commit is contained in:
Tom Hvitved
2020-10-27 13:09:10 +01:00
parent 69ce24d4b8
commit 6ee5cdf2b2
10 changed files with 49 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
csharp/ql/src/semmle/code/cil/Declaration.qll
View File

@@ -24,9 +24,6 @@ class Declaration extends DotNet::Declaration, Element, @cil_declaration {
override Declaration getUnboundDeclaration() { result = this }
/** Holds if this declaration is a source declaration. */
final predicate isUnboundDeclaration() { this = getUnboundDeclaration() }
/**
* DEPRECATED: Use `isUnboundDeclaration()` instead.
*

3
csharp/ql/src/semmle/code/csharp/Member.qll
View File

@@ -16,9 +16,6 @@ private import TypeRef
class Declaration extends DotNet::Declaration, Element, @declaration {
override ValueOrRefType getDeclaringType() { none() }
/** Holds if this declaration is unbound. */
final predicate isUnboundDeclaration() { this = this.getUnboundDeclaration() }
/** Holds if this declaration is unconstructed and in source code. */
final predicate isSourceDeclaration() { this.fromSource() and this.isUnboundDeclaration() }

2
csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll
View File

@@ -391,7 +391,7 @@ class SummaryDelegateCall extends DelegateDataFlowCall, TSummaryDelegateCall {
override DataFlowCallable getARuntimeTarget(CallContext::CallContext cc) {
exists(SummaryDelegateParameterSink p |
p = TSummaryParameterNode(c, pos) and
p.isParameterOf(c, pos) and
result = p.getARuntimeTarget(cc)
)
}

116
csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
View File

@@ -278,14 +278,20 @@ module LocalFlow {
)
}
private Ssa::Definition getSsaDefinition(Node n) {
result = n.(SsaDefinitionNode).getDefinition()
or
result = n.(ExplicitParameterNode).getSsaDefinition()
}
/**
* Holds if there is a local flow step from `nodeFrom` to `nodeTo` involving
* SSA definition `def.
*/
predicate localSsaFlowStep(Ssa::Definition def, Node nodeFrom, Node nodeTo) {
// Flow from SSA definition to first read
// Flow from SSA definition/parameter to first read
exists(ControlFlow::Node cfn |
def = nodeFrom.(SsaDefinitionNode).getDefinition() and
def = getSsaDefinition(nodeFrom) and
nodeTo.asExprAtNode(cfn) = def.getAFirstReadAtNode(cfn)
)
or
@@ -323,11 +329,10 @@ module LocalFlow {
)
}
predicate localFlowCapturedVarStep(SsaDefinitionNode nodeFrom, ImplicitCapturedArgumentNode nodeTo) {
predicate localFlowCapturedVarStep(Node nodeFrom, ImplicitCapturedArgumentNode nodeTo) {
// Flow from SSA definition to implicit captured variable argument
exists(Ssa::ExplicitDefinition def, ControlFlow::Nodes::ElementNode call |
def = nodeFrom.getDefinition()
|
def = getSsaDefinition(nodeFrom) and
def.isCapturedVariableDefinitionFlowIn(_, call, _) and
nodeTo = TImplicitCapturedArgumentNode(call, def.getSourceVariable().getAssignable())
)
@@ -569,9 +574,15 @@ private module Cached {
Stages::DataFlowStage::forceCachingInSameStage() and cfn.getElement() instanceof Expr
} or
TCilExprNode(CIL::Expr e) { e.getImplementation() instanceof CIL::BestImplementation } or
TSsaDefinitionNode(Ssa::Definition def) or
TInstanceParameterNode(Callable c) { c.hasBody() and not c.(Modifiable).isStatic() } or
TCilParameterNode(CIL::MethodParameter p) { p.getMethod().hasBody() } or
TSsaDefinitionNode(Ssa::Definition def) {
// Handled by `TExplicitParameterNode` below
not def.(Ssa::ExplicitDefinition).getADefinition() instanceof
AssignableDefinitions::ImplicitParameterDefinition
} or
TExplicitParameterNode(DotNet::Parameter p) { p.isUnboundDeclaration() } or
TInstanceParameterNode(Callable c) {
c.isUnboundDeclaration() and not c.(Modifiable).isStatic()
} or
TYieldReturnNode(ControlFlow::Nodes::ElementNode cfn) {
any(Callable c).canYieldReturn(cfn.getElement())
} or
@@ -605,18 +616,6 @@ private module Cached {
cfn.getElement() = fla.getQualifier()
)
} or
TSummaryParameterNode(SummarizedCallable c, int i) {
exists(SummaryInput input | FlowSummaryImpl::Private::summary(c, input, _, _, _, _) |
input = SummaryInput::parameter(i)
or
input = SummaryInput::delegate(i)
)
or
exists(SummaryOutput output |
FlowSummaryImpl::Private::summary(c, _, _, output, _, _) and
output = SummaryOutput::delegate(i, _)
)
} or
TSummaryInternalNode(
SummarizedCallable c, FlowSummaryImpl::Private::SummaryInternalNodeState state
) {
@@ -801,12 +800,10 @@ private module Cached {
cached
predicate isUnreachableInCall(Node n, DataFlowCall call) {
exists(
SsaDefinitionNode paramNode, Ssa::ExplicitDefinition param, Guard guard,
ControlFlow::SuccessorTypes::BooleanSuccessor bs
ExplicitParameterNode paramNode, Guard guard, ControlFlow::SuccessorTypes::BooleanSuccessor bs
|
viableConstantBooleanParamArg(paramNode, bs.getValue().booleanNot(), call) and
paramNode.getDefinition() = param and
param.getARead() = guard and
paramNode.getSsaDefinition().getARead() = guard and
guard.controlsBlock(n.getControlFlowNode().getBasicBlock(), bs, _)
)
}
@@ -874,6 +871,11 @@ private module Cached {
def instanceof Ssa::ImplicitCallDefinition
)
or
exists(Parameter p |
p = n.(ParameterNode).getParameter() and
not p.fromSource()
)
or
n instanceof YieldReturnNode
or
n instanceof ImplicitCapturedArgumentNode
@@ -905,33 +907,25 @@ class SsaDefinitionNode extends NodeImpl, TSsaDefinitionNode {
override Location getLocationImpl() { result = def.getLocation() }
override string toStringImpl() {
not explicitParameterNode(this, _) and
result = def.toString()
}
override string toStringImpl() { result = def.toString() }
}
private module ParameterNodes {
abstract private class ParameterNodeImpl extends ParameterNode, NodeImpl { }
/**
* Holds if definition node `node` is an entry definition for parameter `p`.
*/
predicate explicitParameterNode(AssignableDefinitionNode node, Parameter p) {
p = node.getDefinition().(AssignableDefinitions::ImplicitParameterDefinition).getParameter()
}
/**
* The value of an explicit parameter at function entry, viewed as a node in a data
* flow graph.
*/
class ExplicitParameterNode extends ParameterNodeImpl {
class ExplicitParameterNode extends ParameterNodeImpl, TExplicitParameterNode {
private DotNet::Parameter parameter;
ExplicitParameterNode() {
explicitParameterNode(this, parameter)
or
this = TCilParameterNode(parameter)
ExplicitParameterNode() { this = TExplicitParameterNode(parameter) }
/** Gets the SSA definition corresponding to this parameter, if any. */
Ssa::ExplicitDefinition getSsaDefinition() {
result.getADefinition().(AssignableDefinitions::ImplicitParameterDefinition).getParameter() =
this.getParameter()
}
override DotNet::Parameter getParameter() { result = parameter }
@@ -1037,46 +1031,6 @@ private module ParameterNodes {
c = this.getEnclosingCallable()
}
}
/** A parameter node for a callable with a flow summary. */
class SummaryParameterNode extends ParameterNodeImpl, SummaryNodeImpl, TSummaryParameterNode {
private SummarizedCallable sc;
private int i;
SummaryParameterNode() { this = TSummaryParameterNode(sc, i) }
override Parameter getParameter() { result = sc.getParameter(i) }
override predicate isParameterOf(DataFlowCallable c, int pos) {
c = sc and
pos = i
}
override Callable getEnclosingCallableImpl() { result = sc }
override Type getTypeImpl() {
result = sc.getParameter(i).getType()
or
i = -1 and
result = sc.getDeclaringType()
}
override ControlFlow::Node getControlFlowNodeImpl() { none() }
override Location getLocationImpl() {
result = sc.getParameter(i).getLocation()
or
i = -1 and
result = sc.getLocation()
}
override string toStringImpl() {
result = "[summary] " + sc.getParameter(i)
or
i = -1 and
result = "[summary] this"
}
}
}
import ParameterNodes
@@ -1934,7 +1888,7 @@ private class ConstantBooleanArgumentNode extends ExprNode {
pragma[noinline]
private predicate viableConstantBooleanParamArg(
SsaDefinitionNode paramNode, boolean b, DataFlowCall call
ParameterNode paramNode, boolean b, DataFlowCall call
) {
exists(ConstantBooleanArgumentNode arg |
viableParamArg(call, paramNode, arg) and

6
csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
View File

@@ -119,12 +119,10 @@ class ExprNode extends Node {
class ParameterNode extends Node {
ParameterNode() {
// charpred needed to avoid making `ParameterNode` abstract
explicitParameterNode(this, _) or
this = TExplicitParameterNode(_) or
this.(SsaDefinitionNode).getDefinition() instanceof
ImplicitCapturedParameterNodeImpl::SsaCapturedEntryDefinition or
this = TInstanceParameterNode(_) or
this = TCilParameterNode(_) or
this = TSummaryParameterNode(_, _)
this = TInstanceParameterNode(_)
}
/** Gets the parameter corresponding to this node, if any. */

6
csharp/ql/src/semmle/code/csharp/dataflow/internal/DelegateDataFlow.qll
View File

@@ -10,6 +10,7 @@ private import semmle.code.csharp.dataflow.CallContext
private import semmle.code.csharp.dataflow.internal.DataFlowDispatch
private import semmle.code.csharp.dataflow.internal.DataFlowPrivate
private import semmle.code.csharp.dataflow.internal.DataFlowPublic
private import semmle.code.csharp.dataflow.FlowSummary
private import semmle.code.csharp.dispatch.Dispatch
private import semmle.code.csharp.frameworks.system.linq.Expressions
@@ -102,9 +103,10 @@ class DelegateCallExpr extends DelegateFlowSink, DataFlow::ExprNode {
}
/** A parameter of delegate type belonging to a callable with a flow summary. */
class SummaryDelegateParameterSink extends DelegateFlowSink, SummaryParameterNode {
class SummaryDelegateParameterSink extends DelegateFlowSink, ParameterNode {
SummaryDelegateParameterSink() {
this.getType() instanceof SystemLinqExpressions::DelegateExtType
this.getType() instanceof SystemLinqExpressions::DelegateExtType and
this.isParameterOf(any(SummarizedCallable c), _)
}
}

2
csharp/ql/src/semmle/code/csharp/dataflow/internal/FlowSummarySpecific.qll
View File

@@ -87,7 +87,7 @@ module Private {
NodeImpl inputNode(SummarizableCallable c, SummaryInput input) {
exists(int i |
input = TParameterSummaryInput(i) and
result = DataFlowPrivate::TSummaryParameterNode(c, i)
result.(ParameterNode).isParameterOf(c, i)
)
or
exists(int i |

3
csharp/ql/src/semmle/code/dotnet/Declaration.qll
View File

@@ -52,6 +52,9 @@ class Declaration extends NamedElement, @dotnet_declaration {
* | `C<int>.Method<string>` | `C<>.Method<>` |
*/
Declaration getUnboundDeclaration() { result = this }
/** Holds if this declaration is unbound. */
final predicate isUnboundDeclaration() { this.getUnboundDeclaration() = this }
}
/** A member of a type. */

1
csharp/ql/test/library-tests/csharp7/LocalTaintFlow.expected
View File

@@ -1,3 +1,4 @@
| CSharp7.cs:7:7:7:14 | this | CSharp7.cs:9:9:9:9 | this access |
| CSharp7.cs:9:9:9:9 | [post] this access | CSharp7.cs:10:9:10:9 | this access |
| CSharp7.cs:9:9:9:9 | this access | CSharp7.cs:10:9:10:9 | this access |
| CSharp7.cs:10:9:10:9 | [post] this access | CSharp7.cs:11:9:11:9 | this access |

4
csharp/ql/test/library-tests/dataflow/delegates/DelegateFlow.expected
View File

@@ -1,6 +1,6 @@
summaryDelegateCall
| file://:0:0:0:0 | [summary] valueFactory | DelegateFlow.cs:104:23:104:30 | (...) => ... | DelegateFlow.cs:105:23:105:23 | access to local variable f |
| file://:0:0:0:0 | [summary] valueFactory | DelegateFlow.cs:106:13:106:20 | (...) => ... | DelegateFlow.cs:107:23:107:23 | access to local variable f |
| file://:0:0:0:0 | valueFactory | DelegateFlow.cs:104:23:104:30 | (...) => ... | DelegateFlow.cs:105:23:105:23 | access to local variable f |
| file://:0:0:0:0 | valueFactory | DelegateFlow.cs:106:13:106:20 | (...) => ... | DelegateFlow.cs:107:23:107:23 | access to local variable f |
delegateCall
| DelegateFlow.cs:9:9:9:12 | delegate call | DelegateFlow.cs:5:10:5:11 | M1 | DelegateFlow.cs:17:12:17:13 | delegate creation of type Action<Int32> |
| DelegateFlow.cs:9:9:9:12 | delegate call | DelegateFlow.cs:5:10:5:11 | M1 | DelegateFlow.cs:22:12:22:12 | access to parameter a |