Skip to content

Commit 62383fb

Browse files
asgerfasgerf
committed
Ruby: add TypeModel hook for adding type-defs from CodeQL
1 parent 296aa52 commit 62383fb

File tree

5 files changed

+91
-11
lines changed

5 files changed

+91
-11
lines changed

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

Lines changed: 72 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ private class Unit = Specific::Unit;
7272

7373
private module API = Specific::API;
7474

75+
private module DataFlow = Specific::DF;
76+
7577
private import Specific::AccessPathSyntax
7678

7779
/** Module containing hooks for providing input data to be interpreted as a model. */
@@ -156,6 +158,22 @@ module ModelInput {
156158
abstract predicate row(string row);
157159
}
158160

161+
/**
162+
* A unit class for adding additional type model rows from CodeQL models.
163+
*/
164+
class TypeModel extends Unit {
165+
/**
166+
* Gets a data-flow node that is a source of the type `package;type`.
167+
*/
168+
DataFlow::Node getASource(string package, string type) { none() }
169+
170+
/**
171+
* Gets a data flow node that is a sink of the type `package;type`,
172+
* usually because it is an argument passed to a parameter of that type.
173+
*/
174+
DataFlow::Node getASink(string package, string type) { none() }
175+
}
176+
159177
/**
160178
* A unit class for adding additional type variable model rows.
161179
*/
@@ -368,6 +386,58 @@ private predicate invocationMatchesCallSiteFilter(Specific::InvokeNode invoke, A
368386
Specific::invocationMatchesExtraCallSiteFilter(invoke, token)
369387
}
370388

