Skip to content

Commit 6e0f3df

Browse files
authored
Merge pull request github#14120 from asgerf/dynamic/typemodel-istypeused
Dynamic: add TypeModel.isTypeUsed
2 parents 5deb900 + 0b78d1d commit 6e0f3df

File tree

10 files changed

+90
-2
lines changed

10 files changed

+90
-2
lines changed

javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowNode.qll

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,10 @@ newtype TNode =
3333
TExceptionalInvocationReturnNode(InvokeExpr e) or
3434
TGlobalAccessPathRoot() or
3535
TTemplatePlaceholderTag(Templating::TemplatePlaceholderTag tag) or
36-
TReflectiveParametersNode(Function f)
36+
TReflectiveParametersNode(Function f) or
37+
TForbiddenRecursionGuard() {
38+
none() and
39+
// We want to prune irrelevant models before materialising data flow nodes, so types contributed
40+
// directly from CodeQL must expose their pruning info without depending on data flow nodes.
41+
(any(ModelInput::TypeModel tm).isTypeUsed("") implies any())
42+
}

javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,20 @@ module ModelInput {
168168
* A unit class for adding additional type model rows from CodeQL models.
169169
*/
170170
class TypeModel extends Unit {
171+
/**
172+
* Holds if any of the other predicates in this class might have a result
173+
* for the given `type`.
174+
*
175+
* The implementation of this predicate should not depend on `DataFlow::Node`.
176+
*/
177+
bindingset[type]
178+
predicate isTypeUsed(string type) { none() }
179+
171180
/**
172181
* Gets a data-flow node that is a source of the given `type`.
173182
*
183+
* Note that `type` should also be included in `isTypeUsed`.
184+
*
174185
* This must not depend on API graphs, but ensures that an API node is generated for
175186
* the source.
176187
*/
@@ -180,6 +191,8 @@ module ModelInput {
180191
* Gets a data-flow node that is a sink of the given `type`,
181192
* usually because it is an argument passed to a parameter of that type.
182193
*
194+
* Note that `type` should also be included in `isTypeUsed`.
195+
*
183196
* This must not depend on API graphs, but ensures that an API node is generated for
184197
* the sink.
185198
*/
@@ -188,6 +201,8 @@ module ModelInput {
188201
/**
189202
* Gets an API node that is a source or sink of the given `type`.
190203
*
204+
* Note that `type` should also be included in `isTypeUsed`.
205+
*
191206
* Unlike `getASource` and `getASink`, this may depend on API graphs.
192207
*/
193208
API::Node getAnApiNode(string type) { none() }
@@ -367,6 +382,8 @@ predicate isRelevantType(string type) {
367382
(
368383
Specific::isTypeUsed(type)
369384
or
385+
any(TypeModel model).isTypeUsed(type)
386+
or
370387
exists(TestAllModels t)
371388
)
372389
or

javascript/ql/test/library-tests/frameworks/data/test.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ taintFlow
7979
| test.js:269:10:269:31 | this.ba ... ource() | test.js:269:10:269:31 | this.ba ... ource() |
8080
| test.js:272:6:272:40 | new MyS ... ource() | test.js:272:6:272:40 | new MyS ... ource() |
8181
| test.js:274:6:274:39 | testlib ... eName() | test.js:274:6:274:39 | testlib ... eName() |
82+
| test.js:277:8:277:31 | "danger ... .danger | test.js:277:8:277:31 | "danger ... .danger |
8283
isSink
8384
| test.js:54:18:54:25 | source() | test-sink |
8485
| test.js:55:22:55:29 | source() | test-sink |

javascript/ql/test/library-tests/frameworks/data/test.ext.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ extensions:
1111
- ['testlib', 'Member[ParamDecoratorSource].DecoratedParameter', 'test-source']
1212
- ['testlib', 'Member[getSource].ReturnValue', 'test-source']
1313
- ['(testlib)', 'Member[parenthesizedPackageName].ReturnValue', 'test-source']
14+
- ['danger-constant', 'Member[danger]', 'test-source']
1415

1516
- addsTo:
1617
pack: codeql/javascript-all

javascript/ql/test/library-tests/frameworks/data/test.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,3 +272,9 @@ class MySubclass2 extends MySubclass {
272272
sink(new MySubclass2().baseclassSource()); // NOT OK
273273

274274
sink(testlib.parenthesizedPackageName()); // NOT OK
275+
276+
function dangerConstant() {
277+
sink("danger-constant".danger); // NOT OK
278+
sink("danger-constant".safe); // OK
279+
sink("danger-constant"); // OK
280+
}

javascript/ql/test/library-tests/frameworks/data/test.ql

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@ import javascript
22
import testUtilities.ConsistencyChecking
33
import semmle.javascript.frameworks.data.internal.ApiGraphModels as ApiGraphModels
44

5+
class TypeModelFromCodeQL extends ModelInput::TypeModel {
6+
override predicate isTypeUsed(string type) { type = "danger-constant" }
7+
8+
override DataFlow::Node getASource(string type) {
9+
type = "danger-constant" and
10+
result.getStringValue() = "danger-constant"
11+
}
12+
}
13+
514
class BasicTaintTracking extends TaintTracking::Configuration {
615
BasicTaintTracking() { this = "BasicTaintTracking" }
716

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import Attributes
99
import LocalSources
1010
private import semmle.python.essa.SsaCompute
1111
private import semmle.python.dataflow.new.internal.ImportStar
12+
private import semmle.python.frameworks.data.ModelsAsData
1213
private import FlowSummaryImpl as FlowSummaryImpl
1314
private import semmle.python.frameworks.data.ModelsAsData
1415

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

130138
private import semmle.python.internal.CachedStages

python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,20 @@ module ModelInput {
168168
* A unit class for adding additional type model rows from CodeQL models.
169169
*/
170170
class TypeModel extends Unit {
171+
/**
172+
* Holds if any of the other predicates in this class might have a result
173+
* for the given `type`.
174+
*
175+
* The implementation of this predicate should not depend on `DataFlow::Node`.
176+
*/
177+
bindingset[type]
178+
predicate isTypeUsed(string type) { none() }
179+
171180
/**
172181
* Gets a data-flow node that is a source of the given `type`.
173182
*
183+
* Note that `type` should also be included in `isTypeUsed`.
184+
*
174185
* This must not depend on API graphs, but ensures that an API node is generated for
175186
* the source.
176187
*/
@@ -180,6 +191,8 @@ module ModelInput {
180191
* Gets a data-flow node that is a sink of the given `type`,
181192
* usually because it is an argument passed to a parameter of that type.
182193
*
194+
* Note that `type` should also be included in `isTypeUsed`.
195+
*
183196
* This must not depend on API graphs, but ensures that an API node is generated for
184197
* the sink.
185198
*/
@@ -188,6 +201,8 @@ module ModelInput {
188201
/**
189202
* Gets an API node that is a source or sink of the given `type`.
190203
*
204+
* Note that `type` should also be included in `isTypeUsed`.
205+
*
191206
* Unlike `getASource` and `getASink`, this may depend on API graphs.
192207
*/
193208
API::Node getAnApiNode(string type) { none() }
@@ -367,6 +382,8 @@ predicate isRelevantType(string type) {
367382
(
368383
Specific::isTypeUsed(type)
369384
or
385+
any(TypeModel model).isTypeUsed(type)
386+
or
370387
exists(TestAllModels t)
371388
)
372389
or

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,13 @@ private module Cached {
588588
n in [-1 .. 10] and
589589
splatPos = unique(int i | splatArgumentAt(c, i) and i > 0)
590590
} or
591-
TCaptureNode(VariableCapture::Flow::SynthesizedCaptureNode cn)
591+
TCaptureNode(VariableCapture::Flow::SynthesizedCaptureNode cn) or
592+
TForbiddenRecursionGuard() {
593+
none() and
594+
// We want to prune irrelevant models before materialising data flow nodes, so types contributed
595+
// directly from CodeQL must expose their pruning info without depending on data flow nodes.
596+
(any(ModelInput::TypeModel tm).isTypeUsed("") implies any())
597+
}
592598

593599
class TSelfParameterNode = TSelfMethodParameterNode or TSelfToplevelParameterNode;
594600

ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,20 @@ module ModelInput {
168168
* A unit class for adding additional type model rows from CodeQL models.
169169
*/
170170
class TypeModel extends Unit {
171+
/**
172+
* Holds if any of the other predicates in this class might have a result
173+
* for the given `type`.
174+
*
175+
* The implementation of this predicate should not depend on `DataFlow::Node`.
176+
*/
177+
bindingset[type]
178+
predicate isTypeUsed(string type) { none() }
179+
171180
/**
172181
* Gets a data-flow node that is a source of the given `type`.
173182
*
183+
* Note that `type` should also be included in `isTypeUsed`.
184+
*
174185
* This must not depend on API graphs, but ensures that an API node is generated for
175186
* the source.
176187
*/
@@ -180,6 +191,8 @@ module ModelInput {
180191
* Gets a data-flow node that is a sink of the given `type`,
181192
* usually because it is an argument passed to a parameter of that type.
182193
*
194+
* Note that `type` should also be included in `isTypeUsed`.
195+
*
183196
* This must not depend on API graphs, but ensures that an API node is generated for
184197
* the sink.
185198
*/
@@ -188,6 +201,8 @@ module ModelInput {
188201
/**
189202
* Gets an API node that is a source or sink of the given `type`.
190203
*
204+
* Note that `type` should also be included in `isTypeUsed`.
205+
*
191206
* Unlike `getASource` and `getASink`, this may depend on API graphs.
192207
*/
193208
API::Node getAnApiNode(string type) { none() }
@@ -367,6 +382,8 @@ predicate isRelevantType(string type) {
367382
(
368383
Specific::isTypeUsed(type)
369384
or
385+
any(TypeModel model).isTypeUsed(type)
386+
or
370387
exists(TestAllModels t)
371388
)
372389
or

0 commit comments

Comments
 (0)