diff --git a/ql/lib/codeql/ruby/ApiGraphs.qll b/ql/lib/codeql/ruby/ApiGraphs.qll index cfefb81504e..5330a4c00ea 100644 --- a/ql/lib/codeql/ruby/ApiGraphs.qll +++ b/ql/lib/codeql/ruby/ApiGraphs.qll @@ -253,6 +253,11 @@ module API { /** A use of an API member at the node `nd`. */ MkUse(DataFlow::Node nd) { use(_, _, nd) } + private string resolveTopLevel(ConstantReadAccess read) { + TResolved(result) = resolveScopeExpr(read) and + not result.matches("%::%") + } + /** * Holds if `ref` is a use of a node that should have an incoming edge from `base` labeled * `lbl` in the API graph. @@ -265,15 +270,14 @@ module API { lbl = Label::member(read.getName()) and read = access.getExpr() | - TResolved(name) = resolveScopeExpr(read) and - not name.matches("%::%") + name = resolveTopLevel(read) or name = read.getName() and - not exists(resolveScopeExpr(read)) and + not exists(resolveTopLevel(read)) and not exists(read.getScopeExpr()) ) or - exists(DataFlow::LocalSourceNode src, DataFlow::LocalSourceNode pred | + exists(ExprCfgNode node | // First, we find a predecessor of the node `ref` that we want to determine. The predecessor // is any node that is a type-tracked use of a data flow node (`src`), which is itself a // reference to the API node `base`. Thus, `pred` and `src` both represent uses of `base`. @@ -281,25 +285,23 @@ module API { // Once we have identified the predecessor, we define its relation to the successor `ref` as // well as the label on the edge from `pred` to `ref`. This label describes the nature of // the relationship between `pred` and `ref`. - use(base, src) and pred = trackUseNode(src) + useExpr(node, base) | // // Referring to an attribute on a node that is a use of `base`: // pred = `Rails` part of `Rails::Whatever` // lbl = `Whatever` // ref = `Rails::Whatever` - exists(ExprNodes::ConstantAccessCfgNode c, DataFlow::Node node, ConstantReadAccess read | - not exists(resolveScopeExpr(read)) and - pred.flowsTo(node) and - node.asExpr() = c.getScopeExpr() and + exists(ExprNodes::ConstantAccessCfgNode c, ConstantReadAccess read | + not exists(resolveTopLevel(read)) and + node = c.getScopeExpr() and lbl = Label::member(read.getName()) and ref.asExpr() = c and read = c.getExpr() ) or // Calling a method on a node that is a use of `base` - exists(ExprNodes::MethodCallCfgNode call, DataFlow::Node node, string name | - pred.flowsTo(node) and - node.asExpr() = call.getReceiver() and + exists(ExprNodes::MethodCallCfgNode call, string name | + node = call.getReceiver() and name = call.getExpr().getMethodName() and lbl = Label::return(name) and name != "new" and @@ -307,9 +309,8 @@ module API { ) or // Calling the `new` method on a node that is a use of `base`, which creates a new instance - exists(ExprNodes::MethodCallCfgNode call, DataFlow::Node node | - pred.flowsTo(node) and - node.asExpr() = call.getReceiver() and + exists(ExprNodes::MethodCallCfgNode call | + node = call.getReceiver() and lbl = Label::instance() and call.getExpr().getMethodName() = "new" and ref.asExpr() = call @@ -317,6 +318,15 @@ module API { ) } + pragma[nomagic] + private predicate useExpr(ExprCfgNode node, TApiNode base) { + exists(DataFlow::LocalSourceNode src, DataFlow::LocalSourceNode pred | + use(base, src) and + pred = trackUseNode(src) and + pred.flowsTo(any(DataFlow::ExprNode n | n.getExprNode() = node)) + ) + } + /** * Holds if `ref` is a use of node `nd`. */ @@ -328,7 +338,10 @@ module API { * * The flow from `src` to that node may be inter-procedural. */ - private DataFlow::LocalSourceNode trackUseNode(DataFlow::LocalSourceNode src, TypeTracker t) { + private DataFlow::LocalSourceNode trackUseNode(DataFlow::Node src, TypeTracker t) { + // Declaring `src` to be a `SourceNode` currently causes a redundant check in the + // recursive case, so instead we check it explicitly here. + src instanceof DataFlow::LocalSourceNode and t.start() and use(_, src) and result = src diff --git a/ql/test/library-tests/dataflow/api-graphs/test1.rb b/ql/test/library-tests/dataflow/api-graphs/test1.rb index f45647d64e4..bc9113bb5d0 100644 --- a/ql/test/library-tests/dataflow/api-graphs/test1.rb +++ b/ql/test/library-tests/dataflow/api-graphs/test1.rb @@ -22,3 +22,10 @@ foo::Bar::Baz #$ use=getMember("Foo").getMember("Bar").getMember("Baz") FooAlias = Foo #$ use=getMember("Foo") FooAlias::Bar::Baz #$ use=getMember("Foo").getMember("Bar").getMember("Baz") + +module Outer + module Inner + end +end + +Outer::Inner.foo #$ use=getMember("Outer").getMember("Inner").getReturn("foo")