389+
private class TypeModelUseEntry extends API::EntryPoint {
390+
private string package;
391+
private string type;
392+
393+
TypeModelUseEntry() {
394+
exists(any(TypeModel tm).getASource(package, type)) and
395+
this = "TypeModelUseEntry;" + package + ";" + type
396+
}
397+
398+
override DataFlow::LocalSourceNode getASource() {
399+
result = any(TypeModel tm).getASource(package, type)
400+
}
401+
402+
API::Node getNodeForType(string package_, string type_) {
403+
package = package_ and type = type_ and result = this.getANode()
404+
}
405+
}
406+
407+
private class TypeModelDefEntry extends API::EntryPoint {
408+
private string package;
409+
private string type;
410+
411+
TypeModelDefEntry() {
412+
exists(any(TypeModel tm).getASink(package, type)) and
413+
this = "TypeModelDefEntry;" + package + ";" + type
414+
}
415+
416+
override DataFlow::Node getASink() { result = any(TypeModel tm).getASink(package, type) }
417+
418+
API::Node getNodeForType(string package_, string type_) {
419+
package = package_ and type = type_ and result = this.getANode()
420+
}
421+
}
422+
423+
/**
424+
* Gets an API node identified by the given `(package,type)` pair.
425+
*/
426+
pragma[nomagic]
427+
private API::Node getNodeFromType(string package, string type) {
428+
exists(string package2, string type2, AccessPath path2 |
429+
typeModel(package, type, package2, type2, path2) and
430+
result = getNodeFromPath(package2, type2, path2, path2.getNumToken())
431+
)
432+
or
433+
result = any(TypeModelUseEntry e).getNodeForType(package, type)
434+
or
435+
result = any(TypeModelDefEntry e).getNodeForType(package, type)
436+
or
437+
isRelevantFullPath(package, type, _) and
438+
result = Specific::getExtraNodeFromPath(package, type, _, 0)
439+
}
440+
371441
/**
372442
* Gets the API node identified by the first `n` tokens of `path` in the given `(package, type, path)` tuple.
373443
*/
@@ -376,12 +446,8 @@ private API::Node getNodeFromPath(string package, string type, AccessPath path,
376446
isRelevantFullPath(package, type, path) and
377447
(
378448
n = 0 and
379-
exists(string package2, string type2, AccessPath path2 |
380-
typeModel(package, type, package2, type2, path2) and
381-
result = getNodeFromPath(package2, type2, path2, path2.getNumToken())
382-
)
449+
result = getNodeFromType(package, type)
383450
or
384-
// Language-specific cases, such as handling of global variables
385451
result = Specific::getExtraNodeFromPath(package, type, path, n)
386452
)
387453
or
@@ -581,12 +647,7 @@ module ModelOutput {
581647
* Holds if `node` is seen as an instance of `(package,type)` due to a type definition
582648
* contributed by a CSV model.
583649
*/
584-
API::Node getATypeNode(string package, string type) {
585-
exists(string package2, string type2, AccessPath path |
586-
typeModel(package, type, package2, type2, path) and
587-
result = getNodeFromPath(package2, type2, path)
588-
)
589-
}
650+
API::Node getATypeNode(string package, string type) { result = getNodeFromType(package, type) }
590651

591652
/**
592653
* Gets an error message relating to an invalid CSV row in a model.

‎ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsSpecific.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ class Unit = DataFlowPrivate::Unit;
2929
// Re-export libraries needed by ApiGraphModels.qll
3030
import codeql.ruby.ApiGraphs
3131
import codeql.ruby.dataflow.internal.AccessPathSyntax as AccessPathSyntax
32+
import codeql.ruby.dataflow.internal.DataFlowPublic as DF
3233
private import AccessPathSyntax
3334
private import codeql.ruby.dataflow.internal.FlowSummaryImplSpecific as FlowSummaryImplSpecific
3435
private import codeql.ruby.dataflow.internal.DataFlowDispatch as DataFlowDispatch

‎ruby/ql/test/library-tests/dataflow/summaries/Summaries.expected

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ edges
3232
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:114:21:114:27 | tainted |
3333
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:117:26:117:32 | tainted |
3434
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:117:26:117:32 | tainted |
35+
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:119:23:119:29 | tainted |
36+
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:119:23:119:29 | tainted |
3537
| summaries.rb:1:20:1:36 | call to source : | summaries.rb:1:11:1:36 | call to identity : |
3638
| summaries.rb:1:20:1:36 | call to source : | summaries.rb:1:11:1:36 | call to identity : |
3739
| summaries.rb:4:12:7:3 | call to apply_block : | summaries.rb:9:6:9:13 | tainted2 |
@@ -109,6 +111,7 @@ edges
109111
| summaries.rb:104:16:104:22 | [post] tainted : | summaries.rb:113:16:113:22 | tainted |
110112
| summaries.rb:104:16:104:22 | [post] tainted : | summaries.rb:114:21:114:27 | tainted |
111113
| summaries.rb:104:16:104:22 | [post] tainted : | summaries.rb:117:26:117:32 | tainted |
114+
| summaries.rb:104:16:104:22 | [post] tainted : | summaries.rb:119:23:119:29 | tainted |
112115
| summaries.rb:104:16:104:22 | tainted : | summaries.rb:104:16:104:22 | [post] tainted : |
113116
| summaries.rb:104:16:104:22 | tainted : | summaries.rb:104:25:104:25 | [post] y : |
114117
| summaries.rb:104:16:104:22 | tainted : | summaries.rb:104:33:104:33 | [post] z : |
@@ -242,6 +245,8 @@ nodes
242245
| summaries.rb:114:21:114:27 | tainted | semmle.label | tainted |
243246
| summaries.rb:117:26:117:32 | tainted | semmle.label | tainted |
244247
| summaries.rb:117:26:117:32 | tainted | semmle.label | tainted |
248+
| summaries.rb:119:23:119:29 | tainted | semmle.label | tainted |
249+
| summaries.rb:119:23:119:29 | tainted | semmle.label | tainted |
245250
subpaths
246251
invalidSpecComponent
247252
#select
@@ -299,6 +304,8 @@ invalidSpecComponent
299304
| summaries.rb:114:21:114:27 | tainted | summaries.rb:1:20:1:36 | call to source : | summaries.rb:114:21:114:27 | tainted | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
300305
| summaries.rb:117:26:117:32 | tainted | summaries.rb:1:20:1:36 | call to source : | summaries.rb:117:26:117:32 | tainted | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
301306
| summaries.rb:117:26:117:32 | tainted | summaries.rb:1:20:1:36 | call to source : | summaries.rb:117:26:117:32 | tainted | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
307+
| summaries.rb:119:23:119:29 | tainted | summaries.rb:1:20:1:36 | call to source : | summaries.rb:119:23:119:29 | tainted | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
308+
| summaries.rb:119:23:119:29 | tainted | summaries.rb:1:20:1:36 | call to source : | summaries.rb:119:23:119:29 | tainted | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
302309
warning
303310
| CSV type row should have 5 columns but has 2: test;TooFewColumns |
304311
| CSV type row should have 5 columns but has 8: test;TooManyColumns;;;Member[Foo].Instance;too;many;columns |

‎ruby/ql/test/library-tests/dataflow/summaries/Summaries.ql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,14 @@ private class TypeFromModel extends ModelInput::TypeModelCsv {
102102
}
103103
}
104104

105+
private class TypeFromCodeQL extends ModelInput::TypeModel {
106+
override DataFlow::Node getASource(string package, string type) {
107+
package = "test" and
108+
type = "FooOrBar" and
109+
result.asExpr().getExpr().getConstantValue().getString() = "magic_string"
110+
}
111+
}
112+
105113
private class InvalidTypeModel extends ModelInput::TypeModelCsv {
106114
override predicate row(string row) {
107115
row =

‎ruby/ql/test/library-tests/dataflow/summaries/summaries.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,3 +115,6 @@ def userDefinedFunction(x, y)
115115

116116
Foo.sinkAnyNamedArg(tainted)
117117
Foo.sinkAnyNamedArg(key: tainted) # $ hasValueFlow=tainted
118+
119+
"magic_string".method(tainted) # $ hasValueFlow=tainted
120+
"magic_string2".method(tainted)

0 commit comments

Comments
 (0)