From 16cb5c3b3f31d2b36f4f3a13d858a3ad239bd076 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Tue, 19 Nov 2024 15:01:32 +0100 Subject: [PATCH] Rust: Use nodes from `CfgNodes.qll` in `DataFlowImpl.qll` --- rust/ql/lib/codeql/rust/dataflow/Ssa.qll | 9 +-- .../rust/dataflow/internal/DataFlowImpl.qll | 57 ++++++++----------- .../codeql/rust/dataflow/internal/SsaImpl.qll | 4 +- .../dataflow/local/DataFlowStep.expected | 4 +- rust/ql/test/utils/InlineFlowTest.qll | 9 +-- 5 files changed, 37 insertions(+), 46 deletions(-) diff --git a/rust/ql/lib/codeql/rust/dataflow/Ssa.qll b/rust/ql/lib/codeql/rust/dataflow/Ssa.qll index ff1aae058aaac..158128a12a9ad 100644 --- a/rust/ql/lib/codeql/rust/dataflow/Ssa.qll +++ b/rust/ql/lib/codeql/rust/dataflow/Ssa.qll @@ -9,6 +9,7 @@ module Ssa { private import rust private import codeql.rust.controlflow.BasicBlocks private import codeql.rust.controlflow.ControlFlowGraph + private import codeql.rust.controlflow.CfgNodes private import codeql.rust.controlflow.internal.ControlFlowGraphImpl as CfgImpl private import internal.SsaImpl as SsaImpl @@ -221,11 +222,11 @@ module Ssa { * end * ``` */ - predicate assigns(CfgNode value) { - exists(AssignmentExpr ae, BasicBlock bb, int i | + predicate assigns(ExprCfgNode value) { + exists(AssignmentExprCfgNode ae, BasicBlock bb, int i | this.definesAt(_, bb, i) and - ae.getLhs() = bb.getNode(i).getAstNode() and - value.getAstNode() = ae.getRhs() + ae.getLhs() = bb.getNode(i) and + value = ae.getRhs() ) } diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll index def5780537330..84d6ddd196c01 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll @@ -48,7 +48,7 @@ final class DataFlowCall extends TDataFlowCall { MethodCallExprCfgNode asMethodCallExprCfgNode() { this = TMethodCall(result) } - ExprCfgNode asExprCfgNode() { + CallExprBaseCfgNode asExprCfgNode() { result = this.asCallExprCfgNode() or result = this.asMethodCallExprCfgNode() } @@ -76,7 +76,7 @@ module Node { /** * Gets the expression that corresponds to this node, if any. */ - Expr asExpr() { none() } + ExprCfgNode asExpr() { none() } /** Gets the enclosing callable. */ DataFlowCallable getEnclosingCallable() { result = TCfgScope(this.getCfgScope()) } @@ -115,13 +115,13 @@ module Node { abstract private class AstCfgFlowNode extends Node { AstCfgNode n; - override CfgNode getCfgNode() { result = n } + final override CfgNode getCfgNode() { result = n } - override CfgScope getCfgScope() { result = n.getAstNode().getEnclosingCfgScope() } + final override CfgScope getCfgScope() { result = n.getAstNode().getEnclosingCfgScope() } - override Location getLocation() { result = n.getAstNode().getLocation() } + final override Location getLocation() { result = n.getAstNode().getLocation() } - override string toString() { result = n.getAstNode().toString() } + final override string toString() { result = n.getAstNode().toString() } } /** @@ -137,7 +137,7 @@ module Node { ExprNode() { this = TExprNode(n) } - override Expr asExpr() { result = n.getExpr() } + override ExprCfgNode asExpr() { result = n } } final class PatNode extends AstCfgFlowNode, TPatNode { @@ -159,7 +159,7 @@ module Node { ParameterNode() { this = TParameterNode(n) } /** Gets the parameter in the AST that this node corresponds to. */ - Param getParameter() { result = n.getParam() } + ParamCfgNode getParameter() { result = n } } final class ArgumentNode = NaNode; @@ -197,7 +197,7 @@ module Node { } final private class ExprOutNode extends ExprNode, OutNode { - ExprOutNode() { this.asExpr() instanceof CallExpr } + ExprOutNode() { this.asExpr() instanceof CallExprCfgNode } /** Gets the underlying call CFG node that includes this out node. */ override DataFlowCall getCall() { result.asExprCfgNode() = this.getCfgNode() } @@ -228,13 +228,13 @@ final class Node = Node::Node; module SsaFlow { private module Impl = SsaImpl::DataFlowIntegration; - private Node::ParameterNode toParameterNode(Param p) { result.getParameter() = p } + private Node::ParameterNode toParameterNode(ParamCfgNode p) { result.getParameter() = p } /** Converts a control flow node into an SSA control flow node. */ Impl::Node asNode(Node n) { n = TSsaNode(result) or - result.(Impl::ExprNode).getExpr() = n.(Node::ExprNode).getCfgNode() + result.(Impl::ExprNode).getExpr() = n.asExpr() or n = toParameterNode(result.(Impl::ParameterNode).getParameter()) } @@ -248,29 +248,18 @@ module SsaFlow { } } -/** - * Holds for expressions `e` that evaluate to the value of any last (in - * evaluation order) subexpressions within it. E.g., expressions that propagate - * a values from a subexpression. - * - * For instance, the predicate holds for if expressions as `if b { e1 } else { - * e2 }` evalates to the value of one of the subexpressions `e1` or `e2`. - */ -private predicate propagatesValue(Expr e) { - e instanceof IfExpr or - e instanceof LoopExpr or - e instanceof ReturnExpr or - e instanceof BreakExpr or - e.(BlockExpr).getStmtList().hasTailExpr() or - e instanceof MatchExpr -} - /** * Gets a node that may execute last in `n`, and which, when it executes last, * will be the value of `n`. */ -private ExprCfgNode getALastEvalNode(ExprCfgNode n) { - propagatesValue(n.getExpr()) and result.getASuccessor() = n +private ExprCfgNode getALastEvalNode(ExprCfgNode e) { + e = any(IfExprCfgNode n | result = [n.getThen(), n.getElse()]) or + result = e.(LoopExprCfgNode).getLoopBody() or + result = e.(ReturnExprCfgNode).getExpr() or + result = e.(BreakExprCfgNode).getExpr() or + result = e.(BlockExprCfgNode).getTailExpr() or + result = e.(MatchExprCfgNode).getArmExpr(_) or + result.(BreakExprCfgNode).getTarget() = e } module LocalFlow { @@ -278,9 +267,9 @@ module LocalFlow { predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) { nodeFrom.getCfgNode() = getALastEvalNode(nodeTo.getCfgNode()) or - exists(LetStmt s | - nodeFrom.getCfgNode().getAstNode() = s.getInitializer() and - nodeTo.getCfgNode().getAstNode() = s.getPat() + exists(LetStmtCfgNode s | + nodeFrom.getCfgNode() = s.getInitializer() and + nodeTo.getCfgNode() = s.getPat() ) } } @@ -323,7 +312,7 @@ module RustDataFlow implements InputSig { class DataFlowExpr = ExprCfgNode; /** Gets the node corresponding to `e`. */ - Node exprNode(DataFlowExpr e) { result.getCfgNode() = e } + Node exprNode(DataFlowExpr e) { result.asExpr() = e } final class DataFlowCall = DataFlowCallAlias; diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll index d5263fca81060..ed0ea686495f7 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll @@ -477,11 +477,11 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu exists(BasicBlock bb, int i | def.definesAt(_, bb, i) and value = bb.getNode(i)) } - class Parameter = Param; + class Parameter = CfgNodes::ParamCfgNode; /** Holds if SSA definition `def` initializes parameter `p` at function entry. */ predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { - exists(BasicBlock bb, int i | bb.getNode(i).getAstNode() = p and def.definesAt(_, bb, i)) + exists(BasicBlock bb, int i | bb.getNode(i) = p and def.definesAt(_, bb, i)) } class Guard extends CfgNodes::AstCfgNode { diff --git a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected index 96dbd89b7e8e5..701abfb5f166b 100644 --- a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected +++ b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected @@ -36,7 +36,6 @@ | main.rs:39:22:39:22 | [SSA] b | main.rs:41:12:41:12 | b | | main.rs:40:9:40:9 | [SSA] a | main.rs:46:5:46:5 | a | | main.rs:40:13:45:5 | BlockExpr | main.rs:40:9:40:9 | a | -| main.rs:41:12:41:12 | b | main.rs:41:9:43:9 | IfExpr | | main.rs:42:13:42:26 | BreakExpr | main.rs:40:13:45:5 | BlockExpr | | main.rs:42:26:42:26 | 1 | main.rs:42:13:42:26 | BreakExpr | | main.rs:44:9:44:9 | 2 | main.rs:40:13:45:5 | BlockExpr | @@ -44,7 +43,8 @@ | main.rs:49:22:49:22 | [SSA] b | main.rs:51:12:51:12 | b | | main.rs:50:9:50:9 | [SSA] a | main.rs:56:5:56:5 | a | | main.rs:50:13:55:5 | BlockExpr | main.rs:50:9:50:9 | a | -| main.rs:51:12:51:12 | b | main.rs:51:9:53:9 | IfExpr | +| main.rs:52:13:52:26 | BreakExpr | main.rs:50:13:55:5 | BlockExpr | | main.rs:52:26:52:26 | 1 | main.rs:52:13:52:26 | BreakExpr | +| main.rs:54:9:54:22 | BreakExpr | main.rs:50:13:55:5 | BlockExpr | | main.rs:54:22:54:22 | 2 | main.rs:54:9:54:22 | BreakExpr | | main.rs:56:5:56:5 | a | main.rs:49:38:57:1 | BlockExpr | diff --git a/rust/ql/test/utils/InlineFlowTest.qll b/rust/ql/test/utils/InlineFlowTest.qll index b4960a055ee12..ad196871273fc 100644 --- a/rust/ql/test/utils/InlineFlowTest.qll +++ b/rust/ql/test/utils/InlineFlowTest.qll @@ -5,26 +5,27 @@ import rust private import codeql.dataflow.test.InlineFlowTest +private import codeql.rust.controlflow.CfgNodes private import codeql.rust.dataflow.DataFlow private import codeql.rust.dataflow.internal.DataFlowImpl private import codeql.rust.dataflow.internal.TaintTrackingImpl private import internal.InlineExpectationsTestImpl as InlineExpectationsTestImpl // Holds if the target expression of `call` is a path and the string representation of the path is `name`. -private predicate callTargetName(CallExpr call, string name) { - call.getExpr().(PathExpr).toString() = name +private predicate callTargetName(CallExprCfgNode call, string name) { + call.getExpr().(PathExprCfgNode).toString() = name } private module FlowTestImpl implements InputSig { predicate defaultSource(DataFlow::Node source) { callTargetName(source.asExpr(), "source") } predicate defaultSink(DataFlow::Node sink) { - any(CallExpr call | callTargetName(call, "sink")).getArgList().getAnArg() = sink.asExpr() + any(CallExprCfgNode call | callTargetName(call, "sink")).getArgument(_) = sink.asExpr() } private string getSourceArgString(DataFlow::Node src) { defaultSource(src) and - result = src.asExpr().(CallExpr).getArgList().getArg(0).toString() + result = src.asExpr().(CallExprCfgNode).getArgument(0).toString() } bindingset[src, sink]