From 299339f817387e012b88058f88517fdf2c4772c0 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Fri, 30 Sep 2022 14:28:58 +0200 Subject: [PATCH 1/3] Ruby: Expose relevant predicates from `internal/Module.qll` and make sure they are cached --- ruby/ql/lib/codeql/ruby/ApiGraphs.qll | 5 ++--- ruby/ql/lib/codeql/ruby/ast/Constant.qll | 11 ++++++++++- ruby/ql/lib/codeql/ruby/ast/Module.qll | 7 +++++++ ruby/ql/lib/codeql/ruby/ast/internal/Module.qll | 5 ++++- .../lib/codeql/ruby/frameworks/ActionController.qll | 3 +-- ruby/ql/lib/codeql/ruby/frameworks/ActionView.qll | 1 - ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll | 5 ++--- ruby/ql/lib/codeql/ruby/frameworks/ActiveResource.qll | 1 - ruby/ql/lib/codeql/ruby/frameworks/GraphQL.qll | 3 +-- ruby/ql/lib/codeql/ruby/frameworks/Rails.qll | 5 ++--- ruby/ql/lib/codeql/ruby/frameworks/Railties.qll | 5 ++++- ruby/ql/src/queries/analysis/Definitions.ql | 5 ++--- ruby/ql/test/library-tests/modules/modules.ql | 2 +- 13 files changed, 36 insertions(+), 22 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/ApiGraphs.qll b/ruby/ql/lib/codeql/ruby/ApiGraphs.qll index 3101a34e7b4..98311e0ec22 100644 --- a/ruby/ql/lib/codeql/ruby/ApiGraphs.qll +++ b/ruby/ql/lib/codeql/ruby/ApiGraphs.qll @@ -10,7 +10,6 @@ private import codeql.ruby.AST private import codeql.ruby.DataFlow private import codeql.ruby.typetracking.TypeTracker private import codeql.ruby.typetracking.TypeTrackerSpecific as TypeTrackerSpecific -private import codeql.ruby.ast.internal.Module private import codeql.ruby.controlflow.CfgNodes private import codeql.ruby.dataflow.internal.DataFlowPrivate as DataFlowPrivate private import codeql.ruby.dataflow.internal.DataFlowDispatch as DataFlowDispatch @@ -482,7 +481,7 @@ module API { MkDef(DataFlow::Node nd) { isDef(nd) } private string resolveTopLevel(ConstantReadAccess read) { - TResolved(result) = resolveConstantReadAccess(read) and + result = read.getModule().getQualifiedName() and not result.matches("%::%") } @@ -706,7 +705,7 @@ module API { exists(ClassDeclaration c, DataFlow::Node a, DataFlow::Node b | use(pred, a) and use(succ, b) and - resolveConstant(b.asExpr().getExpr()) = resolveConstantWriteAccess(c) and + b.asExpr().getExpr().(ConstantReadAccess).getAQualifiedName() = c.getAQualifiedName() and pragma[only_bind_into](c).getSuperclassExpr() = a.asExpr().getExpr() and lbl = Label::subclass() ) diff --git a/ruby/ql/lib/codeql/ruby/ast/Constant.qll b/ruby/ql/lib/codeql/ruby/ast/Constant.qll index 9da4d2a9813..d62056c866f 100644 --- a/ruby/ql/lib/codeql/ruby/ast/Constant.qll +++ b/ruby/ql/lib/codeql/ruby/ast/Constant.qll @@ -293,6 +293,15 @@ class ConstantReadAccess extends ConstantAccess { */ Expr getValue() { result = getConstantReadAccessValue(this) } + /** + * Gets a fully qualified name for this constant read, based on the context in + * which it occurs. + */ + string getAQualifiedName() { result = resolveConstant(this) } + + /** Gets the module that this read access resolves to, if any. */ + Module getModule() { result = resolveConstantReadAccess(this) } + final override string getAPrimaryQlClass() { result = "ConstantReadAccess" } } @@ -354,7 +363,7 @@ class ConstantWriteAccess extends ConstantAccess { * constants up the namespace chain, the fully qualified name of a nested * constant can be ambiguous from just statically looking at the AST. */ - string getAQualifiedName() { result = resolveConstantWriteAccess(this) } + string getAQualifiedName() { result = resolveConstantWrite(this) } /** * Gets a qualified name for this constant. Deprecated in favor of diff --git a/ruby/ql/lib/codeql/ruby/ast/Module.qll b/ruby/ql/lib/codeql/ruby/ast/Module.qll index 31034222636..8391fe5edcf 100644 --- a/ruby/ql/lib/codeql/ruby/ast/Module.qll +++ b/ruby/ql/lib/codeql/ruby/ast/Module.qll @@ -34,6 +34,13 @@ class Module extends TModule { exists(Namespace n | this = TUnresolved(n) and result = "...::" + n.toString()) } + /** + * Gets the qualified name of this module, if any. + * + * Only modules that can be resolved will have a qualified name. + */ + final string getQualifiedName() { this = TResolved(result) } + /** Gets the location of this module. */ Location getLocation() { exists(Namespace n | this = TUnresolved(n) and result = n.getLocation()) diff --git a/ruby/ql/lib/codeql/ruby/ast/internal/Module.qll b/ruby/ql/lib/codeql/ruby/ast/internal/Module.qll index 19846b7be33..9a9626cf74f 100644 --- a/ruby/ql/lib/codeql/ruby/ast/internal/Module.qll +++ b/ruby/ql/lib/codeql/ruby/ast/internal/Module.qll @@ -135,6 +135,9 @@ private module Cached { ) } + cached + string resolveConstantWrite(ConstantWriteAccess c) { result = resolveConstantWriteAccess(c) } + cached Method lookupMethod(Module m, string name) { TMethod(result) = lookupMethodOrConst(m, name) } @@ -472,7 +475,7 @@ private module ResolveImpl { } } -import ResolveImpl +private import ResolveImpl /** * A variant of AstNode::getEnclosingModule that excludes diff --git a/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll b/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll index 4dd8c31c5e8..3e2faf103ea 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll @@ -7,7 +7,6 @@ private import codeql.ruby.Concepts private import codeql.ruby.controlflow.CfgNodes private import codeql.ruby.DataFlow private import codeql.ruby.dataflow.RemoteFlowSources -private import codeql.ruby.ast.internal.Module private import codeql.ruby.ApiGraphs private import ActionView private import codeql.ruby.frameworks.ActionDispatch @@ -101,7 +100,7 @@ private predicate isRoute( ActionDispatch::Routing::Route route, string name, ActionControllerControllerClass controllerClass ) { route.getController() + "_controller" = - ActionDispatch::Routing::underscore(namespaceDeclaration(controllerClass)) and + ActionDispatch::Routing::underscore(controllerClass.getAQualifiedName()) and name = route.getAction() } diff --git a/ruby/ql/lib/codeql/ruby/frameworks/ActionView.qll b/ruby/ql/lib/codeql/ruby/frameworks/ActionView.qll index ef5b0a0204e..e9ef259d064 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/ActionView.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/ActionView.qll @@ -8,7 +8,6 @@ private import codeql.ruby.Concepts private import codeql.ruby.controlflow.CfgNodes private import codeql.ruby.DataFlow private import codeql.ruby.dataflow.RemoteFlowSources -private import codeql.ruby.ast.internal.Module private import ActionController /** diff --git a/ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll b/ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll index b6918242cbf..eff81adde23 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll @@ -8,7 +8,6 @@ private import codeql.ruby.controlflow.CfgNodes private import codeql.ruby.DataFlow private import codeql.ruby.dataflow.internal.DataFlowDispatch private import codeql.ruby.dataflow.internal.DataFlowPrivate -private import codeql.ruby.ast.internal.Module private import codeql.ruby.ApiGraphs private import codeql.ruby.frameworks.Stdlib private import codeql.ruby.frameworks.Core @@ -95,7 +94,7 @@ class ActiveRecordModelClassMethodCall extends MethodCall { ActiveRecordModelClassMethodCall() { // e.g. Foo.where(...) - recvCls.getModule() = resolveConstantReadAccess(this.getReceiver()) + recvCls.getModule() = this.getReceiver().(ConstantReadAccess).getModule() or // e.g. Foo.joins(:bars).where(...) recvCls = this.getReceiver().(ActiveRecordModelClassMethodCall).getReceiverClass() @@ -282,7 +281,7 @@ private class ActiveRecordModelFinderCall extends ActiveRecordModelInstantiation recv = getUltimateReceiver(call) and ( // The receiver refers to an `ActiveRecordModelClass` by name - resolveConstant(recv) = cls.getAQualifiedName() + recv.(ConstantReadAccess).getAQualifiedName() = cls.getAQualifiedName() or // The receiver is self, and the call is within a singleton method of // the `ActiveRecordModelClass` diff --git a/ruby/ql/lib/codeql/ruby/frameworks/ActiveResource.qll b/ruby/ql/lib/codeql/ruby/frameworks/ActiveResource.qll index 125a1a34cff..bae38d7bbd5 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/ActiveResource.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/ActiveResource.qll @@ -6,7 +6,6 @@ private import codeql.ruby.AST private import codeql.ruby.Concepts private import codeql.ruby.controlflow.CfgNodes -private import codeql.ruby.ast.internal.Module private import codeql.ruby.DataFlow private import codeql.ruby.ApiGraphs diff --git a/ruby/ql/lib/codeql/ruby/frameworks/GraphQL.qll b/ruby/ql/lib/codeql/ruby/frameworks/GraphQL.qll index 8815242a723..92f9ad275a7 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/GraphQL.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/GraphQL.qll @@ -7,7 +7,6 @@ private import codeql.ruby.Concepts private import codeql.ruby.controlflow.CfgNodes private import codeql.ruby.DataFlow private import codeql.ruby.dataflow.RemoteFlowSources -private import codeql.ruby.ast.internal.Module private import codeql.ruby.ApiGraphs private API::Node graphQlSchema() { result = API::getTopLevelMember("GraphQL").getMember("Schema") } @@ -233,7 +232,7 @@ private class GraphqlSchemaObjectClassMethodCall extends MethodCall { GraphqlSchemaObjectClassMethodCall() { // e.g. Foo.some_method(...) - recvCls.getModule() = resolveConstantReadAccess(this.getReceiver()) + recvCls.getModule() = this.getReceiver().(ConstantReadAccess).getModule() or // e.g. self.some_method(...) within a graphql Object or Interface this.getReceiver() instanceof SelfVariableAccess and diff --git a/ruby/ql/lib/codeql/ruby/frameworks/Rails.qll b/ruby/ql/lib/codeql/ruby/frameworks/Rails.qll index 49d86b0c10a..6a4aefb8a6f 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/Rails.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/Rails.qll @@ -9,7 +9,6 @@ private import codeql.ruby.frameworks.ActionController private import codeql.ruby.frameworks.ActionView private import codeql.ruby.frameworks.ActiveRecord private import codeql.ruby.frameworks.ActiveStorage -private import codeql.ruby.ast.internal.Module private import codeql.ruby.ApiGraphs private import codeql.ruby.security.OpenSSL @@ -29,7 +28,7 @@ private class RailtieClass extends ClassDeclaration { RailtieClass() { this.getSuperclassExpr() instanceof RailtieClassAccess or exists(RailtieClass other | - other.getModule() = resolveConstantReadAccess(this.getSuperclassExpr()) + other.getModule() = this.getSuperclassExpr().(ConstantReadAccess).getModule() ) } } @@ -41,7 +40,7 @@ private DataFlow::CallNode getAConfigureCallNode() { // `Rails::Application.configure` exists(ConstantReadAccess read, RailtieClass cls | read = result.getReceiver().asExpr().getExpr() and - resolveConstantReadAccess(read) = cls.getModule() and + read.getModule() = cls.getModule() and result.asExpr().getExpr().(MethodCall).getMethodName() = "configure" ) } diff --git a/ruby/ql/lib/codeql/ruby/frameworks/Railties.qll b/ruby/ql/lib/codeql/ruby/frameworks/Railties.qll index 179b6b0655e..74f64c58fcb 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/Railties.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/Railties.qll @@ -7,12 +7,15 @@ private import codeql.ruby.AST private import codeql.ruby.Concepts private import codeql.ruby.ApiGraphs private import codeql.ruby.DataFlow -private import codeql.ruby.ast.internal.Module /** * Modeling for `railties`. */ module Railties { + private class IncludeOrPrependCall extends MethodCall { + IncludeOrPrependCall() { this.getMethodName() = ["include", "prepend"] } + } + /** * A class which `include`s `Rails::Generators::Actions`. */ diff --git a/ruby/ql/src/queries/analysis/Definitions.ql b/ruby/ql/src/queries/analysis/Definitions.ql index 4bcd3696677..24e968b124d 100644 --- a/ruby/ql/src/queries/analysis/Definitions.ql +++ b/ruby/ql/src/queries/analysis/Definitions.ql @@ -11,7 +11,6 @@ */ import codeql.ruby.AST -import codeql.ruby.ast.internal.Module import codeql.ruby.dataflow.SSA from DefLoc loc, Expr src, Expr target, string kind @@ -36,7 +35,7 @@ select src, target, kind newtype DefLoc = /** A constant, module or class. */ ConstantDefLoc(ConstantReadAccess read, ConstantWriteAccess write) { - write = definitionOf(resolveConstant(read)) + write = definitionOf(read.getAQualifiedName()) } or /** A method call. */ MethodLoc(MethodCall call, Method meth) { meth = call.getATarget() } or @@ -75,7 +74,7 @@ newtype DefLoc = */ pragma[noinline] ConstantWriteAccess definitionOf(string fqn) { - fqn = resolveConstant(_) and + fqn = any(ConstantReadAccess read).getAQualifiedName() and result = min(ConstantWriteAccess w, Location l | w.getAQualifiedName() = fqn and l = w.getLocation() diff --git a/ruby/ql/test/library-tests/modules/modules.ql b/ruby/ql/test/library-tests/modules/modules.ql index af5c3574294..17dd407d673 100644 --- a/ruby/ql/test/library-tests/modules/modules.ql +++ b/ruby/ql/test/library-tests/modules/modules.ql @@ -16,7 +16,7 @@ query predicate resolveConstantReadAccess(ConstantReadAccess a, string s) { } query predicate resolveConstantWriteAccess(ConstantWriteAccess c, string s) { - s = Internal::resolveConstantWriteAccess(c) + s = c.getAQualifiedName() } query predicate enclosingModule(AstNode n, ModuleBase m) { m = n.getEnclosingModule() } From e5d884a90518928deb030e609c0e362c071c5ec3 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Fri, 30 Sep 2022 14:30:24 +0200 Subject: [PATCH 2/3] Ruby: Cache predicates in `ApiGraphModels::ModelOutput` --- .../data/internal/ApiGraphModels.qll | 110 ++++++++++-------- 1 file changed, 61 insertions(+), 49 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll index 635ec55e916..9b2428f3413 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll @@ -544,7 +544,7 @@ private API::Node getNodeFromSubPath(API::Node base, AccessPath subPath) { } /** Gets the node identified by the given `(package, type, path)` tuple. */ -API::Node getNodeFromPath(string package, string type, AccessPath path) { +private API::Node getNodeFromPath(string package, string type, AccessPath path) { result = getNodeFromPath(package, type, path, path.getNumToken()) } @@ -567,7 +567,9 @@ private predicate typeStep(API::Node pred, API::Node succ) { * * Unlike `getNodeFromPath`, the `path` may end with one or more call-site filters. */ -Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPath path, int n) { +private Specific::InvokeNode getInvocationFromPath( + string package, string type, AccessPath path, int n +) { result = Specific::getAnInvocationOf(getNodeFromPath(package, type, path, n)) or result = getInvocationFromPath(package, type, path, n - 1) and @@ -575,7 +577,7 @@ Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPa } /** Gets an invocation identified by the given `(package, type, path)` tuple. */ -Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPath path) { +private Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPath path) { result = getInvocationFromPath(package, type, path, path.getNumToken()) } @@ -583,7 +585,7 @@ Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPa * Holds if `name` is a valid name for an access path token in the identifying access path. */ bindingset[name] -predicate isValidTokenNameInIdentifyingAccessPath(string name) { +private predicate isValidTokenNameInIdentifyingAccessPath(string name) { name = ["Argument", "Parameter", "ReturnValue", "WithArity", "TypeVar"] or Specific::isExtraValidTokenNameInIdentifyingAccessPath(name) @@ -594,7 +596,7 @@ predicate isValidTokenNameInIdentifyingAccessPath(string name) { * in an identifying access path. */ bindingset[name] -predicate isValidNoArgumentTokenInIdentifyingAccessPath(string name) { +private predicate isValidNoArgumentTokenInIdentifyingAccessPath(string name) { name = "ReturnValue" or Specific::isExtraValidNoArgumentTokenInIdentifyingAccessPath(name) @@ -605,7 +607,7 @@ predicate isValidNoArgumentTokenInIdentifyingAccessPath(string name) { * in an identifying access path. */ bindingset[name, argument] -predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argument) { +private predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argument) { name = ["Argument", "Parameter"] and argument.regexpMatch("(N-|-)?\\d+(\\.\\.((N-|-)?\\d+)?)?") or @@ -622,51 +624,61 @@ predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argume * Module providing access to the imported models in terms of API graph nodes. */ module ModelOutput { - /** - * Holds if a CSV source model contributed `source` with the given `kind`. - */ - API::Node getASourceNode(string kind) { - exists(string package, string type, string path | - sourceModel(package, type, path, kind) and - result = getNodeFromPath(package, type, path) - ) + cached + private module Cached { + /** + * Holds if a CSV source model contributed `source` with the given `kind`. + */ + cached + API::Node getASourceNode(string kind) { + exists(string package, string type, string path | + sourceModel(package, type, path, kind) and + result = getNodeFromPath(package, type, path) + ) + } + + /** + * Holds if a CSV sink model contributed `sink` with the given `kind`. + */ + cached + API::Node getASinkNode(string kind) { + exists(string package, string type, string path | + sinkModel(package, type, path, kind) and + result = getNodeFromPath(package, type, path) + ) + } + + /** + * Holds if a relevant CSV summary exists for these parameters. + */ + cached + predicate relevantSummaryModel( + string package, string type, string path, string input, string output, string kind + ) { + isRelevantPackage(package) and + summaryModel(package, type, path, input, output, kind) + } + + /** + * Holds if a `baseNode` is an invocation identified by the `package,type,path` part of a summary row. + */ + cached + predicate resolvedSummaryBase( + string package, string type, string path, Specific::InvokeNode baseNode + ) { + summaryModel(package, type, path, _, _, _) and + baseNode = getInvocationFromPath(package, type, path) + } + + /** + * Holds if `node` is seen as an instance of `(package,type)` due to a type definition + * contributed by a CSV model. + */ + cached + API::Node getATypeNode(string package, string type) { result = getNodeFromType(package, type) } } - /** - * Holds if a CSV sink model contributed `sink` with the given `kind`. - */ - API::Node getASinkNode(string kind) { - exists(string package, string type, string path | - sinkModel(package, type, path, kind) and - result = getNodeFromPath(package, type, path) - ) - } - - /** - * Holds if a relevant CSV summary exists for these parameters. - */ - predicate relevantSummaryModel( - string package, string type, string path, string input, string output, string kind - ) { - isRelevantPackage(package) and - summaryModel(package, type, path, input, output, kind) - } - - /** - * Holds if a `baseNode` is an invocation identified by the `package,type,path` part of a summary row. - */ - predicate resolvedSummaryBase( - string package, string type, string path, Specific::InvokeNode baseNode - ) { - summaryModel(package, type, path, _, _, _) and - baseNode = getInvocationFromPath(package, type, path) - } - - /** - * Holds if `node` is seen as an instance of `(package,type)` due to a type definition - * contributed by a CSV model. - */ - API::Node getATypeNode(string package, string type) { result = getNodeFromType(package, type) } + import Cached /** * Gets an error message relating to an invalid CSV row in a model. From dc432c77744d391dadb45d3406cca16a9f8ea9f0 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Fri, 30 Sep 2022 14:31:09 +0200 Subject: [PATCH 3/3] Sync shared files --- .../data/internal/ApiGraphModels.qll | 110 ++++++++++-------- .../data/internal/ApiGraphModels.qll | 110 ++++++++++-------- 2 files changed, 122 insertions(+), 98 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll index 635ec55e916..9b2428f3413 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll @@ -544,7 +544,7 @@ private API::Node getNodeFromSubPath(API::Node base, AccessPath subPath) { } /** Gets the node identified by the given `(package, type, path)` tuple. */ -API::Node getNodeFromPath(string package, string type, AccessPath path) { +private API::Node getNodeFromPath(string package, string type, AccessPath path) { result = getNodeFromPath(package, type, path, path.getNumToken()) } @@ -567,7 +567,9 @@ private predicate typeStep(API::Node pred, API::Node succ) { * * Unlike `getNodeFromPath`, the `path` may end with one or more call-site filters. */ -Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPath path, int n) { +private Specific::InvokeNode getInvocationFromPath( + string package, string type, AccessPath path, int n +) { result = Specific::getAnInvocationOf(getNodeFromPath(package, type, path, n)) or result = getInvocationFromPath(package, type, path, n - 1) and @@ -575,7 +577,7 @@ Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPa } /** Gets an invocation identified by the given `(package, type, path)` tuple. */ -Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPath path) { +private Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPath path) { result = getInvocationFromPath(package, type, path, path.getNumToken()) } @@ -583,7 +585,7 @@ Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPa * Holds if `name` is a valid name for an access path token in the identifying access path. */ bindingset[name] -predicate isValidTokenNameInIdentifyingAccessPath(string name) { +private predicate isValidTokenNameInIdentifyingAccessPath(string name) { name = ["Argument", "Parameter", "ReturnValue", "WithArity", "TypeVar"] or Specific::isExtraValidTokenNameInIdentifyingAccessPath(name) @@ -594,7 +596,7 @@ predicate isValidTokenNameInIdentifyingAccessPath(string name) { * in an identifying access path. */ bindingset[name] -predicate isValidNoArgumentTokenInIdentifyingAccessPath(string name) { +private predicate isValidNoArgumentTokenInIdentifyingAccessPath(string name) { name = "ReturnValue" or Specific::isExtraValidNoArgumentTokenInIdentifyingAccessPath(name) @@ -605,7 +607,7 @@ predicate isValidNoArgumentTokenInIdentifyingAccessPath(string name) { * in an identifying access path. */ bindingset[name, argument] -predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argument) { +private predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argument) { name = ["Argument", "Parameter"] and argument.regexpMatch("(N-|-)?\\d+(\\.\\.((N-|-)?\\d+)?)?") or @@ -622,51 +624,61 @@ predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argume * Module providing access to the imported models in terms of API graph nodes. */ module ModelOutput { - /** - * Holds if a CSV source model contributed `source` with the given `kind`. - */ - API::Node getASourceNode(string kind) { - exists(string package, string type, string path | - sourceModel(package, type, path, kind) and - result = getNodeFromPath(package, type, path) - ) + cached + private module Cached { + /** + * Holds if a CSV source model contributed `source` with the given `kind`. + */ + cached + API::Node getASourceNode(string kind) { + exists(string package, string type, string path | + sourceModel(package, type, path, kind) and + result = getNodeFromPath(package, type, path) + ) + } + + /** + * Holds if a CSV sink model contributed `sink` with the given `kind`. + */ + cached + API::Node getASinkNode(string kind) { + exists(string package, string type, string path | + sinkModel(package, type, path, kind) and + result = getNodeFromPath(package, type, path) + ) + } + + /** + * Holds if a relevant CSV summary exists for these parameters. + */ + cached + predicate relevantSummaryModel( + string package, string type, string path, string input, string output, string kind + ) { + isRelevantPackage(package) and + summaryModel(package, type, path, input, output, kind) + } + + /** + * Holds if a `baseNode` is an invocation identified by the `package,type,path` part of a summary row. + */ + cached + predicate resolvedSummaryBase( + string package, string type, string path, Specific::InvokeNode baseNode + ) { + summaryModel(package, type, path, _, _, _) and + baseNode = getInvocationFromPath(package, type, path) + } + + /** + * Holds if `node` is seen as an instance of `(package,type)` due to a type definition + * contributed by a CSV model. + */ + cached + API::Node getATypeNode(string package, string type) { result = getNodeFromType(package, type) } } - /** - * Holds if a CSV sink model contributed `sink` with the given `kind`. - */ - API::Node getASinkNode(string kind) { - exists(string package, string type, string path | - sinkModel(package, type, path, kind) and - result = getNodeFromPath(package, type, path) - ) - } - - /** - * Holds if a relevant CSV summary exists for these parameters. - */ - predicate relevantSummaryModel( - string package, string type, string path, string input, string output, string kind - ) { - isRelevantPackage(package) and - summaryModel(package, type, path, input, output, kind) - } - - /** - * Holds if a `baseNode` is an invocation identified by the `package,type,path` part of a summary row. - */ - predicate resolvedSummaryBase( - string package, string type, string path, Specific::InvokeNode baseNode - ) { - summaryModel(package, type, path, _, _, _) and - baseNode = getInvocationFromPath(package, type, path) - } - - /** - * Holds if `node` is seen as an instance of `(package,type)` due to a type definition - * contributed by a CSV model. - */ - API::Node getATypeNode(string package, string type) { result = getNodeFromType(package, type) } + import Cached /** * Gets an error message relating to an invalid CSV row in a model. diff --git a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll index 635ec55e916..9b2428f3413 100644 --- a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll +++ b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll @@ -544,7 +544,7 @@ private API::Node getNodeFromSubPath(API::Node base, AccessPath subPath) { } /** Gets the node identified by the given `(package, type, path)` tuple. */ -API::Node getNodeFromPath(string package, string type, AccessPath path) { +private API::Node getNodeFromPath(string package, string type, AccessPath path) { result = getNodeFromPath(package, type, path, path.getNumToken()) } @@ -567,7 +567,9 @@ private predicate typeStep(API::Node pred, API::Node succ) { * * Unlike `getNodeFromPath`, the `path` may end with one or more call-site filters. */ -Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPath path, int n) { +private Specific::InvokeNode getInvocationFromPath( + string package, string type, AccessPath path, int n +) { result = Specific::getAnInvocationOf(getNodeFromPath(package, type, path, n)) or result = getInvocationFromPath(package, type, path, n - 1) and @@ -575,7 +577,7 @@ Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPa } /** Gets an invocation identified by the given `(package, type, path)` tuple. */ -Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPath path) { +private Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPath path) { result = getInvocationFromPath(package, type, path, path.getNumToken()) } @@ -583,7 +585,7 @@ Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPa * Holds if `name` is a valid name for an access path token in the identifying access path. */ bindingset[name] -predicate isValidTokenNameInIdentifyingAccessPath(string name) { +private predicate isValidTokenNameInIdentifyingAccessPath(string name) { name = ["Argument", "Parameter", "ReturnValue", "WithArity", "TypeVar"] or Specific::isExtraValidTokenNameInIdentifyingAccessPath(name) @@ -594,7 +596,7 @@ predicate isValidTokenNameInIdentifyingAccessPath(string name) { * in an identifying access path. */ bindingset[name] -predicate isValidNoArgumentTokenInIdentifyingAccessPath(string name) { +private predicate isValidNoArgumentTokenInIdentifyingAccessPath(string name) { name = "ReturnValue" or Specific::isExtraValidNoArgumentTokenInIdentifyingAccessPath(name) @@ -605,7 +607,7 @@ predicate isValidNoArgumentTokenInIdentifyingAccessPath(string name) { * in an identifying access path. */ bindingset[name, argument] -predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argument) { +private predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argument) { name = ["Argument", "Parameter"] and argument.regexpMatch("(N-|-)?\\d+(\\.\\.((N-|-)?\\d+)?)?") or @@ -622,51 +624,61 @@ predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argume * Module providing access to the imported models in terms of API graph nodes. */ module ModelOutput { - /** - * Holds if a CSV source model contributed `source` with the given `kind`. - */ - API::Node getASourceNode(string kind) { - exists(string package, string type, string path | - sourceModel(package, type, path, kind) and - result = getNodeFromPath(package, type, path) - ) + cached + private module Cached { + /** + * Holds if a CSV source model contributed `source` with the given `kind`. + */ + cached + API::Node getASourceNode(string kind) { + exists(string package, string type, string path | + sourceModel(package, type, path, kind) and + result = getNodeFromPath(package, type, path) + ) + } + + /** + * Holds if a CSV sink model contributed `sink` with the given `kind`. + */ + cached + API::Node getASinkNode(string kind) { + exists(string package, string type, string path | + sinkModel(package, type, path, kind) and + result = getNodeFromPath(package, type, path) + ) + } + + /** + * Holds if a relevant CSV summary exists for these parameters. + */ + cached + predicate relevantSummaryModel( + string package, string type, string path, string input, string output, string kind + ) { + isRelevantPackage(package) and + summaryModel(package, type, path, input, output, kind) + } + + /** + * Holds if a `baseNode` is an invocation identified by the `package,type,path` part of a summary row. + */ + cached + predicate resolvedSummaryBase( + string package, string type, string path, Specific::InvokeNode baseNode + ) { + summaryModel(package, type, path, _, _, _) and + baseNode = getInvocationFromPath(package, type, path) + } + + /** + * Holds if `node` is seen as an instance of `(package,type)` due to a type definition + * contributed by a CSV model. + */ + cached + API::Node getATypeNode(string package, string type) { result = getNodeFromType(package, type) } } - /** - * Holds if a CSV sink model contributed `sink` with the given `kind`. - */ - API::Node getASinkNode(string kind) { - exists(string package, string type, string path | - sinkModel(package, type, path, kind) and - result = getNodeFromPath(package, type, path) - ) - } - - /** - * Holds if a relevant CSV summary exists for these parameters. - */ - predicate relevantSummaryModel( - string package, string type, string path, string input, string output, string kind - ) { - isRelevantPackage(package) and - summaryModel(package, type, path, input, output, kind) - } - - /** - * Holds if a `baseNode` is an invocation identified by the `package,type,path` part of a summary row. - */ - predicate resolvedSummaryBase( - string package, string type, string path, Specific::InvokeNode baseNode - ) { - summaryModel(package, type, path, _, _, _) and - baseNode = getInvocationFromPath(package, type, path) - } - - /** - * Holds if `node` is seen as an instance of `(package,type)` due to a type definition - * contributed by a CSV model. - */ - API::Node getATypeNode(string package, string type) { result = getNodeFromType(package, type) } + import Cached /** * Gets an error message relating to an invalid CSV row in a model.