diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index 4bbb675a9d16a..3b26093ff5def 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -1809,7 +1809,7 @@ module IteratorFlow { * Holds if `(bb, i)` contains a write to an iterator that may have been obtained * by calling `begin` (or related functions) on the variable `v`. */ - predicate variableWrite(IRBlock bb, int i, SourceVariable v, boolean certain) { + predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) { certain = false and exists(GetsIteratorCall beginCall, Instruction writeToDeref, IRBlock bbQual, int iQual | isIteratorStoreInstruction(beginCall, writeToDeref) and @@ -1820,7 +1820,7 @@ module IteratorFlow { } /** Holds if `(bb, i)` reads the container variable `v`. */ - predicate variableRead(IRBlock bb, int i, SourceVariable v, boolean certain) { + predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain) { Ssa::variableRead(bb, i, v, certain) } } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll index 203437a918617..34fdb500139a9 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll @@ -981,7 +981,7 @@ private module SsaInput implements SsaImplCommon::InputSig { * Holds if the `i`'th write in block `bb` writes to the variable `v`. * `certain` is `true` if the write is guaranteed to overwrite the entire variable. */ - predicate variableWrite(IRBlock bb, int i, SourceVariable v, boolean certain) { + predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) { DataFlowImplCommon::forceCachingInSameStage() and ( exists(DefImpl def | def.hasIndexInBlock(bb, i, v) | @@ -999,7 +999,7 @@ private module SsaInput implements SsaImplCommon::InputSig { * Holds if the `i`'th read in block `bb` reads to the variable `v`. * `certain` is `true` if the read is guaranteed. For C++, this is always the case. */ - predicate variableRead(IRBlock bb, int i, SourceVariable v, boolean certain) { + predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain) { exists(UseImpl use | use.hasIndexInBlock(bb, i, v) | if use.isCertain() then certain = true else certain = false ) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll index 0920e5a386579..65f1c57b80c36 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll @@ -757,13 +757,19 @@ import Cached * between the SSA pruning stage, and the final SSA stage. */ module InputSigCommon { - class BasicBlock = IRBlock; + class BasicBlock extends IRBlock { + ControlFlowNode getNode(int i) { result = this.getInstruction(i) } + + int length() { result = this.getInstructionCount() } + } + + class ControlFlowNode = Instruction; BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.immediatelyDominates(bb) } BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class ExitBasicBlock extends IRBlock { + class ExitBasicBlock extends BasicBlock { ExitBasicBlock() { this.getLastInstruction() instanceof ExitFunctionInstruction } } } diff --git a/csharp/ql/lib/semmle/code/cil/Ssa.qll b/csharp/ql/lib/semmle/code/cil/Ssa.qll index c1dcad4fef5ac..4aebad1a602f1 100644 --- a/csharp/ql/lib/semmle/code/cil/Ssa.qll +++ b/csharp/ql/lib/semmle/code/cil/Ssa.qll @@ -35,7 +35,7 @@ deprecated module Ssa { } /** Gets the location of this SSA definition. */ - Location getLocation() { result = this.getVariableUpdate().getLocation() } + override Location getLocation() { result = this.getVariableUpdate().getLocation() } } /** A phi node. */ diff --git a/csharp/ql/lib/semmle/code/cil/internal/SsaImpl.qll b/csharp/ql/lib/semmle/code/cil/internal/SsaImpl.qll index 70e77c66ddb77..41acf8ed1f6f2 100644 --- a/csharp/ql/lib/semmle/code/cil/internal/SsaImpl.qll +++ b/csharp/ql/lib/semmle/code/cil/internal/SsaImpl.qll @@ -1,14 +1,17 @@ private import cil +private import CIL private import codeql.ssa.Ssa as SsaImplCommon deprecated private module SsaInput implements SsaImplCommon::InputSig { class BasicBlock = CIL::BasicBlock; + class ControlFlowNode = CIL::ControlFlowNode; + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() } BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class ExitBasicBlock = CIL::ExitBasicBlock; + class ExitBasicBlock extends BasicBlock, CIL::ExitBasicBlock { } class SourceVariable = CIL::StackVariable; diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreSsa.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreSsa.qll index c7c1de2308b6f..0984d0ac41ba5 100644 --- a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreSsa.qll +++ b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreSsa.qll @@ -80,7 +80,11 @@ module PreSsa { } module SsaInput implements SsaImplCommon::InputSig { - class BasicBlock = PreBasicBlocks::PreBasicBlock; + class BasicBlock extends PreBasicBlocks::PreBasicBlock { + ControlFlowNode getNode(int i) { result = this.getElement(i) } + } + + class ControlFlowNode = ControlFlowElement; BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.immediatelyDominates(bb) } @@ -192,7 +196,7 @@ module PreSsa { SsaImpl::ssaDefReachesEndOfBlock(bb, this, _) } - Location getLocation() { + override Location getLocation() { result = this.getDefinition().getLocation() or exists(Callable c, SsaInput::BasicBlock bb, SsaInput::SourceVariable v | diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll index 0d79eafdf5c6c..a5960fd349f62 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll @@ -427,7 +427,7 @@ module Ssa { } /** Gets the location of this SSA definition. */ - Location getLocation() { none() } + override Location getLocation() { none() } } /** diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll index f30b1769107eb..9e72b0280868c 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll @@ -36,13 +36,15 @@ module BaseSsa { class BasicBlock = ControlFlow::BasicBlock; + class ControlFlowNode = ControlFlow::Node; + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() } BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class ExitBasicBlock = ControlFlow::BasicBlocks::ExitBlock; + class ExitBasicBlock extends BasicBlock, ControlFlow::BasicBlocks::ExitBlock { } class SourceVariable = PreSsa::SimpleLocalScopeVariable; @@ -93,7 +95,7 @@ module BaseSsa { not result instanceof PhiNode } - Location getLocation() { + override Location getLocation() { result = this.getDefinition().getLocation() or exists(Callable c, SsaInput::BasicBlock bb, SsaInput::SourceVariable v | diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll index 8fe50958f20aa..f14e93d7efda4 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -263,7 +263,7 @@ module VariableCapture { private module CaptureInput implements Shared::InputSig { private import csharp as Cs - private import semmle.code.csharp.controlflow.ControlFlowGraph + private import semmle.code.csharp.controlflow.ControlFlowGraph as Cfg private import semmle.code.csharp.controlflow.BasicBlocks as BasicBlocks private import TaintTrackingPrivate as TaintTrackingPrivate @@ -271,6 +271,8 @@ module VariableCapture { Callable getEnclosingCallable() { result = super.getCallable() } } + class ControlFlowNode = Cfg::ControlFlow::Node; + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll index 9757121566b23..1215c564a0628 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll @@ -10,11 +10,13 @@ private import semmle.code.csharp.controlflow.internal.PreSsa private module SsaInput implements SsaImplCommon::InputSig { class BasicBlock = ControlFlow::BasicBlock; + class ControlFlowNode = ControlFlow::Node; + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() } BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class ExitBasicBlock = ControlFlow::BasicBlocks::ExitBlock; + class ExitBasicBlock extends BasicBlock, ControlFlow::BasicBlocks::ExitBlock { } class SourceVariable = Ssa::SourceVariable; @@ -24,7 +26,7 @@ private module SsaInput implements SsaImplCommon::InputSig { * * This includes implicit writes via calls. */ - predicate variableWrite(ControlFlow::BasicBlock bb, int i, Ssa::SourceVariable v, boolean certain) { + predicate variableWrite(BasicBlock bb, int i, Ssa::SourceVariable v, boolean certain) { variableWriteDirect(bb, i, v, certain) or variableWriteQualifier(bb, i, v, certain) @@ -38,7 +40,7 @@ private module SsaInput implements SsaImplCommon::InputSig { * * This includes implicit reads via calls. */ - predicate variableRead(ControlFlow::BasicBlock bb, int i, Ssa::SourceVariable v, boolean certain) { + predicate variableRead(BasicBlock bb, int i, Ssa::SourceVariable v, boolean certain) { variableReadActual(bb, i, v) and certain = true or @@ -1089,7 +1091,7 @@ class DefinitionExt extends Impl::DefinitionExt { override string toString() { result = this.(Ssa::Definition).toString() } /** Gets the location of this definition. */ - Location getLocation() { result = this.(Ssa::Definition).getLocation() } + override Location getLocation() { result = this.(Ssa::Definition).getLocation() } /** Gets the enclosing callable of this definition. */ Callable getEnclosingCallable() { result = this.(Ssa::Definition).getEnclosingCallable() } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll index dd8d1ee3d7674..a8f12a4da51a5 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll @@ -72,11 +72,17 @@ private module CaptureInput implements VariableCapture::InputSig { class BasicBlock instanceof J::BasicBlock { string toString() { result = super.toString() } + ControlFlowNode getNode(int i) { result = super.getNode(i) } + + int length() { result = super.length() } + Callable getEnclosingCallable() { result = super.getEnclosingCallable() } Location getLocation() { result = super.getLocation() } } + class ControlFlowNode = J::ControlFlowNode; + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { bbIDominates(result, bb) } BasicBlock getABasicBlockSuccessor(BasicBlock bb) { diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/VariableCapture.qll b/python/ql/lib/semmle/python/dataflow/new/internal/VariableCapture.qll index f1ba86344a493..86dcee2bfe367 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/VariableCapture.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/VariableCapture.qll @@ -24,6 +24,8 @@ private module CaptureInput implements Shared::InputSig { } class BasicBlock extends PY::BasicBlock { + int length() { result = count(int i | exists(this.getNode(i))) } + Callable getEnclosingCallable() { result = this.getScope() } // Note `PY:BasicBlock` does not have a `getLocation`. @@ -34,6 +36,8 @@ private module CaptureInput implements Shared::InputSig { Location getLocation() { result = super.getNode(0).getLocation() } } + class ControlFlowNode = PY::ControlFlowNode; + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() } BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } diff --git a/ruby/ql/lib/codeql/ruby/dataflow/SSA.qll b/ruby/ql/lib/codeql/ruby/dataflow/SSA.qll index 23623e4d6178d..f21dadb7c5abe 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/SSA.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/SSA.qll @@ -176,9 +176,6 @@ module Ssa { override string toString() { result = this.getControlFlowNode().toString() } - /** Gets the location of this SSA definition. */ - Location getLocation() { result = this.getControlFlowNode().getLocation() } - /** Gets the scope of this SSA definition. */ CfgScope getScope() { result = this.getBasicBlock().getScope() } } diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll index 1a2622454ab19..58af154c694a7 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll @@ -72,54 +72,8 @@ CfgNodes::ExprCfgNode getAPostUpdateNodeForArg(Argument arg) { not exists(getALastEvalNode(result)) } -/** An SSA definition into which another SSA definition may flow. */ -class SsaInputDefinitionExt extends SsaImpl::DefinitionExt { - SsaInputDefinitionExt() { - this instanceof Ssa::PhiNode - or - this instanceof SsaImpl::PhiReadNode - } - - predicate hasInputFromBlock(SsaImpl::DefinitionExt def, BasicBlock bb, int i, BasicBlock input) { - SsaImpl::lastRefBeforeRedefExt(def, bb, i, input, this) - } -} - /** Provides predicates related to local data flow. */ module LocalFlow { - /** - * Holds if `nodeFrom` is a node for SSA definition `def`, which can reach `next`. - */ - pragma[nomagic] - private predicate localFlowSsaInputFromDef( - SsaDefinitionExtNode nodeFrom, SsaImpl::DefinitionExt def, SsaInputNode nodeTo - ) { - exists(BasicBlock bb, int i, BasicBlock input, SsaInputDefinitionExt next | - next.hasInputFromBlock(def, bb, i, input) and - def = nodeFrom.getDefinitionExt() and - def.definesAt(_, bb, i, _) and - nodeTo = TSsaInputNode(next, input) - ) - } - - /** - * Holds if `nodeFrom` is a last read of SSA definition `def`, which - * can reach `nodeTo`. - */ - pragma[nomagic] - predicate localFlowSsaInputFromRead(SsaImpl::DefinitionExt def, Node nodeFrom, SsaInputNode nodeTo) { - exists( - BasicBlock bb, int i, CfgNodes::ExprCfgNode exprFrom, BasicBlock input, - SsaInputDefinitionExt next - | - next.hasInputFromBlock(def, bb, i, input) and - exprFrom = bb.getNode(i) and - exprFrom.getExpr() instanceof VariableReadAccess and - exprFrom = [nodeFrom.asExpr(), nodeFrom.(PostUpdateNodeImpl).getPreUpdateNode().asExpr()] and - nodeTo = TSsaInputNode(next, input) - ) - } - /** Gets the SSA definition node corresponding to parameter `p`. */ pragma[nomagic] SsaDefinitionExtNode getParameterDefNode(NamedParameter p) { @@ -139,14 +93,6 @@ module LocalFlow { nodeTo.getDefinitionExt() = nodeFrom.(SelfParameterNodeImpl).getSelfDefinition() } - /** - * Holds if there is a local use-use flow step from `nodeFrom` to `nodeTo` - * involving SSA definition `def`. - */ - predicate localSsaFlowStepUseUse(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) { - SsaImpl::adjacentReadPairExt(def, nodeFrom.asExpr(), nodeTo.asExpr()) - } - /** * Holds if SSA definition `def` assigns `value` to the underlying variable. * @@ -173,33 +119,6 @@ module LocalFlow { ) } - /** - * Holds if there is a local flow step from `nodeFrom` to `nodeTo` involving - * SSA definition `def`. - */ - pragma[nomagic] - predicate localSsaFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) { - // Flow from assignment into SSA definition - ssaDefAssigns(def, nodeFrom.asExpr()) and - nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def - or - // Flow from SSA definition to first read - def = nodeFrom.(SsaDefinitionExtNode).getDefinitionExt() and - SsaImpl::firstReadExt(def, nodeTo.asExpr()) - or - // Flow from post-update read to next read - localSsaFlowStepUseUse(def, nodeFrom.(PostUpdateNodeImpl).getPreUpdateNode(), nodeTo) - or - // Flow into phi (read) SSA definition node from def - localFlowSsaInputFromDef(nodeFrom, def, nodeTo) - or - nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def and - def = nodeFrom.(SsaInputNode).getDefinitionExt() - or - localFlowSsaParamInput(nodeFrom, nodeTo) and - def = nodeTo.(SsaDefinitionExtNode).getDefinitionExt() - } - pragma[nomagic] predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) { nodeFrom.asExpr() = nodeTo.asExpr().(CfgNodes::ExprNodes::BlockArgumentCfgNode).getValue() @@ -338,6 +257,97 @@ predicate isNonConstantExpr(CfgNodes::ExprCfgNode n) { not n.getExpr() instanceof ConstantAccess } +module SsaFlow { + module Input implements SsaImpl::Impl::DataFlowIntegrationInputSig { + private import ruby::Ast as R + + class Expr extends CfgNodes::ExprCfgNode { + predicate hasCfgNode(SsaImpl::SsaInput::BasicBlock bb, int i) { this = bb.getNode(i) } + } + + /** + * Holds if SSA definition `def` assigns `value` to the underlying variable. + * + * This is either a direct assignment, `x = value`, or an assignment via + * simple pattern matching + * + * ```rb + * case value + * in Foo => x then ... + * in y => then ... + * end + * ``` + */ + predicate ssaDefAssigns(SsaImpl::WriteDefinition def, Expr value) { + def.(Ssa::WriteDefinition).assigns(value) + or + exists(CfgNodes::ExprNodes::CaseExprCfgNode case, CfgNodes::AstCfgNode pattern | + case.getValue() = value and + pattern = case.getBranch(_).(CfgNodes::ExprNodes::InClauseCfgNode).getPattern() + | + def.(Ssa::WriteDefinition).getWriteAccess() = + [pattern, pattern.(CfgNodes::ExprNodes::AsPatternCfgNode).getVariableAccess()] + ) + } + + class Parameter = ParameterNodeImpl; + + predicate ssaDefInitializesParam(SsaImpl::WriteDefinition def, ParameterNodeImpl p) { + exists(BasicBlock bb, int i | + bb.getNode(i).getAstNode() = p.getParameter().(NamedParameter).getDefiningAccess() and + def.definesAt(_, bb, i) + ) + or + def = p.(SelfParameterNodeImpl).getSelfDefinition() + } + } + + module Impl = SsaImpl::Impl::DataFlowIntegration; + + Impl::Node asNode(Node n) { + result = n.(SsaNode).getSsaNode() + or + result.(Impl::ExprNode).getExpr() = n.asExpr() + or + result.(Impl::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr() + or + result.(Impl::ParameterNode).getParameter() = n + } + + predicate localFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) { + Impl::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo)) + } + + module BarrierGuardsInput implements Impl::BarrierGuardsInputSig { + class Guard = CfgNodes::AstCfgNode; + + predicate guardIsLastInBlock(Guard guard, SsaImpl::SsaInput::BasicBlock bb) { + bb.getLastNode() = guard + } + + /** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */ + predicate guardControlsBlock(Guard guard, SsaImpl::SsaInput::BasicBlock bb, boolean branch) { + exists(ConditionBlock conditionBlock, SuccessorTypes::ConditionalSuccessor s | + guard = conditionBlock.getLastNode() and + s.getValue() = branch and + conditionBlock.controls(bb, s) + ) + } + + /** Gets an immediate conditional successor of basic block `bb`, if any. */ + SsaImpl::SsaInput::BasicBlock getAConditionalBasicBlockSuccessor( + SsaImpl::SsaInput::BasicBlock bb, boolean branch + ) { + exists(SuccessorTypes::ConditionalSuccessor s | + result = bb.getASuccessor(s) and + s.getValue() = branch + ) + } + } + + module BarrierGuardsImpl = Impl::BarrierGuards; +} + /** Provides logic related to captured variables. */ module VariableCapture { private import codeql.dataflow.VariableCapture as Shared @@ -353,7 +363,7 @@ module VariableCapture { private module CaptureInput implements Shared::InputSig { private import ruby as R - private import codeql.ruby.controlflow.ControlFlowGraph + private import codeql.ruby.controlflow.ControlFlowGraph as Cfg private import codeql.ruby.controlflow.BasicBlocks as BasicBlocks private import TaintTrackingPrivate as TaintTrackingPrivate @@ -361,6 +371,8 @@ module VariableCapture { Callable getEnclosingCallable() { result = this.getScope() } } + class ControlFlowNode = Cfg::CfgNode; + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() } @@ -537,10 +549,7 @@ private module Cached { newtype TNode = TExprNode(CfgNodes::ExprCfgNode n) or TReturningNode(CfgNodes::ReturningCfgNode n) { exists(n.getReturnedValueNode()) } or - TSsaDefinitionExtNode(SsaImpl::DefinitionExt def) or - TSsaInputNode(SsaInputDefinitionExt def, BasicBlock input) { - def.hasInputFromBlock(_, _, _, input) - } or + TSsaNode(SsaFlow::Impl::SsaNode node) or TCapturedVariableNode(VariableCapture::CapturedVariable v) or TNormalParameterNode(Parameter p) { p instanceof SimpleParameter or @@ -619,14 +628,8 @@ private module Cached { or exists(SsaImpl::DefinitionExt def | // captured variables are handled by the shared `VariableCapture` library - not def instanceof VariableCapture::CapturedSsaDefinitionExt - | - LocalFlow::localSsaFlowStep(def, nodeFrom, nodeTo) - or - LocalFlow::localSsaFlowStepUseUse(def, nodeFrom, nodeTo) and - not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _) - or - LocalFlow::localFlowSsaInputFromRead(def, nodeFrom, nodeTo) and + not def instanceof VariableCapture::CapturedSsaDefinitionExt and + SsaFlow::localFlowStep(def, nodeFrom, nodeTo) and not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _) ) or @@ -642,11 +645,7 @@ private module Cached { predicate localFlowStepImpl(Node nodeFrom, Node nodeTo) { LocalFlow::localFlowStepCommon(nodeFrom, nodeTo) or - LocalFlow::localSsaFlowStep(_, nodeFrom, nodeTo) - or - LocalFlow::localSsaFlowStepUseUse(_, nodeFrom, nodeTo) - or - LocalFlow::localFlowSsaInputFromRead(_, nodeFrom, nodeTo) + SsaFlow::localFlowStep(_, nodeFrom, nodeTo) or // Simple flow through library code is included in the exposed local // step relation, even though flow is technically inter-procedural @@ -660,11 +659,7 @@ private module Cached { predicate localFlowStepTypeTracker(Node nodeFrom, Node nodeTo) { LocalFlow::localFlowStepCommon(nodeFrom, nodeTo) or - LocalFlow::localSsaFlowStep(_, nodeFrom, nodeTo) - or - LocalFlow::localSsaFlowStepUseUse(_, nodeFrom, nodeTo) - or - LocalFlow::localFlowSsaInputFromRead(_, nodeFrom, nodeTo) + SsaFlow::localFlowStep(_, nodeFrom, nodeTo) or VariableCapture::flowInsensitiveStep(nodeFrom, nodeTo) or @@ -864,14 +859,28 @@ predicate nodeIsHidden(Node n) { } /** An SSA definition, viewed as a node in a data flow graph. */ -class SsaDefinitionExtNode extends NodeImpl, TSsaDefinitionExtNode { +abstract class SsaNode extends NodeImpl, TSsaNode { + SsaFlow::Impl::SsaNode node; SsaImpl::DefinitionExt def; - SsaDefinitionExtNode() { this = TSsaDefinitionExtNode(def) } + SsaNode() { + this = TSsaNode(node) and + def = node.getDefinitionExt() + } + + SsaFlow::Impl::SsaNode getSsaNode() { result = node } - /** Gets the underlying SSA definition. */ SsaImpl::DefinitionExt getDefinitionExt() { result = def } + override Location getLocationImpl() { result = node.getLocation() } + + override string toStringImpl() { result = node.toString() } +} + +/** An SSA definition, viewed as a node in a data flow graph. */ +class SsaDefinitionExtNode extends SsaNode { + override SsaFlow::Impl::SsaDefinitionExtNode node; + /** Gets the underlying variable. */ Variable getVariable() { result = def.getSourceVariable() } @@ -883,10 +892,16 @@ class SsaDefinitionExtNode extends NodeImpl, TSsaDefinitionExtNode { } override CfgScope getCfgScope() { result = def.getBasicBlock().getScope() } +} + +class SsaDefinitionNodeImpl extends SsaDefinitionExtNode { + Ssa::Definition ssaDef; - override Location getLocationImpl() { result = def.getLocation() } + SsaDefinitionNodeImpl() { ssaDef = def } - override string toStringImpl() { result = def.toString() } + override Location getLocationImpl() { result = ssaDef.getLocation() } + + override string toStringImpl() { result = ssaDef.toString() } } /** @@ -924,20 +939,20 @@ class SsaDefinitionExtNode extends NodeImpl, TSsaDefinitionExtNode { * * both inputs into the phi read node after the outer condition are guarded. */ -class SsaInputNode extends NodeImpl, TSsaInputNode { - SsaImpl::DefinitionExt def; +class SsaInputNode extends SsaNode { + override SsaFlow::Impl::SsaInputNode node; BasicBlock input; - SsaInputNode() { this = TSsaInputNode(def, input) } + SsaInputNode() { node.hasInputFromBlock(def, input) } - /** Gets the underlying SSA definition. */ - SsaImpl::DefinitionExt getDefinitionExt() { result = def } + predicate isInputInto(BasicBlock bb, SsaImpl::DefinitionExt target) { + bb = input and + target = def + } override CfgScope getCfgScope() { result = input.getScope() } - - override Location getLocationImpl() { result = input.getLastNode().getLocation() } - - override string toStringImpl() { result = "[input] " + def } + // override Location getLocationImpl() { result = input.getLastNode().getLocation() } + // override string toStringImpl() { result = "[input] " + def } } /** An SSA definition for a `self` variable. */ diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll index d4e07e0653e10..50fc07a7fd08d 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll @@ -360,16 +360,12 @@ class PostUpdateNode extends Node { } /** An SSA definition, viewed as a node in a data flow graph. */ -class SsaDefinitionNode extends Node instanceof SsaDefinitionExtNode { - Ssa::Definition def; - - SsaDefinitionNode() { this = TSsaDefinitionExtNode(def) } - +class SsaDefinitionNode extends Node instanceof SsaDefinitionNodeImpl { /** Gets the underlying SSA definition. */ - Ssa::Definition getDefinition() { result = def } + Ssa::Definition getDefinition() { result = super.getDefinitionExt() } /** Gets the underlying variable. */ - Variable getVariable() { result = def.getSourceVariable() } + Variable getVariable() { result = this.getDefinition().getSourceVariable() } } cached @@ -872,55 +868,6 @@ private predicate sameSourceVariable(Ssa::Definition def1, Ssa::Definition def2) module BarrierGuard { private import SsaImpl as SsaImpl - pragma[nomagic] - private predicate guardChecksSsaDef(CfgNodes::AstCfgNode g, boolean branch, Ssa::Definition def) { - guardChecks(g, def.getARead(), branch) - } - - pragma[nomagic] - private predicate guardControlsSsaRead( - CfgNodes::AstCfgNode g, boolean branch, Ssa::Definition def, Node n - ) { - def.getARead() = n.asExpr() and - guardControlsBlock(g, n.asExpr().getBasicBlock(), branch) - } - - pragma[nomagic] - private predicate guardControlsPhiInput( - CfgNodes::AstCfgNode g, boolean branch, Ssa::Definition def, BasicBlock input, - SsaInputDefinitionExt phi - ) { - phi.hasInputFromBlock(def, _, _, input) and - ( - guardControlsBlock(g, input, branch) - or - exists(SuccessorTypes::ConditionalSuccessor s | - g = input.getLastNode() and - s.getValue() = branch and - input.getASuccessor(s) = phi.getBasicBlock() - ) - ) - } - - /** Gets a node that is safely guarded by the given guard check. */ - Node getABarrierNode() { - exists(CfgNodes::AstCfgNode g, boolean branch, Ssa::Definition def | - guardChecksSsaDef(g, branch, def) and - guardControlsSsaRead(g, branch, def, result) - ) - or - exists( - CfgNodes::AstCfgNode g, boolean branch, Ssa::Definition def, BasicBlock input, - SsaInputDefinitionExt phi - | - guardChecksSsaDef(g, branch, def) and - guardControlsPhiInput(g, branch, def, input, phi) and - result = TSsaInputNode(phi, input) - ) - or - result.asExpr() = getAMaybeGuardedCapturedDef().getARead() - } - /** * Gets an implicit entry definition for a captured variable that * may be guarded, because a call to the capturing callable is guarded. @@ -935,20 +882,23 @@ module BarrierGuard { | def.getARead() = testedNode and guardChecks(g, testedNode, branch) and - guardControlsBlock(g, call.getBasicBlock(), branch) and + SsaFlow::BarrierGuardsInput::guardControlsBlock(g, call.getBasicBlock(), branch) and result.getBasicBlock().getScope() = call.getExpr().(MethodCall).getBlock() and sameSourceVariable(def, result) ) } -} -/** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */ -private predicate guardControlsBlock(CfgNodes::AstCfgNode guard, BasicBlock bb, boolean branch) { - exists(ConditionBlock conditionBlock, SuccessorTypes::ConditionalSuccessor s | - guard = conditionBlock.getLastNode() and - s.getValue() = branch and - conditionBlock.controls(bb, s) - ) + private predicate guardChecksAdj(CfgNodes::AstCfgNode g, SsaFlow::Input::Expr e, boolean branch) { + guardChecks(g, e, branch) + } + + /** Gets a node that is safely guarded by the given guard check. */ + Node getABarrierNode() { + SsaFlow::asNode(result) = + SsaFlow::BarrierGuardsImpl::BarrierGuard::getABarrierNode() + or + result.asExpr() = getAMaybeGuardedCapturedDef().getARead() + } } /** diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll index 1490a8c7ca332..477110dbe4c6f 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll @@ -6,16 +6,19 @@ private import codeql.ruby.dataflow.SSA private import codeql.ruby.ast.Variable private import Cfg::CfgNodes::ExprNodes -private module SsaInput implements SsaImplCommon::InputSig { +module SsaInput implements SsaImplCommon::InputSig { + private import codeql.ruby.controlflow.ControlFlowGraph as Cfg private import codeql.ruby.controlflow.BasicBlocks as BasicBlocks class BasicBlock = BasicBlocks::BasicBlock; + class ControlFlowNode = Cfg::CfgNode; + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() } BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class ExitBasicBlock = BasicBlocks::ExitBasicBlock; + class ExitBasicBlock extends BasicBlock, BasicBlocks::ExitBasicBlock { } class SourceVariable = LocalVariable; @@ -62,7 +65,7 @@ private module SsaInput implements SsaImplCommon::InputSig { } } -private import SsaImplCommon::Make as Impl +import SsaImplCommon::Make as Impl class Definition = Impl::Definition; @@ -280,15 +283,6 @@ private predicate adjacentDefSkipUncertainReads( SsaInput::variableRead(bb2, i2, _, true) } -/** Same as `adjacentDefReadExt`, but skips uncertain reads. */ -pragma[nomagic] -private predicate adjacentDefSkipUncertainReadsExt( - DefinitionExt def, SsaInput::BasicBlock bb1, int i1, SsaInput::BasicBlock bb2, int i2 -) { - adjacentDefReachesReadExt(def, bb1, i1, bb2, i2) and - SsaInput::variableRead(bb2, i2, _, true) -} - private predicate adjacentDefReachesUncertainReadExt( DefinitionExt def, SsaInput::BasicBlock bb1, int i1, SsaInput::BasicBlock bb2, int i2 ) { @@ -390,19 +384,6 @@ private module Cached { ) } - /** - * Holds if the value defined at SSA definition `def` can reach a read at `read`, - * without passing through any other non-pseudo read. - */ - cached - predicate firstReadExt(DefinitionExt def, VariableReadAccessCfgNode read) { - exists(Cfg::BasicBlock bb1, int i1, Cfg::BasicBlock bb2, int i2 | - def.definesAt(_, bb1, i1, _) and - adjacentDefSkipUncertainReadsExt(def, bb1, i1, bb2, i2) and - read = bb2.getNode(i2) - ) - } - /** * Holds if the read at `read2` is a read of the same SSA definition `def` * as the read at `read1`, and `read2` can be reached from `read1` without @@ -420,23 +401,6 @@ private module Cached { ) } - /** - * Holds if the read at `read2` is a read of the same SSA definition `def` - * as the read at `read1`, and `read2` can be reached from `read1` without - * passing through another non-pseudo read. - */ - cached - predicate adjacentReadPairExt( - DefinitionExt def, VariableReadAccessCfgNode read1, VariableReadAccessCfgNode read2 - ) { - exists(Cfg::BasicBlock bb1, int i1, Cfg::BasicBlock bb2, int i2 | - read1 = bb1.getNode(i1) and - variableReadActual(bb1, i1, _) and - adjacentDefSkipUncertainReadsExt(def, bb1, i1, bb2, i2) and - read2 = bb2.getNode(i2) - ) - } - /** * Holds if the read of `def` at `read` may be a last read. That is, `read` * can either reach another definition of the underlying source variable or @@ -451,28 +415,6 @@ private module Cached { ) } - /** - * Holds if the reference to `def` at index `i` in basic block `bb` can reach - * another definition `next` of the same underlying source variable, without - * passing through another write or non-pseudo read. - * - * The reference is either a read of `def` or `def` itself. - */ - cached - predicate lastRefBeforeRedefExt( - DefinitionExt def, Cfg::BasicBlock bb, int i, Cfg::BasicBlock input, DefinitionExt next - ) { - exists(LocalVariable v | - Impl::lastRefRedefExt(def, v, bb, i, input, next) and - not SsaInput::variableRead(bb, i, v, false) - ) - or - exists(SsaInput::BasicBlock bb0, int i0 | - Impl::lastRefRedefExt(def, _, bb0, i0, input, next) and - adjacentDefReachesUncertainReadExt(def, bb, i, bb0, i0) - ) - } - cached Definition uncertainWriteDefinitionInput(UncertainWriteDefinition def) { Impl::uncertainWriteDefinitionInput(def, result) @@ -494,8 +436,7 @@ class DefinitionExt extends Impl::DefinitionExt { override string toString() { result = this.(Ssa::Definition).toString() } - /** Gets the location of this definition. */ - Location getLocation() { result = this.(Ssa::Definition).getLocation() } + override Location getLocation() { result = this.(Ssa::Definition).getLocation() } } /** @@ -506,5 +447,5 @@ class DefinitionExt extends Impl::DefinitionExt { class PhiReadNode extends DefinitionExt, Impl::PhiReadNode { override string toString() { result = "SSA phi read(" + this.getSourceVariable() + ")" } - override Location getLocation() { result = this.getBasicBlock().getLocation() } + override Location getLocation() { result = Impl::PhiReadNode.super.getLocation() } } diff --git a/shared/dataflow/codeql/dataflow/VariableCapture.qll b/shared/dataflow/codeql/dataflow/VariableCapture.qll index c48b46e8a7ba0..73f22fced7375 100644 --- a/shared/dataflow/codeql/dataflow/VariableCapture.qll +++ b/shared/dataflow/codeql/dataflow/VariableCapture.qll @@ -17,6 +17,12 @@ signature module InputSig { /** Gets a textual representation of this basic block. */ string toString(); + /** Gets the `i`th node in this basic block. */ + ControlFlowNode getNode(int i); + + /** Gets the length of this basic block. */ + int length(); + /** Gets the enclosing callable. */ Callable getEnclosingCallable(); @@ -24,6 +30,15 @@ signature module InputSig { Location getLocation(); } + /** A control flow node. */ + class ControlFlowNode { + /** Gets a textual representation of this control flow node. */ + string toString(); + + /** Gets the location of this control flow node. */ + Location getLocation(); + } + /** * Gets the basic block that immediately dominates basic block `bb`, if any. * @@ -672,6 +687,8 @@ module Flow Input> implements OutputSig private module CaptureSsaInput implements Ssa::InputSig { final class BasicBlock = Input::BasicBlock; + final class ControlFlowNode = Input::ControlFlowNode; + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = Input::getImmediateBasicBlockDominator(bb) } @@ -717,10 +734,10 @@ module Flow Input> implements OutputSig TSynthPhi(CaptureSsa::DefinitionExt phi) { phi instanceof CaptureSsa::PhiNode or phi instanceof CaptureSsa::PhiReadNode } or - TExprNode(Expr expr, boolean isPost) { - expr instanceof VariableRead and isPost = [false, true] + TExprNode(Expr expr, Boolean isPost) { + expr instanceof VariableRead or - synthRead(_, _, _, _, expr) and isPost = [false, true] + synthRead(_, _, _, _, expr) } or TParamNode(CapturedParameter p) or TThisParamNode(Callable c) { captureAccess(_, c) } or diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index ee397375e655e..fbd0f191af589 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -15,10 +15,25 @@ signature module InputSig { /** Gets a textual representation of this basic block. */ string toString(); + /** Gets the `i`th node in this basic block. */ + ControlFlowNode getNode(int i); + + /** Gets the length of this basic block. */ + int length(); + /** Gets the location of this basic block. */ Location getLocation(); } + /** A control flow node. */ + class ControlFlowNode { + /** Gets a textual representation of this control flow node. */ + string toString(); + + /** Gets the location of this control flow node. */ + Location getLocation(); + } + /** * Gets the basic block that immediately dominates basic block `bb`, if any. * @@ -905,6 +920,13 @@ module Make Input> { /** Gets a textual representation of this SSA definition. */ string toString() { result = "SSA def(" + this.getSourceVariable() + ")" } + + /** Gets the location of this SSA definition. */ + Location getLocation() { + exists(BasicBlock bb, int i | this.definesAt(_, bb, i) | + if i = -1 then result = bb.getLocation() else result = bb.getNode(i).getLocation() + ) + } } /** An SSA definition that corresponds to a write. */ @@ -961,6 +983,13 @@ module Make Input> { /** Gets a textual representation of this SSA definition. */ string toString() { result = this.(Definition).toString() } + + /** Gets the location of this SSA definition. */ + Location getLocation() { + exists(BasicBlock bb, int i | this.definesAt(_, bb, i, _) | + if i = -1 then result = bb.getLocation() else result = bb.getNode(i).getLocation() + ) + } } /** @@ -1134,4 +1163,470 @@ module Make Input> { ) } } + + /** Provides the input to `DataFlowIntegration`. */ + signature module DataFlowIntegrationInputSig { + /** + * An expression with a value. That is, we expect these expressions to be + * represented in the data flow graph. + */ + class Expr { + /** Gets a textual representation of this expression. */ + string toString(); + + /** Holds if the `i`th node of basic block `bb` evaluates this expression. */ + predicate hasCfgNode(BasicBlock bb, int i); + } + + predicate ssaDefAssigns(WriteDefinition def, Expr value); + + /** A parameter. */ + class Parameter { + /** Gets a textual representation of this expression. */ + string toString(); + + /** Gets the location of this parameter. */ + Location getLocation(); + } + + predicate ssaDefInitializesParam(WriteDefinition def, Parameter p); + } + + module DataFlowIntegration { + private import codeql.util.Boolean + + final private class DefinitionExtFinal = DefinitionExt; + + pragma[noinline] + private predicate adjacentDefReadExt0( + DefinitionExt def, BasicBlock bb1, int i1, BasicBlock bb2, int i2, SourceVariable v + ) { + adjacentDefReadExt(def, _, bb1, i1, bb2, i2) and + v = def.getSourceVariable() + } + + private predicate adjacentDefReachesReadExt( + DefinitionExt def, SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 + ) { + adjacentDefReadExt0(def, bb1, i1, bb2, i2, v) and + ( + def.definesAt(v, bb1, i1, _) + or + variableRead(bb1, i1, v, true) + ) + or + exists(BasicBlock bb3, int i3 | + adjacentDefReachesReadExt(def, v, bb1, i1, bb3, i3) and + variableRead(bb3, i3, v, false) and + adjacentDefReadExt(def, v, bb3, i3, bb2, i2) + ) + } + + private predicate adjacentDefReachesUncertainReadExt( + DefinitionExt def, SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 + ) { + adjacentDefReachesReadExt(def, v, bb1, i1, bb2, i2) and + variableRead(bb2, i2, v, false) + } + + /** + * Holds if the reference to `def` at index `i` in basic block `bb` can reach + * another definition `next` of the same underlying source variable, without + * passing through another write or non-pseudo read. + * + * The reference is either a read of `def` or `def` itself. + */ + private predicate lastRefBeforeRedefExt( + DefinitionExt def, SourceVariable v, BasicBlock bb, int i, BasicBlock input, + DefinitionExt next + ) { + lastRefRedefExt(def, v, bb, i, input, next) and + not variableRead(bb, i, v, false) + or + exists(BasicBlock bb0, int i0 | + lastRefRedefExt(def, v, bb0, i0, input, next) and + adjacentDefReachesUncertainReadExt(def, v, bb, i, bb0, i0) + ) + } + + /** Same as `adjacentDefReadExt`, but skips uncertain reads. */ + pragma[nomagic] + private predicate adjacentDefSkipUncertainReadsExt( + DefinitionExt def, SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 + ) { + adjacentDefReachesReadExt(def, v, bb1, i1, bb2, i2) and + variableRead(bb2, i2, v, true) + } + + private predicate adjacentReadPairExt(DefinitionExt def, ReadNode read1, ReadNode read2) { + exists(SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 | + read1.readsAt(bb1, i1, v) and + adjacentDefSkipUncertainReadsExt(def, v, bb1, i1, bb2, i2) and + read2.readsAt(bb2, i2, v) + ) + } + + /** An SSA definition into which another SSA definition may flow. */ + private class SsaInputDefinitionExt extends DefinitionExtFinal { + SsaInputDefinitionExt() { + this instanceof PhiNode + or + this instanceof PhiReadNode + } + + predicate hasInputFromBlock( + DefinitionExt def, SourceVariable v, BasicBlock bb, int i, BasicBlock input + ) { + lastRefBeforeRedefExt(def, v, bb, i, input, this) + } + } + + private newtype TNode = + TParamNode(DfInput::Parameter p) or + TExprNode(DfInput::Expr e, Boolean isPost) { + exists(BasicBlock bb, int i | + variableRead(bb, i, _, true) and + e.hasCfgNode(bb, i) + ) + or + isPost = false + } or + TSsaDefinitionNode(DefinitionExt def) or + TSsaInputNode(SsaInputDefinitionExt def, BasicBlock input) { + def.hasInputFromBlock(_, _, _, _, input) + } + + /** + * A data flow node that we need to reference in the step relations for + * captured variables. + * + * Note that only the `SynthesizedCaptureNode` subclass is expected to be + * added as additional nodes in `DataFlow::Node`. The other subclasses are + * expected to already be present and are included here in order to reference + * them in the step relations. + */ + abstract private class NodeImpl extends TNode { + abstract Location getLocation(); + + abstract string toString(); + } + + final class Node = NodeImpl; + + private class ParameterNodeImpl extends NodeImpl, TParamNode { + private DfInput::Parameter p; + + ParameterNodeImpl() { this = TParamNode(p) } + + DfInput::Parameter getParameter() { result = p } + + override string toString() { result = p.toString() } + + override Location getLocation() { result = p.getLocation() } + } + + final class ParameterNode = ParameterNodeImpl; + + abstract private class ExprNodePreOrPostImpl extends NodeImpl, TExprNode { + DfInput::Expr e; + boolean isPost; + + ExprNodePreOrPostImpl() { this = TExprNode(e, isPost) } + + DfInput::Expr getExpr() { result = e } + + predicate isPostUpdate() { isPost = true } + + override Location getLocation() { + exists(BasicBlock bb, int i | + e.hasCfgNode(bb, i) and + result = bb.getNode(i).getLocation() + ) + } + } + + final class ExprNodePreOrPost = ExprNodePreOrPostImpl; + + private class ExprNodeImpl extends ExprNodePreOrPostImpl { + ExprNodeImpl() { isPost = false } + + override string toString() { result = e.toString() } + } + + final class ExprNode = ExprNodeImpl; + + private class ExprPostUpdateNodeImpl extends ExprNodePreOrPostImpl { + ExprPostUpdateNodeImpl() { isPost = true } + + ExprNode getPreUpdateNode() { result = TExprNode(e, false) } + + override string toString() { result = e.toString() + " [postupdate]" } + } + + final class ExprPostUpdateNode = ExprPostUpdateNodeImpl; + + private class ReadNodeImpl extends ExprNodeImpl { + private BasicBlock bb_; + private int i_; + private SourceVariable v_; + + ReadNodeImpl() { + variableRead(bb_, i_, v_, true) and + this.getExpr().hasCfgNode(bb_, i_) + } + + SourceVariable getVariable() { result = v_ } + + pragma[nomagic] + predicate readsAt(BasicBlock bb, int i, SourceVariable v) { + bb = bb_ and + i = i_ and + v = v_ + } + } + + final class ReadNode = ReadNodeImpl; + + /** + * A synthesized data flow node representing the storage of a captured + * variable. + */ + abstract private class SsaNodeImpl extends NodeImpl { + abstract DefinitionExt getDefinitionExt(); + } + + final class SsaNode = SsaNodeImpl; + + /** An SSA definition, viewed as a node in a data flow graph. */ + private class SsaDefinitionExtNodeImpl extends SsaNodeImpl, TSsaDefinitionNode { + private DefinitionExt def; + + SsaDefinitionExtNodeImpl() { this = TSsaDefinitionNode(def) } + + override DefinitionExt getDefinitionExt() { result = def } + + override Location getLocation() { result = def.getLocation() } + + override string toString() { result = def.toString() } + } + + final class SsaDefinitionExtNode = SsaDefinitionExtNodeImpl; + + /** + * A node that represents an input to an SSA phi (read) definition. + * + * This allows for barrier guards to filter input to phi nodes. For example, in + * + * ```rb + * x = taint + * if x != "safe" then + * x = "safe" + * end + * sink x + * ``` + * + * the `false` edge out of `x != "safe"` guards the input from `x = taint` into the + * `phi` node after the condition. + * + * It is also relevant to filter input into phi read nodes: + * + * ```rb + * x = taint + * if b then + * if x != "safe1" then + * return + * end + * else + * if x != "safe2" then + * return + * end + * end + * + * sink x + * ``` + * + * both inputs into the phi read node after the outer condition are guarded. + */ + private class SsaInputNodeImpl extends SsaNodeImpl, TSsaInputNode { + private SsaInputDefinitionExt def_; + private BasicBlock input_; + + SsaInputNodeImpl() { this = TSsaInputNode(def_, input_) } + + predicate hasInputFromBlock(DefinitionExt def, BasicBlock input) { + def = def_ and + input = input_ + } + + override SsaInputDefinitionExt getDefinitionExt() { result = def_ } + + override Location getLocation() { result = input_.getNode(input_.length() - 1).getLocation() } + + override string toString() { result = "[input] " + def_.toString() } + } + + final class SsaInputNode = SsaInputNodeImpl; + + /** + * Holds if `nodeFrom` is a node for SSA definition `def`, which can reach `next`. + */ + pragma[nomagic] + private predicate localFlowSsaInputFromDef( + DefinitionExt def, SsaDefinitionExtNode nodeFrom, SsaInputNode nodeTo + ) { + exists(SourceVariable v, BasicBlock bb, int i, BasicBlock input, SsaInputDefinitionExt next | + next.hasInputFromBlock(def, v, bb, i, input) and + def = nodeFrom.getDefinitionExt() and + def.definesAt(v, bb, i, _) and + nodeTo = TSsaInputNode(next, input) + ) + } + + /** + * Holds if `nodeFrom` is a last read of SSA definition `def`, which + * can reach `nodeTo`. + */ + pragma[nomagic] + private predicate localFlowSsaInputFromRead( + DefinitionExt def, ReadNode nodeFrom, SsaInputNode nodeTo + ) { + exists(SourceVariable v, BasicBlock bb, int i, BasicBlock input, SsaInputDefinitionExt next | + next.hasInputFromBlock(def, v, bb, i, input) and + nodeFrom.readsAt(bb, i, v) and + nodeTo = TSsaInputNode(next, input) + ) + } + + private predicate firstReadExt(DefinitionExt def, ReadNode read) { + exists(SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 | + def.definesAt(v, bb1, i1, _) and + adjacentDefSkipUncertainReadsExt(def, v, bb1, i1, bb2, i2) and + read.readsAt(bb2, i2, v) + ) + } + + /** + * Holds if there is a local use-use flow step from `nodeFrom` to `nodeTo` + * involving SSA definition `def`. + */ + private predicate localSsaFlowStepUseUse(DefinitionExt def, ReadNode nodeFrom, ReadNode nodeTo) { + adjacentReadPairExt(def, nodeFrom, nodeTo) + } + + predicate localFlowStep(DefinitionExt def, Node nodeFrom, Node nodeTo) { + // Flow from assignment into SSA definition + DfInput::ssaDefAssigns(def, nodeFrom.(ExprNode).getExpr()) and + nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def + or + // Flow from SSA definition to first read + def = nodeFrom.(SsaDefinitionExtNode).getDefinitionExt() and + firstReadExt(def, nodeTo) + or + // Flow from (post-update) read to next read + localSsaFlowStepUseUse(def, [nodeFrom, nodeFrom.(ExprPostUpdateNode).getPreUpdateNode()], + nodeTo) + or + // Flow into phi (read) SSA definition node from def + localFlowSsaInputFromDef(def, nodeFrom, nodeTo) + or + // Flow from input node to def + nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def and + def = nodeFrom.(SsaInputNode).getDefinitionExt() + or + // Flow from parameter into entry definition + DfInput::ssaDefInitializesParam(nodeTo.(SsaDefinitionExtNode).getDefinitionExt(), + nodeFrom.(ParameterNode).getParameter()) + or + localFlowSsaInputFromRead(def, [nodeFrom, nodeFrom.(ExprPostUpdateNode).getPreUpdateNode()], + nodeTo) + } + + signature module BarrierGuardsInputSig { + class Guard; + + predicate guardIsLastInBlock(Guard guard, BasicBlock bb); + + /** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */ + predicate guardControlsBlock(Guard guard, BasicBlock bb, boolean branch); + + /** Gets an immediate conditional successor of basic block `bb`, if any. */ + BasicBlock getAConditionalBasicBlockSuccessor(BasicBlock bb, boolean branch); + } + + module BarrierGuards { + /** + * Holds if the guard `g` validates the expression `e` upon evaluating to `branch`. + * + * The expression `e` is expected to be a syntactic part of the guard `g`. + * For example, the guard `g` might be a call `isSafe(x)` and the expression `e` + * the argument `x`. + */ + signature predicate guardChecksSig(BgInput::Guard g, DfInput::Expr e, boolean branch); + + /** + * Provides a set of barrier nodes for a guard that validates an expression. + * + * This is expected to be used in `isBarrier`/`isSanitizer` definitions + * in data flow and taint tracking. + */ + module BarrierGuard { + pragma[nomagic] + private DfInput::Expr getARead(Definition def) { + exists(SourceVariable v, BasicBlock bb, int i | + ssaDefReachesRead(v, def, bb, i) and + variableRead(bb, i, v, true) and + result.hasCfgNode(bb, i) + ) + } + + pragma[nomagic] + private predicate guardChecksSsaDef(BgInput::Guard g, boolean branch, Definition def) { + guardChecks(g, getARead(def), branch) + } + + pragma[nomagic] + private predicate guardControlsSsaRead( + BgInput::Guard g, boolean branch, Definition def, ExprNode n + ) { + exists(BasicBlock bb, DfInput::Expr e | + e = n.getExpr() and + getARead(def) = e and + BgInput::guardControlsBlock(g, bb, branch) and + e.hasCfgNode(bb, _) + ) + } + + pragma[nomagic] + private predicate guardControlsPhiInput( + BgInput::Guard g, boolean branch, Definition def, BasicBlock input, + SsaInputDefinitionExt phi + ) { + phi.hasInputFromBlock(def, _, _, _, input) and + ( + BgInput::guardControlsBlock(g, input, branch) + or + BgInput::guardIsLastInBlock(g, input) and + BgInput::getAConditionalBasicBlockSuccessor(input, branch) = phi.getBasicBlock() + ) + } + + /** Gets a node that is safely guarded by the given guard check. */ + Node getABarrierNode() { + exists(BgInput::Guard g, boolean branch, Definition def | + guardChecksSsaDef(g, branch, def) and + guardControlsSsaRead(g, branch, def, result) + ) + or + exists( + BgInput::Guard g, boolean branch, Definition def, BasicBlock input, + SsaInputDefinitionExt phi + | + guardChecksSsaDef(g, branch, def) and + guardControlsPhiInput(g, branch, def, input, phi) and + result.(SsaInputNode).hasInputFromBlock(phi, input) + ) + } + } + } + } } diff --git a/swift/ql/lib/codeql/swift/dataflow/Ssa.qll b/swift/ql/lib/codeql/swift/dataflow/Ssa.qll index 7a8c5a300d004..971cc8cc705cb 100644 --- a/swift/ql/lib/codeql/swift/dataflow/Ssa.qll +++ b/swift/ql/lib/codeql/swift/dataflow/Ssa.qll @@ -8,18 +8,20 @@ module Ssa { private module SsaInput implements SsaImplCommon::InputSig { private import internal.DataFlowPrivate - private import codeql.swift.controlflow.ControlFlowGraph + private import codeql.swift.controlflow.ControlFlowGraph as Cfg private import codeql.swift.controlflow.CfgNodes class BasicBlock = BasicBlocks::BasicBlock; + class ControlFlowNode = Cfg::ControlFlowNode; + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() } BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class ExitBasicBlock = BasicBlocks::ExitBasicBlock; + class ExitBasicBlock extends BasicBlock, BasicBlocks::ExitBasicBlock { } private newtype TSourceVariable = TNormalSourceVariable(VarDecl v) or @@ -138,7 +140,7 @@ module Ssa { cached class Definition extends SsaImpl::Definition { cached - Location getLocation() { none() } + override Location getLocation() { none() } cached ControlFlowNode getARead() { diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll index eb2caea8c2f80..de54324e005be 100644 --- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll +++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll @@ -886,16 +886,23 @@ private predicate closureFlowStep(CaptureInput::Expr e1, CaptureInput::Expr e2) private module CaptureInput implements VariableCapture::InputSig { private import swift as S + private import codeql.swift.controlflow.ControlFlowGraph as Cfg private import codeql.swift.controlflow.BasicBlocks as B class BasicBlock instanceof B::BasicBlock { string toString() { result = super.toString() } + ControlFlowNode getNode(int i) { result = super.getNode(i) } + + int length() { result = super.length() } + Callable getEnclosingCallable() { result = super.getScope() } Location getLocation() { result = super.getLocation() } } + class ControlFlowNode = Cfg::ControlFlowNode; + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.(B::BasicBlock).immediatelyDominates(bb) }