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

Merge pull request #20738 from aschackmull/csharp/guards-misc

Browse Source 
C#: Misc Guards-related cleanup.
This commit is contained in:
Anders Schack-Mulligen
2025-11-05 10:22:07 +01:00
committed by GitHub
parent 5d9d6b9a00 7ab25b593d
commit 02fd1dcccd
6 changed files with 29 additions and 169 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
csharp/ql/lib/change-notes/2025-10-04-deprecate-controlsblock.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: deprecated
---
* `ControlFlowElement.controlsBlock` has been deprecated in favor of the Guards library.

2
csharp/ql/lib/semmle/code/csharp/Caching.qll
View File

@@ -33,8 +33,6 @@ module Stages {
cached
private predicate forceCachingInSameStageRev() {
any(ControlFlowElement cfe).controlsBlock(_, _, _)
or
exists(GuardedExpr ge)
or
forceCachingInSameStageRev()

136
csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowElement.qll
View File

@@ -87,148 +87,20 @@ class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
result.getAControlFlowNode()
}
pragma[noinline]
private predicate immediatelyControlsBlockSplit0(
ConditionBlock cb, BasicBlock succ, ConditionalSuccessor s
) {
// Only calculate dominance by explicit recursion for split nodes;
// all other nodes can use regular CFG dominance
this instanceof Impl::SplitAstNode and
cb.getLastNode() = this.getAControlFlowNode() and
succ = cb.getASuccessor(s)
}
pragma[noinline]
private predicate immediatelyControlsBlockSplit1(
ConditionBlock cb, BasicBlock succ, ConditionalSuccessor s, BasicBlock pred, SuccessorType t
) {
this.immediatelyControlsBlockSplit0(cb, succ, s) and
pred = succ.getAPredecessorByType(t) and
pred != cb
}
pragma[noinline]
private predicate immediatelyControlsBlockSplit2(
ConditionBlock cb, BasicBlock succ, ConditionalSuccessor s, BasicBlock pred, SuccessorType t
) {
this.immediatelyControlsBlockSplit1(cb, succ, s, pred, t) and
(
succ.dominates(pred)
or
// `pred` might be another split of this element
pred.getLastNode().getAstNode() = this and
t = s
)
}
/**
* Holds if basic block `succ` is immediately controlled by this control flow
* element with conditional value `s`. That is, `succ` can only be reached from
* the callable entry point by going via the `s` edge out of *some* basic block
* `pred` ending with this element, and `pred` is an immediate predecessor
* of `succ`.
* DEPRECATED: Use `Guard` class instead.
*
* Moreover, this control flow element corresponds to multiple control flow nodes,
* which is why
*
* ```ql
* exists(ConditionBlock cb |
* cb.getLastNode() = this.getAControlFlowNode() |
* cb.immediatelyControls(succ, s)
* )
* ```
*
* does not work.
*
* `cb` records all of the possible condition blocks for this control flow element
* that a path from the callable entry point to `succ` may go through.
*/
pragma[nomagic]
private predicate immediatelyControlsBlockSplit(
BasicBlock succ, ConditionalSuccessor s, ConditionBlock cb
) {
this.immediatelyControlsBlockSplit0(cb, succ, s) and
forall(BasicBlock pred, SuccessorType t |
this.immediatelyControlsBlockSplit1(cb, succ, s, pred, t)
|
this.immediatelyControlsBlockSplit2(cb, succ, s, pred, t)
)
}
pragma[noinline]
private predicate controlsJoinBlockPredecessor(
JoinBlock controlled, ConditionalSuccessor s, int i, ConditionBlock cb
) {
this.controlsBlockSplit(controlled.getJoinBlockPredecessor(i), s, cb)
}
private predicate controlsJoinBlockSplit(JoinBlock controlled, ConditionalSuccessor s, int i) {
i = -1 and
this.controlsJoinBlockPredecessor(controlled, s, _, _)
or
this.controlsJoinBlockSplit(controlled, s, i - 1) and
(
this.controlsJoinBlockPredecessor(controlled, s, i, _)
or
controlled.dominates(controlled.getJoinBlockPredecessor(i))
)
}
cached
private predicate controlsBlockSplit(
BasicBlock controlled, ConditionalSuccessor s, ConditionBlock cb
) {
Stages::GuardsStage::forceCachingInSameStage() and
this.immediatelyControlsBlockSplit(controlled, s, cb)
or
// Equivalent with
//
// ```ql
// exists(JoinBlockPredecessor pred | pred = controlled.getAPredecessor() |
// this.controlsBlockSplit(pred, s)
// ) and
// forall(JoinBlockPredecessor pred | pred = controlled.getAPredecessor() |
// this.controlsBlockSplit(pred, s)
// or
// controlled.dominates(pred)
// )
// ```
//
// but uses no universal recursion for better performance.
exists(int last |
last = max(int i | exists(controlled.(JoinBlock).getJoinBlockPredecessor(i)))
|
this.controlsJoinBlockSplit(controlled, s, last)
) and
this.controlsJoinBlockPredecessor(controlled, s, _, cb)
or
not controlled instanceof JoinBlock and
this.controlsBlockSplit(controlled.getAPredecessor(), s, cb)
}
/**
* Holds if basic block `controlled` is controlled by this control flow element
* with conditional value `s`. That is, `controlled` can only be reached from
* the callable entry point by going via the `s` edge out of *some* basic block
* ending with this element.
*
* This predicate is different from
*
* ```ql
* exists(ConditionBlock cb |
* cb.getLastNode() = this.getAControlFlowNode() |
* cb.controls(controlled, s)
* )
* ```
*
* as control flow splitting is taken into account.
*
* `cb` records all of the possible condition blocks for this control flow element
* that a path from the callable entry point to `controlled` may go through.
*/
predicate controlsBlock(BasicBlock controlled, ConditionalSuccessor s, ConditionBlock cb) {
this.controlsBlockSplit(controlled, s, cb)
or
deprecated predicate controlsBlock(
BasicBlock controlled, ConditionalSuccessor s, ConditionBlock cb
) {
cb.getLastNode() = this.getAControlFlowNode() and
cb.edgeDominates(controlled, s)
}

11
csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll
View File

@@ -49,8 +49,15 @@ private module GuardsInput implements
override predicate isNull() { any() }
}
private class BooleanConstant extends ConstantExpr instanceof BoolLiteral {
override boolean asBooleanValue() { result = super.getBoolValue() }
private predicate boolConst(Expr e, boolean b) {
e.getType() instanceof BoolType and
e.getValue() = b.toString()
}
private class BooleanConstant extends ConstantExpr {
BooleanConstant() { boolConst(this, _) }
override boolean asBooleanValue() { boolConst(this, result) }
}
private predicate intConst(Expr e, int i) {

36
csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
View File

@@ -2583,10 +2583,10 @@ class NodeRegion instanceof ControlFlow::BasicBlock {
* Holds if the nodes in `nr` are unreachable when the call context is `call`.
*/
predicate isUnreachableInCall(NodeRegion nr, DataFlowCall call) {
exists(ExplicitParameterNode paramNode, Guard guard, ControlFlow::BooleanSuccessor bs |
viableConstantBooleanParamArg(paramNode, bs.getValue().booleanNot(), call) and
exists(ExplicitParameterNode paramNode, Guard guard, GuardValue val |
viableConstantParamArg(paramNode, val.getDualValue(), call) and
paramNode.getSsaDefinition().getARead() = guard and
guard.controlsBlock(nr, bs, _)
guard.valueControls(nr, val)
)
}
@@ -2904,33 +2904,19 @@ class CastNode extends Node {
class DataFlowExpr = Expr;
/** Holds if `e` is an expression that always has the same Boolean value `val`. */
private predicate constantBooleanExpr(Expr e, boolean val) {
e.getType() instanceof BoolType and
e.getValue() = val.toString()
or
exists(Ssa::ExplicitDefinition def, Expr src |
e = def.getARead() and
src = def.getADefinition().getSource() and
constantBooleanExpr(src, val)
)
}
/** An argument that always has the same value. */
private class ConstantArgumentNode extends ExprNode {
ConstantArgumentNode() { Guards::InternalUtil::exprHasValue(this.(ArgumentNode).asExpr(), _) }
/** An argument that always has the same Boolean value. */
private class ConstantBooleanArgumentNode extends ExprNode {
ConstantBooleanArgumentNode() { constantBooleanExpr(this.(ArgumentNode).asExpr(), _) }
/** Gets the Boolean value of this expression. */
boolean getBooleanValue() { constantBooleanExpr(this.getExpr(), result) }
/** Gets the value of this expression. */
GuardValue getValue() { Guards::InternalUtil::exprHasValue(this.getExpr(), result) }
}
pragma[noinline]
private predicate viableConstantBooleanParamArg(
ParameterNode paramNode, boolean b, DataFlowCall call
) {
exists(ConstantBooleanArgumentNode arg |
private predicate viableConstantParamArg(ParameterNode paramNode, GuardValue val, DataFlowCall call) {
exists(ConstantArgumentNode arg |
viableParamArg(call, paramNode, arg) and
b = arg.getBooleanValue()
val = arg.getValue()
)
}

9
csharp/ql/lib/semmle/code/csharp/security/dataflow/ConditionalBypassQuery.qll
View File

@@ -72,17 +72,10 @@ class ReverseDnsSource extends Source {
}
}
pragma[noinline]
private predicate conditionControlsCall0(
SensitiveExecutionMethodCall call, Expr e, ControlFlow::BooleanSuccessor s
) {
forex(BasicBlock bb | bb = call.getAControlFlowNode().getBasicBlock() | e.controlsBlock(bb, s, _))
}
private predicate conditionControlsCall(
SensitiveExecutionMethodCall call, SensitiveExecutionMethod def, Expr e, boolean cond
) {
exists(ControlFlow::BooleanSuccessor s | cond = s.getValue() | conditionControlsCall0(call, e, s)) and
e.(Guard).directlyControls(call.getBasicBlock(), cond) and
def = call.getTarget().getUnboundDeclaration()
}