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#: Use shared SuccessorType.

Browse Source
This commit is contained in:
Anders Schack-Mulligen
2025-08-27 13:35:41 +02:00
parent 8434dc3890
commit 8b50ac291f
20 changed files with 38 additions and 379 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -10,7 +10,6 @@ module Stages {
cached
module ControlFlowStage {
private import semmle.code.csharp.controlflow.internal.Splitting
private import semmle.code.csharp.controlflow.internal.SuccessorType
private import semmle.code.csharp.controlflow.Guards as Guards
cached
@@ -20,8 +19,6 @@ module Stages {
private predicate forceCachingInSameStageRev() {
exists(Split s)
or
exists(SuccessorType st)
or
exists(ControlFlow::Node n)
or
Guards::Internal::isCustomNullCheck(_, _, _, _)

4
csharp/ql/lib/semmle/code/csharp/commons/Constants.qll
View File

@@ -6,9 +6,7 @@ private import semmle.code.csharp.commons.StructuralComparison as StructuralComp
pragma[noinline]
private predicate isConstantCondition0(ControlFlow::Node cfn, boolean b) {
exists(
cfn.getASuccessorByType(any(ControlFlow::SuccessorTypes::BooleanSuccessor t | t.getValue() = b))
) and
exists(cfn.getASuccessorByType(any(ControlFlow::BooleanSuccessor t | t.getValue() = b))) and
strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1
}

2
csharp/ql/lib/semmle/code/csharp/controlflow/BasicBlocks.qll
View File

@@ -3,7 +3,7 @@
*/
import csharp
private import ControlFlow::SuccessorTypes
private import ControlFlow
private import semmle.code.csharp.controlflow.internal.ControlFlowGraphImpl as CfgImpl
private import CfgImpl::BasicBlocks as BasicBlocksImpl
private import codeql.controlflow.BasicBlock as BB

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

@@ -5,7 +5,6 @@ private import semmle.code.csharp.ExprOrStmtParent
private import semmle.code.csharp.commons.Compilation
private import ControlFlow
private import ControlFlow::BasicBlocks
private import SuccessorTypes
private import semmle.code.csharp.Caching
private import internal.ControlFlowGraphImpl as Impl

1
csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowGraph.qll
View File

@@ -6,7 +6,6 @@ import csharp
module ControlFlow {
private import semmle.code.csharp.controlflow.BasicBlocks as BBs
import semmle.code.csharp.controlflow.internal.SuccessorType
private import SuccessorTypes
private import internal.ControlFlowGraphImpl as Impl
private import internal.Splitting as Splitting

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

@@ -3,7 +3,7 @@
*/
import csharp
private import ControlFlow::SuccessorTypes
private import ControlFlow
private import semmle.code.csharp.commons.Assertions
private import semmle.code.csharp.commons.ComparisonTest
private import semmle.code.csharp.commons.StructuralComparison as SC

7
csharp/ql/lib/semmle/code/csharp/controlflow/internal/Completion.qll
View File

@@ -26,7 +26,6 @@ private import semmle.code.csharp.frameworks.System
private import ControlFlowGraphImpl
private import NonReturning
private import SuccessorType
private import SuccessorTypes
private newtype TCompletion =
TSimpleCompletion() or
@@ -575,7 +574,7 @@ abstract private class NonNestedNormalCompletion extends NormalCompletion { }
/** A simple (normal) completion. */
class SimpleCompletion extends NonNestedNormalCompletion, TSimpleCompletion {
override NormalSuccessor getAMatchingSuccessorType() { any() }
override DirectSuccessor getAMatchingSuccessorType() { any() }
override string toString() { result = "normal" }
}
@@ -859,7 +858,7 @@ class GotoCompletion extends Completion {
/** Gets the label of the `goto` completion. */
string getLabel() { result = label }
override GotoSuccessor getAMatchingSuccessorType() { result.getLabel() = label }
override GotoSuccessor getAMatchingSuccessorType() { any() }
override string toString() {
// `NestedCompletion` defines `toString()` for the other case
@@ -882,7 +881,7 @@ class ThrowCompletion extends Completion {
/** Gets the type of the exception being thrown. */
ExceptionClass getExceptionClass() { result = ec }
override ExceptionSuccessor getAMatchingSuccessorType() { result.getExceptionClass() = ec }
override ExceptionSuccessor getAMatchingSuccessorType() { any() }
override string toString() {
// `NestedCompletion` defines `toString()` for the other case

12
csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll
View File

@@ -83,17 +83,13 @@ private module CfgInput implements CfgShared::InputSig<Location> {
SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() }
predicate successorTypeIsSimple(SuccessorType t) {
t instanceof ST::SuccessorTypes::NormalSuccessor
}
predicate successorTypeIsSimple(SuccessorType t) { t instanceof ST::DirectSuccessor }
predicate successorTypeIsCondition(SuccessorType t) {
t instanceof ST::SuccessorTypes::ConditionalSuccessor
}
predicate successorTypeIsCondition(SuccessorType t) { t instanceof ST::ConditionalSuccessor }
predicate isAbnormalExitType(SuccessorType t) {
t instanceof ST::SuccessorTypes::ExceptionSuccessor or
t instanceof ST::SuccessorTypes::ExitSuccessor
t instanceof ST::ExceptionSuccessor or
t instanceof ST::ExitSuccessor
}
int idOfAstNode(AstNode node) { result = node.getId() }

2
csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreBasicBlocks.qll
View File

@@ -150,7 +150,7 @@ class ConditionBlock extends PreBasicBlock {
}
pragma[nomagic]
predicate controls(PreBasicBlock controlled, Cfg::SuccessorTypes::ConditionalSuccessor s) {
predicate controls(PreBasicBlock controlled, Cfg::ConditionalSuccessor s) {
exists(PreBasicBlock succ, ConditionalCompletion c |
conditionBlockImmediatelyControls(this, succ, c)
|

17
csharp/ql/lib/semmle/code/csharp/controlflow/internal/Splitting.qll
View File

@@ -470,8 +470,11 @@ module FinallySplitting {
* then the `finally` block must end with a `return` as well (provided that
* the `finally` block exits normally).
*/
class FinallySplitType extends Cfg::SuccessorType {
FinallySplitType() { not this instanceof Cfg::SuccessorTypes::ConditionalSuccessor }
class FinallySplitType instanceof Cfg::SuccessorType {
FinallySplitType() { not this instanceof Cfg::ConditionalSuccessor }
/** Gets a textual representation of this successor type. */
string toString() { result = super.toString() }
/** Holds if this split type matches entry into a `finally` block with completion `c`. */
predicate isSplitForEntryCompletion(Completion c) {
@@ -479,7 +482,7 @@ module FinallySplitting {
then
// If the entry into the `finally` block completes with any normal completion,
// it simply means normal execution after the `finally` block
this instanceof Cfg::SuccessorTypes::NormalSuccessor
this instanceof Cfg::DirectSuccessor
else this = c.getAMatchingSuccessorType()
}
}
@@ -533,7 +536,7 @@ module FinallySplitting {
int getNestLevel() { result = nestLevel }
override string toString() {
if type instanceof Cfg::SuccessorTypes::NormalSuccessor
if type instanceof Cfg::DirectSuccessor
then result = ""
else
if nestLevel > 0
@@ -617,14 +620,14 @@ module FinallySplitting {
or
not c instanceof NormalCompletion
or
type instanceof Cfg::SuccessorTypes::NormalSuccessor
type instanceof Cfg::DirectSuccessor
)
else (
// Finally block can exit with completion `c` inherited from try/catch
// block: must match this split
inherited = true and
type = c.getAMatchingSuccessorType() and
not type instanceof Cfg::SuccessorTypes::NormalSuccessor
not type instanceof Cfg::DirectSuccessor
)
)
or
@@ -657,7 +660,7 @@ module FinallySplitting {
exists(FinallySplit outer |
outer.getNestLevel() = super.getNestLevel() - 1 and
outer.(FinallySplitImpl).exit(pred, c, inherited) and
super.getType() instanceof Cfg::SuccessorTypes::NormalSuccessor and
super.getType() instanceof Cfg::DirectSuccessor and
inherited = true
)
}

327
csharp/ql/lib/semmle/code/csharp/controlflow/internal/SuccessorType.qll
View File

@@ -4,329 +4,4 @@
* Provides different types of control flow successor types.
*/
import csharp
private import Completion
private import semmle.code.csharp.Caching
cached
private newtype TSuccessorType =
TSuccessorSuccessor() { Stages::ControlFlowStage::forceCachingInSameStage() } or
TBooleanSuccessor(boolean b) { b = true or b = false } or
TNullnessSuccessor(boolean isNull) { isNull = true or isNull = false } or
TMatchingSuccessor(boolean isMatch) { isMatch = true or isMatch = false } or
TEmptinessSuccessor(boolean isEmpty) { isEmpty = true or isEmpty = false } or
TReturnSuccessor() or
TBreakSuccessor() or
TContinueSuccessor() or
TGotoSuccessor(string label) { label = any(GotoStmt gs).getLabel() } or
TExceptionSuccessor(ExceptionClass ec) { exists(ThrowCompletion c | c.getExceptionClass() = ec) } or
TExitSuccessor()
/** The type of a control flow successor. */
class SuccessorType extends TSuccessorType {
/** Gets a textual representation of successor type. */
string toString() { none() }
}
/** Provides different types of control flow successor types. */
module SuccessorTypes {
/** A normal control flow successor. */
class NormalSuccessor extends SuccessorType, TSuccessorSuccessor {
override string toString() { result = "successor" }
}
/**
* A conditional control flow successor. Either a Boolean successor (`BooleanSuccessor`),
* a nullness successor (`NullnessSuccessor`), a matching successor (`MatchingSuccessor`),
* or an emptiness successor (`EmptinessSuccessor`).
*/
abstract class ConditionalSuccessor extends SuccessorType {
/** Gets the Boolean value of this successor. */
abstract boolean getValue();
}
/**
* A Boolean control flow successor.
*
* For example, this program fragment:
*
* ```csharp
* if (x < 0)
* return 0;
* else
* return 1;
* ```
*
* has a control flow graph containing Boolean successors:
*
* ```
* if
* |
* x < 0
* / \
* / \
* / \
* true false
* | \
* return 0 return 1
* ```
*/
class BooleanSuccessor extends ConditionalSuccessor, TBooleanSuccessor {
override boolean getValue() { this = TBooleanSuccessor(result) }
override string toString() { result = this.getValue().toString() }
}
/**
* A nullness control flow successor.
*
* For example, this program fragment:
*
* ```csharp
* int? M(string s) => s?.Length;
* ```
*
* has a control flow graph containing nullness successors:
*
* ```
* enter M
* |
* s
* / \
* / \
* / \
* null non-null
* \ |
* \ Length
* \ /
* \ /
* exit M
* ```
*/
class NullnessSuccessor extends ConditionalSuccessor, TNullnessSuccessor {
/** Holds if this is a `null` successor. */
predicate isNull() { this = TNullnessSuccessor(true) }
override boolean getValue() { this = TNullnessSuccessor(result) }
override string toString() { if this.isNull() then result = "null" else result = "non-null" }
}
/**
* A matching control flow successor.
*
* For example, this program fragment:
*
* ```csharp
* switch (x) {
* case 0 :
* return 0;
* default :
* return 1;
* }
* ```
*
* has a control flow graph containing matching successors:
*
* ```
* switch
* |
* x
* |
* case 0
* / \
* / \
* / \
* match no-match
* | \
* return 0 default
* |
* return 1
* ```
*/
class MatchingSuccessor extends ConditionalSuccessor, TMatchingSuccessor {
/** Holds if this is a match successor. */
predicate isMatch() { this = TMatchingSuccessor(true) }
override boolean getValue() { this = TMatchingSuccessor(result) }
override string toString() { if this.isMatch() then result = "match" else result = "no-match" }
}
/**
* An emptiness control flow successor.
*
* For example, this program fragment:
*
* ```csharp
* foreach (var arg in args)
* {
* yield return arg;
* }
* yield return "";
* ```
*
* has a control flow graph containing emptiness successors:
*
* ```
* args
* |
* foreach------<-------
* / \ \
* / \ |
* / \ |
* / \ |
* empty non-empty |
* | \ |
* yield return "" \ |
* var arg |
* | |
* yield return arg |
* \_________/
* ```
*/
class EmptinessSuccessor extends ConditionalSuccessor, TEmptinessSuccessor {
/** Holds if this is an empty successor. */
predicate isEmpty() { this = TEmptinessSuccessor(true) }
override boolean getValue() { this = TEmptinessSuccessor(result) }
override string toString() { if this.isEmpty() then result = "empty" else result = "non-empty" }
}
/**
* A `return` control flow successor.
*
* Example:
*
* ```csharp
* void M()
* {
* return;
* }
* ```
*
* The callable exit node of `M` is a `return` successor of the `return;`
* statement.
*/
class ReturnSuccessor extends SuccessorType, TReturnSuccessor {
override string toString() { result = "return" }
}
/**
* A `break` control flow successor.
*
* Example:
*
* ```csharp
* int M(int x)
* {
* while (true)
* {
* if (x++ > 10)
* break;
* }
* return x;
* }
* ```
*
* The node `return x;` is a `break` successor of the node `break;`.
*/
class BreakSuccessor extends SuccessorType, TBreakSuccessor {
override string toString() { result = "break" }
}
/**
* A `continue` control flow successor.
*
* Example:
*
* ```csharp
* int M(int x)
* {
* while (true) {
* if (x++ < 10)
* continue;
* }
* return x;
* }
* ```
*
* The node `while (true) { ... }` is a `continue` successor of the node
* `continue;`.
*/
class ContinueSuccessor extends SuccessorType, TContinueSuccessor {
override string toString() { result = "continue" }
}
/**
* A `goto` control flow successor.
*
* Example:
*
* ```csharp
* int M(int x)
* {
* while (true)
* {
* if (x++ > 10)
* goto Return;
* }
* Return: return x;
* }
* ```
*
* The node `Return: return x` is a `goto label` successor of the node
* `goto Return;`.
*/
class GotoSuccessor extends SuccessorType, TGotoSuccessor {
/** Gets the `goto` label. */
string getLabel() { this = TGotoSuccessor(result) }
override string toString() { result = "goto(" + this.getLabel() + ")" }
}
/**
* An exceptional control flow successor.
*
* Example:
*
* ```csharp
* int M(string s)
* {
* if (s == null)
* throw new ArgumentNullException(nameof(s));
* return s.Length;
* }
* ```
*
* The callable exit node of `M` is an exceptional successor (of type
* `ArgumentNullException`) of the node `throw new ArgumentNullException(nameof(s));`.
*/
class ExceptionSuccessor extends SuccessorType, TExceptionSuccessor {
/** Gets the type of exception. */
ExceptionClass getExceptionClass() { this = TExceptionSuccessor(result) }
override string toString() { result = "exception(" + this.getExceptionClass().getName() + ")" }
}
/**
* An exit control flow successor.
*
* Example:
*
* ```csharp
* int M(string s)
* {
* if (s == null)
* System.Environment.Exit(0);
* return s.Length;
* }
* ```
*
* The callable exit node of `M` is an exit successor of the node on line 4.
*/
class ExitSuccessor extends SuccessorType, TExitSuccessor {
override string toString() { result = "exit" }
}
}
import codeql.controlflow.SuccessorType

6
csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll
View File

@@ -151,9 +151,7 @@ private predicate exprImpliesSsaDef(
* If the returned element takes the `s` branch, then `def` is guaranteed to be
* `null` if `nv.isNull()` holds, and non-`null` otherwise.
*/
private ControlFlowElement getANullCheck(
Ssa::Definition def, SuccessorTypes::ConditionalSuccessor s, NullValue nv
) {
private ControlFlowElement getANullCheck(Ssa::Definition def, ConditionalSuccessor s, NullValue nv) {
exists(Expr e, G::AbstractValue v | v.branch(result, s, e) | exprImpliesSsaDef(e, v, def, nv))
}
@@ -294,7 +292,7 @@ private predicate defNullImpliesStep(
bb2 = phi.getBasicBlock()
)
) and
not exists(SuccessorTypes::ConditionalSuccessor s, NullValue nv |
not exists(ConditionalSuccessor s, NullValue nv |
bb1.getLastNode() = getANullCheck(def1, s, nv).getAControlFlowNode()
|
bb2 = bb1.getASuccessor(s) and

2
csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll
View File

@@ -28,7 +28,7 @@ module BaseSsa {
private predicate implicitEntryDef(
Callable c, ControlFlow::BasicBlocks::EntryBlock bb, SsaInput::SourceVariable v
) {
exists(ControlFlow::ControlFlow::BasicBlocks::EntryBlock entry |
exists(ControlFlow::BasicBlocks::EntryBlock entry |
c = entry.getCallable() and
// In case `c` has multiple bodies, we want each body to get its own implicit
// entry definition. In case `c` doesn't have multiple bodies, the line below

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

@@ -2583,9 +2583,7 @@ 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::SuccessorTypes::BooleanSuccessor bs
|
exists(ExplicitParameterNode paramNode, Guard guard, ControlFlow::BooleanSuccessor bs |
viableConstantBooleanParamArg(paramNode, bs.getValue().booleanNot(), call) and
paramNode.getSsaDefinition().getARead() = guard and
guard.controlsBlock(nr, bs, _)

8
csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll
View File

@@ -818,8 +818,8 @@ private module Cached {
}
cached
predicate implicitEntryDefinition(ControlFlow::ControlFlow::BasicBlock bb, Ssa::SourceVariable v) {
exists(ControlFlow::ControlFlow::BasicBlocks::EntryBlock entry, Callable c |
predicate implicitEntryDefinition(ControlFlow::BasicBlock bb, Ssa::SourceVariable v) {
exists(ControlFlow::BasicBlocks::EntryBlock entry, Callable c |
c = entry.getCallable() and
// In case `c` has multiple bodies, we want each body to get its own implicit
// entry definition. In case `c` doesn't have multiple bodies, the line below
@@ -1045,7 +1045,7 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
* from `bb1` to `bb2`.
*/
predicate hasValueBranchEdge(BasicBlock bb1, BasicBlock bb2, GuardValue branch) {
exists(ControlFlow::SuccessorTypes::ConditionalSuccessor s |
exists(ControlFlow::ConditionalSuccessor s |
this.getAControlFlowNode() = bb1.getLastNode() and
bb2 = bb1.getASuccessor(s) and
s.getValue() = branch
@@ -1064,7 +1064,7 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
/** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */
predicate guardDirectlyControlsBlock(Guard guard, ControlFlow::BasicBlock bb, GuardValue branch) {
exists(ConditionBlock conditionBlock, ControlFlow::SuccessorTypes::ConditionalSuccessor s |
exists(ConditionBlock conditionBlock, ControlFlow::ConditionalSuccessor s |
guard.getAControlFlowNode() = conditionBlock.getLastNode() and
s.getValue() = branch and
conditionBlock.edgeDominates(bb, s)

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

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

4
csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql
View File

@@ -89,7 +89,7 @@ class ConstantNullnessCondition extends ConstantCondition {
ConstantNullnessCondition() {
forex(ControlFlow::Node cfn | cfn = this.getAControlFlowNode() |
exists(ControlFlow::SuccessorTypes::NullnessSuccessor t, ControlFlow::Node s |
exists(ControlFlow::NullnessSuccessor t, ControlFlow::Node s |
s = cfn.getASuccessorByType(t)
|
b = t.getValue() and
@@ -112,7 +112,7 @@ class ConstantMatchingCondition extends ConstantCondition {
ConstantMatchingCondition() {
forex(ControlFlow::Node cfn | cfn = this.getAControlFlowNode() |
exists(ControlFlow::SuccessorTypes::MatchingSuccessor t | exists(cfn.getASuccessorByType(t)) |
exists(ControlFlow::MatchingSuccessor t | exists(cfn.getASuccessorByType(t)) |
b = t.getValue()
) and
strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1

4
csharp/ql/src/Security Features/CWE-384/AbandonSession.ql
View File

@@ -21,10 +21,10 @@ predicate loginMethod(Method m, ControlFlow::SuccessorType flowFrom) {
or
m = any(SystemWebSecurityFormsAuthenticationClass c).getAuthenticateMethod()
) and
flowFrom.(ControlFlow::SuccessorTypes::BooleanSuccessor).getValue() = true
flowFrom.(ControlFlow::BooleanSuccessor).getValue() = true
or
m = any(SystemWebSecurityFormsAuthenticationClass c).getSignOutMethod() and
flowFrom instanceof ControlFlow::SuccessorTypes::NormalSuccessor
flowFrom instanceof ControlFlow::DirectSuccessor
}
/** The `System.Web.SessionState.HttpSessionState` class. */

3
csharp/ql/test/library-tests/controlflow/graph/Condition.ql
View File

@@ -4,8 +4,7 @@ import ControlFlow
query predicate conditionBlock(
BasicBlocks::ConditionBlock cb, BasicBlock controlled, boolean testIsTrue
) {
cb.edgeDominates(controlled,
any(SuccessorTypes::ConditionalSuccessor s | testIsTrue = s.getValue()))
cb.edgeDominates(controlled, any(ConditionalSuccessor s | testIsTrue = s.getValue()))
}
ControlFlow::Node successor(ControlFlow::Node node, boolean kind) {

2
csharp/ql/test/library-tests/controlflow/graph/Nodes.ql
View File

@@ -12,7 +12,7 @@ class MyFinallySplitControlFlowNode extends ElementNode {
exists(Splitting::FinallySplitting::FinallySplitType type |
type = this.getASplit().(FinallySplit).getType()
|
not type instanceof SuccessorTypes::NormalSuccessor
not type instanceof DirectSuccessor
)
}