Skip to content

Commit

Permalink
Merge pull request #14120 from asgerf/dynamic/typemodel-istypeused
Browse files Browse the repository at this point in the history
Dynamic: add TypeModel.isTypeUsed
  • Loading branch information
asgerf committed Jun 6, 2024
2 parents 5deb900 + 0b78d1d commit 6e0f3df
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,10 @@ newtype TNode =
TExceptionalInvocationReturnNode(InvokeExpr e) or
TGlobalAccessPathRoot() or
TTemplatePlaceholderTag(Templating::TemplatePlaceholderTag tag) or
TReflectiveParametersNode(Function f)
TReflectiveParametersNode(Function f) or
TForbiddenRecursionGuard() {
none() and
// We want to prune irrelevant models before materialising data flow nodes, so types contributed
// directly from CodeQL must expose their pruning info without depending on data flow nodes.
(any(ModelInput::TypeModel tm).isTypeUsed("") implies any())
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,20 @@ module ModelInput {
* A unit class for adding additional type model rows from CodeQL models.
*/
class TypeModel extends Unit {
/**
* Holds if any of the other predicates in this class might have a result
* for the given `type`.
*
* The implementation of this predicate should not depend on `DataFlow::Node`.
*/
bindingset[type]
predicate isTypeUsed(string type) { none() }

/**
* Gets a data-flow node that is a source of the given `type`.
*
* Note that `type` should also be included in `isTypeUsed`.
*
* This must not depend on API graphs, but ensures that an API node is generated for
* the source.
*/
Expand All @@ -180,6 +191,8 @@ module ModelInput {
* Gets a data-flow node that is a sink of the given `type`,
* usually because it is an argument passed to a parameter of that type.
*
* Note that `type` should also be included in `isTypeUsed`.
*
* This must not depend on API graphs, but ensures that an API node is generated for
* the sink.
*/
Expand All @@ -188,6 +201,8 @@ module ModelInput {
/**
* Gets an API node that is a source or sink of the given `type`.
*
* Note that `type` should also be included in `isTypeUsed`.
*
* Unlike `getASource` and `getASink`, this may depend on API graphs.
*/
API::Node getAnApiNode(string type) { none() }
Expand Down Expand Up @@ -367,6 +382,8 @@ predicate isRelevantType(string type) {
(
Specific::isTypeUsed(type)
or
any(TypeModel model).isTypeUsed(type)
or
exists(TestAllModels t)
)
or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ taintFlow
| test.js:269:10:269:31 | this.ba ... ource() | test.js:269:10:269:31 | this.ba ... ource() |
| test.js:272:6:272:40 | new MyS ... ource() | test.js:272:6:272:40 | new MyS ... ource() |
| test.js:274:6:274:39 | testlib ... eName() | test.js:274:6:274:39 | testlib ... eName() |
| test.js:277:8:277:31 | "danger ... .danger | test.js:277:8:277:31 | "danger ... .danger |
isSink
| test.js:54:18:54:25 | source() | test-sink |
| test.js:55:22:55:29 | source() | test-sink |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ extensions:
- ['testlib', 'Member[ParamDecoratorSource].DecoratedParameter', 'test-source']
- ['testlib', 'Member[getSource].ReturnValue', 'test-source']
- ['(testlib)', 'Member[parenthesizedPackageName].ReturnValue', 'test-source']
- ['danger-constant', 'Member[danger]', 'test-source']

- addsTo:
pack: codeql/javascript-all
Expand Down
6 changes: 6 additions & 0 deletions javascript/ql/test/library-tests/frameworks/data/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,3 +272,9 @@ class MySubclass2 extends MySubclass {
sink(new MySubclass2().baseclassSource()); // NOT OK

sink(testlib.parenthesizedPackageName()); // NOT OK

function dangerConstant() {
sink("danger-constant".danger); // NOT OK
sink("danger-constant".safe); // OK
sink("danger-constant"); // OK
}
9 changes: 9 additions & 0 deletions javascript/ql/test/library-tests/frameworks/data/test.ql
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@ import javascript
import testUtilities.ConsistencyChecking
import semmle.javascript.frameworks.data.internal.ApiGraphModels as ApiGraphModels

class TypeModelFromCodeQL extends ModelInput::TypeModel {
override predicate isTypeUsed(string type) { type = "danger-constant" }

override DataFlow::Node getASource(string type) {
type = "danger-constant" and
result.getStringValue() = "danger-constant"
}
}

class BasicTaintTracking extends TaintTracking::Configuration {
BasicTaintTracking() { this = "BasicTaintTracking" }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import Attributes
import LocalSources
private import semmle.python.essa.SsaCompute
private import semmle.python.dataflow.new.internal.ImportStar
private import semmle.python.frameworks.data.ModelsAsData
private import FlowSummaryImpl as FlowSummaryImpl
private import semmle.python.frameworks.data.ModelsAsData

Expand Down Expand Up @@ -125,6 +126,13 @@ newtype TNode =
f = any(VariableCapture::CapturedVariable v).getACapturingScope() and
// TODO: Remove this restriction when adding proper support for captured variables in the body of the function we generate for comprehensions
exists(TFunction(f))
} or
/** An empty, unused node type that exists to prevent unwanted dependencies on data flow nodes. */
TForbiddenRecursionGuard() {
none() and
// We want to prune irrelevant models before materialising data flow nodes, so types contributed
// directly from CodeQL must expose their pruning info without depending on data flow nodes.
(any(ModelInput::TypeModel tm).isTypeUsed("") implies any())
}

private import semmle.python.internal.CachedStages
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,20 @@ module ModelInput {
* A unit class for adding additional type model rows from CodeQL models.
*/
class TypeModel extends Unit {
/**
* Holds if any of the other predicates in this class might have a result
* for the given `type`.
*
* The implementation of this predicate should not depend on `DataFlow::Node`.
*/
bindingset[type]
predicate isTypeUsed(string type) { none() }

/**
* Gets a data-flow node that is a source of the given `type`.
*
* Note that `type` should also be included in `isTypeUsed`.
*
* This must not depend on API graphs, but ensures that an API node is generated for
* the source.
*/
Expand All @@ -180,6 +191,8 @@ module ModelInput {
* Gets a data-flow node that is a sink of the given `type`,
* usually because it is an argument passed to a parameter of that type.
*
* Note that `type` should also be included in `isTypeUsed`.
*
* This must not depend on API graphs, but ensures that an API node is generated for
* the sink.
*/
Expand All @@ -188,6 +201,8 @@ module ModelInput {
/**
* Gets an API node that is a source or sink of the given `type`.
*
* Note that `type` should also be included in `isTypeUsed`.
*
* Unlike `getASource` and `getASink`, this may depend on API graphs.
*/
API::Node getAnApiNode(string type) { none() }
Expand Down Expand Up @@ -367,6 +382,8 @@ predicate isRelevantType(string type) {
(
Specific::isTypeUsed(type)
or
any(TypeModel model).isTypeUsed(type)
or
exists(TestAllModels t)
)
or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,13 @@ private module Cached {
n in [-1 .. 10] and
splatPos = unique(int i | splatArgumentAt(c, i) and i > 0)
} or
TCaptureNode(VariableCapture::Flow::SynthesizedCaptureNode cn)
TCaptureNode(VariableCapture::Flow::SynthesizedCaptureNode cn) or
TForbiddenRecursionGuard() {
none() and
// We want to prune irrelevant models before materialising data flow nodes, so types contributed
// directly from CodeQL must expose their pruning info without depending on data flow nodes.
(any(ModelInput::TypeModel tm).isTypeUsed("") implies any())
}

class TSelfParameterNode = TSelfMethodParameterNode or TSelfToplevelParameterNode;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,20 @@ module ModelInput {
* A unit class for adding additional type model rows from CodeQL models.
*/
class TypeModel extends Unit {
/**
* Holds if any of the other predicates in this class might have a result
* for the given `type`.
*
* The implementation of this predicate should not depend on `DataFlow::Node`.
*/
bindingset[type]
predicate isTypeUsed(string type) { none() }

/**
* Gets a data-flow node that is a source of the given `type`.
*
* Note that `type` should also be included in `isTypeUsed`.
*
* This must not depend on API graphs, but ensures that an API node is generated for
* the source.
*/
Expand All @@ -180,6 +191,8 @@ module ModelInput {
* Gets a data-flow node that is a sink of the given `type`,
* usually because it is an argument passed to a parameter of that type.
*
* Note that `type` should also be included in `isTypeUsed`.
*
* This must not depend on API graphs, but ensures that an API node is generated for
* the sink.
*/
Expand All @@ -188,6 +201,8 @@ module ModelInput {
/**
* Gets an API node that is a source or sink of the given `type`.
*
* Note that `type` should also be included in `isTypeUsed`.
*
* Unlike `getASource` and `getASink`, this may depend on API graphs.
*/
API::Node getAnApiNode(string type) { none() }
Expand Down Expand Up @@ -367,6 +382,8 @@ predicate isRelevantType(string type) {
(
Specific::isTypeUsed(type)
or
any(TypeModel model).isTypeUsed(type)
or
exists(TestAllModels t)
)
or
Expand Down

0 comments on commit 6e0f3df

Please sign in to comment.