diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll index 16e14ce84a2..f8a182685b6 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll @@ -99,6 +99,8 @@ class FormatTemplateVariableAccessTree extends LeafTree, FormatTemplateVariableA class ItemTree extends LeafTree, Item { ItemTree() { not this instanceof MacroCall and + not this instanceof Const and + not this instanceof Static and this = any(StmtList s).getAStatement() } } diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/Scope.qll b/rust/ql/lib/codeql/rust/controlflow/internal/Scope.qll index 94c5300b062..303920ce7d0 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/Scope.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/Scope.qll @@ -45,3 +45,14 @@ final class CallableScope extends CfgScopeImpl, Callable { /** Holds if `scope` is exited when `last` finishes with completion `c`. */ override predicate scopeLast(AstNode last, Completion c) { last(this.getBody(), last, c) } } + +/** + * A special scope used to represent the context in which `const`s and + * `static`s are initialized. We do not actually compute a CFG for such + * scopes. + */ +final class SourceFileScope extends CfgScopeImpl, SourceFile { + override predicate scopeFirst(AstNode first) { none() } + + override predicate scopeLast(AstNode last, Completion c) { none() } +} diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll index 4ad3af0f42a..a7e2e2e4c6b 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll @@ -653,8 +653,22 @@ module RustDataFlowGen implements InputSig */ predicate jumpStep(Node node1, Node node2) { FlowSummaryImpl::Private::Steps::summaryJumpStep(node1.(FlowSummaryNode).getSummaryNode(), - node2.(FlowSummaryNode).getSummaryNode()) or + node2.(FlowSummaryNode).getSummaryNode()) + or FlowSummaryImpl::Private::Steps::sourceJumpStep(node1.(FlowSummaryNode).getSummaryNode(), node2) + or + exists(Const c | + node1.asExpr() = c.getBody() and + node2.asExpr() = c.getAnAccess() + ) + or + exists(StaticNode sn, Static s | s = sn.getStatic() | + node1 = sn and + node2.asExpr() = s.getAnAccess().(StaticReadAccess) + or + node1.asExpr() = [s.getBody(), s.getAnAccess().(StaticWriteAccess)] and + node2 = sn + ) } pragma[nomagic] diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/Node.qll b/rust/ql/lib/codeql/rust/dataflow/internal/Node.qll index 1ccb7db73f5..9ef2c3a08fa 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/Node.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/Node.qll @@ -704,6 +704,17 @@ final class CastNode extends ExprNode { CastNode() { none() } } +/** + * A node in the data flow graph that corresponds to a `static`. + */ +class StaticNode extends AstNodeNode, TStaticNode { + override Static n; + + StaticNode() { this = TStaticNode(n) } + + Static getStatic() { result = n } +} + cached newtype TNode = TExprNode(Expr e) { e.hasEnclosingCfgScope() and Stages::DataFlowStage::ref() } or @@ -755,4 +766,5 @@ newtype TNode = ) } or TClosureSelfReferenceNode(CfgScope c) { lambdaCreationExpr(c) } or - TCaptureNode(VariableCapture::Flow::SynthesizedCaptureNode cn) + TCaptureNode(VariableCapture::Flow::SynthesizedCaptureNode cn) or + TStaticNode(Static s) diff --git a/rust/ql/lib/codeql/rust/elements/StaticAccess.qll b/rust/ql/lib/codeql/rust/elements/StaticAccess.qll index 50b4bea14d1..f0171a23771 100644 --- a/rust/ql/lib/codeql/rust/elements/StaticAccess.qll +++ b/rust/ql/lib/codeql/rust/elements/StaticAccess.qll @@ -5,3 +5,7 @@ private import internal.StaticImpl final class StaticAccess = Impl::StaticAccess; + +final class StaticWriteAccess = Impl::StaticWriteAccess; + +final class StaticReadAccess = Impl::StaticReadAccess; diff --git a/rust/ql/lib/codeql/rust/elements/internal/AstNodeImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/AstNodeImpl.qll index 554942f0fdd..0c9d736cc57 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/AstNodeImpl.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/AstNodeImpl.qll @@ -48,15 +48,29 @@ module Impl { ) } + pragma[nomagic] + private predicate isConstOrStatic(File f) { + f = this.getFile() and + ( + this instanceof Const + or + this instanceof Static + ) + } + /** Gets the CFG scope that encloses this node, if any. */ cached CfgScope getEnclosingCfgScope() { exists(AstNode p | p = this.getParentNode() | - result = p + result = p and + not result instanceof SourceFile or not p instanceof CfgScope and + not this.isConstOrStatic(_) and result = p.getEnclosingCfgScope() ) + or + this.isConstOrStatic(result.(SourceFile).getFile()) } /** Holds if this node is inside a CFG scope. */ diff --git a/rust/ql/lib/codeql/rust/elements/internal/ConstImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/ConstImpl.qll index 776215ee4f4..e90ae621779 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/ConstImpl.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/ConstImpl.qll @@ -25,6 +25,9 @@ module Impl { * ``` */ class Const extends Generated::Const { + /** Gets an access to this constant item. */ + ConstAccess getAnAccess() { this = result.getConst() } + override string toStringImpl() { result = "const " + this.getName().getText() } } diff --git a/rust/ql/lib/codeql/rust/elements/internal/StaticImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/StaticImpl.qll index 42045aea684..8002947bae8 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/StaticImpl.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/StaticImpl.qll @@ -7,6 +7,7 @@ private import codeql.rust.elements.internal.generated.Static private import codeql.rust.elements.internal.AstNodeImpl::Impl as AstNodeImpl private import codeql.rust.elements.internal.PathExprImpl::Impl as PathExprImpl +private import codeql.rust.elements.internal.VariableImpl::Impl as VariableImpl private import codeql.rust.internal.PathResolution /** @@ -24,6 +25,9 @@ module Impl { * ``` */ class Static extends Generated::Static { + /** Gets an access to this static item. */ + StaticAccess getAnAccess() { this = result.getStatic() } + override string toStringImpl() { result = "static " + this.getName().getText() } } @@ -49,4 +53,14 @@ module Impl { override string getAPrimaryQlClass() { result = "StaticAccess" } } + + /** A static write access. */ + class StaticWriteAccess extends StaticAccess { + StaticWriteAccess() { VariableImpl::assignmentOperationDescendant(_, this) } + } + + /** A static read access. */ + class StaticReadAccess extends StaticAccess { + StaticReadAccess() { not this instanceof StaticWriteAccess } + } } diff --git a/rust/ql/test/library-tests/controlflow/Cfg.expected b/rust/ql/test/library-tests/controlflow/Cfg.expected index 438b841ff16..2d1036c93c9 100644 --- a/rust/ql/test/library-tests/controlflow/Cfg.expected +++ b/rust/ql/test/library-tests/controlflow/Cfg.expected @@ -1317,10 +1317,9 @@ edges | test.rs:533:21:533:48 | { ... } | test.rs:533:21:533:48 | { ... } | | | test.rs:533:48:533:48 | 0 | test.rs:533:21:533:48 | ... > ... | | | test.rs:536:9:536:10 | 42 | test.rs:529:41:537:5 | { ... } | | -| test.rs:539:5:548:5 | enter fn const_block_panic | test.rs:540:9:540:30 | const N | | +| test.rs:539:5:548:5 | enter fn const_block_panic | test.rs:541:9:546:9 | ExprStmt | | | test.rs:539:5:548:5 | exit fn const_block_panic (normal) | test.rs:539:5:548:5 | exit fn const_block_panic | | | test.rs:539:35:548:5 | { ... } | test.rs:539:5:548:5 | exit fn const_block_panic (normal) | | -| test.rs:540:9:540:30 | const N | test.rs:541:9:546:9 | ExprStmt | | | test.rs:541:9:546:9 | ExprStmt | test.rs:541:12:541:16 | false | | | test.rs:541:9:546:9 | if false {...} | test.rs:547:9:547:9 | N | | | test.rs:541:12:541:16 | false | test.rs:541:9:546:9 | if false {...} | false | diff --git a/rust/ql/test/library-tests/dataflow/global/inline-flow.expected b/rust/ql/test/library-tests/dataflow/global/inline-flow.expected index 5caa5c1c3ed..4828b56542b 100644 --- a/rust/ql/test/library-tests/dataflow/global/inline-flow.expected +++ b/rust/ql/test/library-tests/dataflow/global/inline-flow.expected @@ -222,6 +222,15 @@ edges | main.rs:415:18:415:38 | i.get_double_number() | main.rs:415:13:415:14 | n4 | provenance | | | main.rs:418:13:418:14 | n5 | main.rs:419:14:419:15 | n5 | provenance | | | main.rs:418:18:418:41 | ...::get_default(...) | main.rs:418:13:418:14 | n5 | provenance | | +| main.rs:426:30:426:39 | source(...) | main.rs:430:35:430:45 | CONST_VALUE | provenance | | +| main.rs:427:5:427:46 | static STATIC_VALUE | main.rs:433:32:433:43 | STATIC_VALUE | provenance | | +| main.rs:427:5:427:46 | static STATIC_VALUE | main.rs:437:18:437:29 | STATIC_VALUE | provenance | | +| main.rs:427:36:427:45 | source(...) | main.rs:427:5:427:46 | static STATIC_VALUE | provenance | | +| main.rs:430:35:430:45 | CONST_VALUE | main.rs:431:14:431:25 | CONST_VALUE2 | provenance | | +| main.rs:433:17:433:28 | static_value | main.rs:434:18:434:29 | static_value | provenance | | +| main.rs:433:32:433:43 | STATIC_VALUE | main.rs:433:17:433:28 | static_value | provenance | | +| main.rs:436:13:436:24 | STATIC_VALUE | main.rs:427:5:427:46 | static STATIC_VALUE | provenance | | +| main.rs:436:28:436:37 | source(...) | main.rs:436:13:436:24 | STATIC_VALUE | provenance | | nodes | main.rs:12:28:14:1 | { ... } | semmle.label | { ... } | | main.rs:13:5:13:13 | source(...) | semmle.label | source(...) | @@ -464,6 +473,17 @@ nodes | main.rs:418:13:418:14 | n5 | semmle.label | n5 | | main.rs:418:18:418:41 | ...::get_default(...) | semmle.label | ...::get_default(...) | | main.rs:419:14:419:15 | n5 | semmle.label | n5 | +| main.rs:426:30:426:39 | source(...) | semmle.label | source(...) | +| main.rs:427:5:427:46 | static STATIC_VALUE | semmle.label | static STATIC_VALUE | +| main.rs:427:36:427:45 | source(...) | semmle.label | source(...) | +| main.rs:430:35:430:45 | CONST_VALUE | semmle.label | CONST_VALUE | +| main.rs:431:14:431:25 | CONST_VALUE2 | semmle.label | CONST_VALUE2 | +| main.rs:433:17:433:28 | static_value | semmle.label | static_value | +| main.rs:433:32:433:43 | STATIC_VALUE | semmle.label | STATIC_VALUE | +| main.rs:434:18:434:29 | static_value | semmle.label | static_value | +| main.rs:436:13:436:24 | STATIC_VALUE | semmle.label | STATIC_VALUE | +| main.rs:436:28:436:37 | source(...) | semmle.label | source(...) | +| main.rs:437:18:437:29 | STATIC_VALUE | semmle.label | STATIC_VALUE | subpaths | main.rs:38:16:38:24 | source(...) | main.rs:26:28:26:33 | ...: i64 | main.rs:26:17:26:25 | SelfParam [Return] [&ref, MyStruct] | main.rs:38:5:38:5 | [post] a [MyStruct] | | main.rs:39:10:39:10 | a [MyStruct] | main.rs:30:17:30:21 | SelfParam [&ref, MyStruct] | main.rs:30:31:32:5 | { ... } | main.rs:39:10:39:21 | a.get_data() | @@ -525,3 +545,8 @@ testFailures | main.rs:412:14:412:15 | n3 | main.rs:371:13:371:21 | source(...) | main.rs:412:14:412:15 | n3 | $@ | main.rs:371:13:371:21 | source(...) | source(...) | | main.rs:416:14:416:15 | n4 | main.rs:391:13:391:22 | source(...) | main.rs:416:14:416:15 | n4 | $@ | main.rs:391:13:391:22 | source(...) | source(...) | | main.rs:419:14:419:15 | n5 | main.rs:395:13:395:21 | source(...) | main.rs:419:14:419:15 | n5 | $@ | main.rs:395:13:395:21 | source(...) | source(...) | +| main.rs:431:14:431:25 | CONST_VALUE2 | main.rs:426:30:426:39 | source(...) | main.rs:431:14:431:25 | CONST_VALUE2 | $@ | main.rs:426:30:426:39 | source(...) | source(...) | +| main.rs:434:18:434:29 | static_value | main.rs:427:36:427:45 | source(...) | main.rs:434:18:434:29 | static_value | $@ | main.rs:427:36:427:45 | source(...) | source(...) | +| main.rs:434:18:434:29 | static_value | main.rs:436:28:436:37 | source(...) | main.rs:434:18:434:29 | static_value | $@ | main.rs:436:28:436:37 | source(...) | source(...) | +| main.rs:437:18:437:29 | STATIC_VALUE | main.rs:427:36:427:45 | source(...) | main.rs:437:18:437:29 | STATIC_VALUE | $@ | main.rs:427:36:427:45 | source(...) | source(...) | +| main.rs:437:18:437:29 | STATIC_VALUE | main.rs:436:28:436:37 | source(...) | main.rs:437:18:437:29 | STATIC_VALUE | $@ | main.rs:436:28:436:37 | source(...) | source(...) | diff --git a/rust/ql/test/library-tests/dataflow/global/main.rs b/rust/ql/test/library-tests/dataflow/global/main.rs index ac737570771..b3a3af1be95 100644 --- a/rust/ql/test/library-tests/dataflow/global/main.rs +++ b/rust/ql/test/library-tests/dataflow/global/main.rs @@ -1,4 +1,4 @@ -fn source(i: i64) -> i64 { +const fn source(i: i64) -> i64 { 1000 + i } @@ -420,6 +420,25 @@ mod not_trait_dispatch { } } +mod const_static { + use super::{sink, source}; + + const CONST_VALUE: i64 = source(42); + static mut STATIC_VALUE: i64 = source(43); + + fn test_const_static() { + const CONST_VALUE2: i64 = CONST_VALUE; + sink(CONST_VALUE2); // $ hasValueFlow=42 + unsafe { + let static_value = STATIC_VALUE; + sink(static_value); // $ hasValueFlow=43 $ SPURIOUS: hasValueFlow=44 (statics are not control-flow sensitive) + + STATIC_VALUE = source(44); + sink(STATIC_VALUE); // $ hasValueFlow=44 $ SPURIOUS: hasValueFlow=43 (statics are not control-flow sensitive) + } + } +} + fn main() { data_out_of_call(); data_out_of_call_side_effect1(); diff --git a/rust/ql/test/library-tests/dataflow/global/viableCallable.expected b/rust/ql/test/library-tests/dataflow/global/viableCallable.expected index 26db4dc3962..6f30416d54a 100644 --- a/rust/ql/test/library-tests/dataflow/global/viableCallable.expected +++ b/rust/ql/test/library-tests/dataflow/global/viableCallable.expected @@ -128,14 +128,20 @@ | main.rs:416:9:416:16 | sink(...) | main.rs:5:1:7:1 | fn sink | | main.rs:418:18:418:41 | ...::get_default(...) | main.rs:394:9:396:9 | fn get_default | | main.rs:419:9:419:16 | sink(...) | main.rs:5:1:7:1 | fn sink | -| main.rs:424:5:424:22 | data_out_of_call(...) | main.rs:16:1:19:1 | fn data_out_of_call | -| main.rs:425:5:425:35 | data_out_of_call_side_effect1(...) | main.rs:35:1:40:1 | fn data_out_of_call_side_effect1 | -| main.rs:426:5:426:35 | data_out_of_call_side_effect2(...) | main.rs:42:1:50:1 | fn data_out_of_call_side_effect2 | -| main.rs:427:5:427:21 | data_in_to_call(...) | main.rs:56:1:59:1 | fn data_in_to_call | -| main.rs:428:5:428:23 | data_through_call(...) | main.rs:65:1:69:1 | fn data_through_call | -| main.rs:429:5:429:34 | data_through_nested_function(...) | main.rs:79:1:88:1 | fn data_through_nested_function | -| main.rs:431:5:431:24 | data_out_of_method(...) | main.rs:152:1:162:1 | fn data_out_of_method | -| main.rs:432:5:432:28 | data_in_to_method_call(...) | main.rs:169:1:179:1 | fn data_in_to_method_call | -| main.rs:433:5:433:25 | data_through_method(...) | main.rs:187:1:199:1 | fn data_through_method | -| main.rs:435:5:435:31 | test_operator_overloading(...) | main.rs:256:1:298:1 | fn test_operator_overloading | -| main.rs:436:5:436:22 | test_async_await(...) | main.rs:353:1:358:1 | fn test_async_await | +| main.rs:426:30:426:39 | source(...) | main.rs:1:1:3:1 | fn source | +| main.rs:427:36:427:45 | source(...) | main.rs:1:1:3:1 | fn source | +| main.rs:431:9:431:26 | sink(...) | main.rs:5:1:7:1 | fn sink | +| main.rs:434:13:434:30 | sink(...) | main.rs:5:1:7:1 | fn sink | +| main.rs:436:28:436:37 | source(...) | main.rs:1:1:3:1 | fn source | +| main.rs:437:13:437:30 | sink(...) | main.rs:5:1:7:1 | fn sink | +| main.rs:443:5:443:22 | data_out_of_call(...) | main.rs:16:1:19:1 | fn data_out_of_call | +| main.rs:444:5:444:35 | data_out_of_call_side_effect1(...) | main.rs:35:1:40:1 | fn data_out_of_call_side_effect1 | +| main.rs:445:5:445:35 | data_out_of_call_side_effect2(...) | main.rs:42:1:50:1 | fn data_out_of_call_side_effect2 | +| main.rs:446:5:446:21 | data_in_to_call(...) | main.rs:56:1:59:1 | fn data_in_to_call | +| main.rs:447:5:447:23 | data_through_call(...) | main.rs:65:1:69:1 | fn data_through_call | +| main.rs:448:5:448:34 | data_through_nested_function(...) | main.rs:79:1:88:1 | fn data_through_nested_function | +| main.rs:450:5:450:24 | data_out_of_method(...) | main.rs:152:1:162:1 | fn data_out_of_method | +| main.rs:451:5:451:28 | data_in_to_method_call(...) | main.rs:169:1:179:1 | fn data_in_to_method_call | +| main.rs:452:5:452:25 | data_through_method(...) | main.rs:187:1:199:1 | fn data_through_method | +| main.rs:454:5:454:31 | test_operator_overloading(...) | main.rs:256:1:298:1 | fn test_operator_overloading | +| main.rs:455:5:455:22 | test_async_await(...) | main.rs:353:1:358:1 | fn test_async_await | diff --git a/rust/ql/test/library-tests/dataflow/global/viableCallable.ql b/rust/ql/test/library-tests/dataflow/global/viableCallable.ql index 3daa8b4b17f..dbb49e4855d 100644 --- a/rust/ql/test/library-tests/dataflow/global/viableCallable.ql +++ b/rust/ql/test/library-tests/dataflow/global/viableCallable.ql @@ -1,5 +1,6 @@ import codeql.rust.dataflow.internal.DataFlowImpl query predicate viableCallable(DataFlowCall call, DataFlowCallable callee) { - RustDataFlow::viableCallable(call) = callee + RustDataFlow::viableCallable(call) = callee and + (call.asCall().fromSource() or call.isImplicitDerefCall(_, _, _, _)) } diff --git a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected index e220a769ece..7d70ff90eaa 100644 --- a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected +++ b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected @@ -1,6 +1,4 @@ localStep -| file://:0:0:0:0 | [summary param] 0 in fn canonicalize | file://:0:0:0:0 | [summary] read: Argument[0].OptionalBarrier[normalize-path] in fn canonicalize | -| file://:0:0:0:0 | [summary] read: Argument[self].Reference in fn canonicalize | file://:0:0:0:0 | [summary] read: Argument[self].Reference.OptionalBarrier[normalize-path] in fn canonicalize | | main.rs:4:11:4:11 | [SSA] i | main.rs:5:12:5:12 | i | | main.rs:4:11:4:11 | i | main.rs:4:11:4:11 | [SSA] i | | main.rs:4:11:4:11 | i | main.rs:4:11:4:11 | i | diff --git a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.ql b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.ql index 21e45974529..14fa90e3d44 100644 --- a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.ql +++ b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.ql @@ -5,7 +5,9 @@ import utils.test.TranslateModels query predicate localStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { // Local flow steps that don't originate from a flow summary. - RustDataFlow::simpleLocalFlowStep(nodeFrom, nodeTo, "") + RustDataFlow::simpleLocalFlowStep(nodeFrom, nodeTo, "") and + nodeFrom.getLocation().fromSource() and + nodeTo.getLocation().fromSource() } class Node extends DataFlow::Node {