From 4e97a9554a16c573557e5a517b3080e58e803bf4 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Mon, 18 Nov 2024 15:21:15 +0100 Subject: [PATCH 1/2] Rust: Add interprocedural tests --- .../library-tests/dataflow/global/main.rs | 94 +++++++++++++++++++ .../dataflow/global/viableCallable.expected | 0 .../dataflow/global/viableCallable.ql | 5 + 3 files changed, 99 insertions(+) create mode 100644 rust/ql/test/library-tests/dataflow/global/main.rs create mode 100644 rust/ql/test/library-tests/dataflow/global/viableCallable.expected create mode 100644 rust/ql/test/library-tests/dataflow/global/viableCallable.ql diff --git a/rust/ql/test/library-tests/dataflow/global/main.rs b/rust/ql/test/library-tests/dataflow/global/main.rs new file mode 100644 index 000000000000..eb3a59856922 --- /dev/null +++ b/rust/ql/test/library-tests/dataflow/global/main.rs @@ -0,0 +1,94 @@ +fn source(i: i64) -> i64 { + 1000 + i +} + +fn sink(s: i64) { + println!("{}", s); +} + +// ----------------------------------------------------------------------------- +// Data flow in, out, and through functions. + +fn get_data(n: i64) -> i64 { + source(n) +} + +fn data_out_of_call() { + let a = get_data(7); + sink(a); // $ hasValueFlow=n +} + +fn data_in(n: i64) { + sink(n + 7); // $ hasValueFlow +} + +fn data_in_to_call() { + let a = source(3); + data_in(a); +} + +fn pass_through(i: i64) -> i64 { + i +} + +fn data_through_call() { + let a = source(1); + let b = pass_through(a); + sink(b); // $ hasValueFlow=1 +} + +// ----------------------------------------------------------------------------- +// Data flow in, out, and through method. + +struct MyFlag { + flag: bool, +} + +impl MyFlag { + fn data_in(&self, n: i64) { + sink(n); // $ hasValueFlow=1 + } + fn get_data(&self) -> i64 { + if self.flag { + 0 + } else { + source(2) + } + } + fn data_through(&self, n: i64) -> i64 { + if self.flag { + 0 + } else { + n + } + } +} + +fn data_out_of_method() { + let mn = MyFlag { flag: true }; + let a = mn.get_data(); + sink(a); +} + +fn data_in_to_method_call() { + let mn = MyFlag { flag: true }; + let a = source(1); + mn.data_in(a) +} + +fn data_through_method() { + let mn = MyFlag { flag: true }; + let a = source(4); + mn.data_through(a); + sink(a); // $ hasValueFlow=4 +} + +fn main() { + data_out_of_call(); + data_in_to_call(); + data_through_call(); + + data_out_of_method(); + data_in_to_method_call(); + data_through_method(); +} diff --git a/rust/ql/test/library-tests/dataflow/global/viableCallable.expected b/rust/ql/test/library-tests/dataflow/global/viableCallable.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/rust/ql/test/library-tests/dataflow/global/viableCallable.ql b/rust/ql/test/library-tests/dataflow/global/viableCallable.ql new file mode 100644 index 000000000000..3daa8b4b17f5 --- /dev/null +++ b/rust/ql/test/library-tests/dataflow/global/viableCallable.ql @@ -0,0 +1,5 @@ +import codeql.rust.dataflow.internal.DataFlowImpl + +query predicate viableCallable(DataFlowCall call, DataFlowCallable callee) { + RustDataFlow::viableCallable(call) = callee +} From 58a1b004ab57fc54e3ff281a535e65d5bb03470c Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Mon, 18 Nov 2024 15:27:21 +0100 Subject: [PATCH 2/2] Rust: Include method calls in DataFlowCall and implement simple call target resolution --- .../lib/codeql/rust/controlflow/CfgNodes.qll | 13 ++++- .../rust/dataflow/internal/DataFlowImpl.qll | 50 +++++++++---------- .../dataflow/global/viableCallable.expected | 24 +++++++++ 3 files changed, 60 insertions(+), 27 deletions(-) diff --git a/rust/ql/lib/codeql/rust/controlflow/CfgNodes.qll b/rust/ql/lib/codeql/rust/controlflow/CfgNodes.qll index 3614035a183e..cbc1a4162374 100644 --- a/rust/ql/lib/codeql/rust/controlflow/CfgNodes.qll +++ b/rust/ql/lib/codeql/rust/controlflow/CfgNodes.qll @@ -31,8 +31,19 @@ class ExprCfgNode extends AstCfgNode { } /** A CFG node that corresponds to a call in the AST. */ -class CallCfgNode extends ExprCfgNode { +class CallExprCfgNode extends ExprCfgNode { override CallExpr node; + + /** Gets the underlying `CallExpr`. */ + CallExpr getCallExpr() { result = node } +} + +/** A CFG node that corresponds to a call in the AST. */ +class MethodCallExprCfgNode extends ExprCfgNode { + override MethodCallExpr node; + + /** Gets the underlying `MethodCallExpr`. */ + MethodCallExpr getMethodCallExpr() { result = node } } final class ExitCfgNode = ExitNode; diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll index 00692f021817..38ee157d8b1f 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll @@ -42,35 +42,23 @@ final class DataFlowCallable extends TDataFlowCallable { Location getLocation() { result = this.asCfgScope().getLocation() } } -abstract class DataFlowCall extends TDataFlowCall { - /** Gets the enclosing callable. */ - abstract DataFlowCallable getEnclosingCallable(); - - /** Gets the underlying source code call, if any. */ - abstract CallCfgNode asCall(); - - /** Gets a textual representation of this call. */ - abstract string toString(); - - /** Gets the location of this call. */ - abstract Location getLocation(); -} - -final class NormalCall extends DataFlowCall, TNormalCall { - private CallCfgNode c; +final class DataFlowCall extends TDataFlowCall { + /** Gets the underlying call in the CFG, if any. */ + CallExprCfgNode asCallExprCfgNode() { this = TNormalCall(result) } - NormalCall() { this = TNormalCall(c) } + MethodCallExprCfgNode asMethodCallExprCfgNode() { this = TMethodCall(result) } - /** Gets the underlying call in the CFG, if any. */ - override CallCfgNode asCall() { result = c } + ExprCfgNode asExprCfgNode() { + result = this.asCallExprCfgNode() or result = this.asMethodCallExprCfgNode() + } - override DataFlowCallable getEnclosingCallable() { - result = TCfgScope(c.getExpr().getEnclosingCfgScope()) + DataFlowCallable getEnclosingCallable() { + result = TCfgScope(this.asExprCfgNode().getExpr().getEnclosingCfgScope()) } - override string toString() { result = c.toString() } + string toString() { result = this.asExprCfgNode().toString() } - override Location getLocation() { result = c.getLocation() } + Location getLocation() { result = this.asExprCfgNode().getLocation() } } module Node { @@ -204,7 +192,7 @@ module Node { ExprOutNode() { this.asExpr() instanceof CallExpr } /** Gets the underlying call CFG node that includes this out node. */ - override DataFlowCall getCall() { result.(NormalCall).asCall() = this.getCfgNode() } + override DataFlowCall getCall() { result.asExprCfgNode() = this.getCfgNode() } } /** @@ -331,7 +319,15 @@ module RustDataFlow implements InputSig { final class ReturnKind = ReturnKindAlias; /** Gets a viable implementation of the target of the given `Call`. */ - DataFlowCallable viableCallable(DataFlowCall c) { none() } + DataFlowCallable viableCallable(DataFlowCall c) { + exists(Function f, string name | result.asCfgScope() = f and name = f.getName().toString() | + if f.getParamList().hasSelfParam() + then name = c.asMethodCallExprCfgNode().getMethodCallExpr().getNameRef().getText() + else + name = + c.asCallExprCfgNode().getCallExpr().getExpr().(PathExpr).getPath().getPart().toString() + ) + } /** * Gets a node that can read the value returned from `call` with return kind @@ -488,7 +484,9 @@ private module Cached { TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) cached - newtype TDataFlowCall = TNormalCall(CallCfgNode c) + newtype TDataFlowCall = + TNormalCall(CallExprCfgNode c) or + TMethodCall(MethodCallExprCfgNode c) cached newtype TOptionalContentSet = diff --git a/rust/ql/test/library-tests/dataflow/global/viableCallable.expected b/rust/ql/test/library-tests/dataflow/global/viableCallable.expected index e69de29bb2d1..6a15bb25249a 100644 --- a/rust/ql/test/library-tests/dataflow/global/viableCallable.expected +++ b/rust/ql/test/library-tests/dataflow/global/viableCallable.expected @@ -0,0 +1,24 @@ +| main.rs:13:5:13:13 | CallExpr | main.rs:1:1:3:1 | source | +| main.rs:17:13:17:23 | CallExpr | main.rs:12:1:14:1 | get_data | +| main.rs:18:5:18:11 | CallExpr | main.rs:5:1:7:1 | sink | +| main.rs:22:5:22:15 | CallExpr | main.rs:5:1:7:1 | sink | +| main.rs:26:13:26:21 | CallExpr | main.rs:1:1:3:1 | source | +| main.rs:27:5:27:14 | CallExpr | main.rs:21:1:23:1 | data_in | +| main.rs:35:13:35:21 | CallExpr | main.rs:1:1:3:1 | source | +| main.rs:36:13:36:27 | CallExpr | main.rs:30:1:32:1 | pass_through | +| main.rs:37:5:37:11 | CallExpr | main.rs:5:1:7:1 | sink | +| main.rs:49:9:49:15 | CallExpr | main.rs:5:1:7:1 | sink | +| main.rs:55:13:55:21 | CallExpr | main.rs:1:1:3:1 | source | +| main.rs:69:13:69:25 | ... .get_data(...) | main.rs:51:5:57:5 | get_data | +| main.rs:70:5:70:11 | CallExpr | main.rs:5:1:7:1 | sink | +| main.rs:75:13:75:21 | CallExpr | main.rs:1:1:3:1 | source | +| main.rs:76:5:76:17 | ... .data_in(...) | main.rs:48:5:50:5 | data_in | +| main.rs:81:13:81:21 | CallExpr | main.rs:1:1:3:1 | source | +| main.rs:82:5:82:22 | ... .data_through(...) | main.rs:58:5:64:5 | data_through | +| main.rs:83:5:83:11 | CallExpr | main.rs:5:1:7:1 | sink | +| main.rs:87:5:87:22 | CallExpr | main.rs:16:1:19:1 | data_out_of_call | +| main.rs:88:5:88:21 | CallExpr | main.rs:25:1:28:1 | data_in_to_call | +| main.rs:89:5:89:23 | CallExpr | main.rs:34:1:38:1 | data_through_call | +| main.rs:91:5:91:24 | CallExpr | main.rs:67:1:71:1 | data_out_of_method | +| main.rs:92:5:92:28 | CallExpr | main.rs:73:1:77:1 | data_in_to_method_call | +| main.rs:93:5:93:25 | CallExpr | main.rs:79:1:84:1 | data_through_method |