Skip to content

Commit 56ebe6c

Browse files
committed
JS: More re-export logic to handle subclass export
1 parent f2ea88a commit 56ebe6c

File tree

4 files changed

+28
-4
lines changed

4 files changed

+28
-4
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,14 @@ module TypeGraphExport<GraphExportSig<API::Node> S, shouldContainTypeSig/1 shoul
106106
shouldContainTypeEx(type1) and
107107
exists(API::Node node |
108108
// A relevant type is exported directly
109-
ModelOutput::getATypeNode(type1).getAValueReachableFromSource() = node.asSink() and
109+
Specific::sourceFlowsToSink(ModelOutput::getATypeNode(type1), node) and
110110
ExportedGraph::pathToNode(type2, path, node)
111111
or
112112
// Something that leads to a relevant type, but didn't finish its access path, is exported
113113
exists(string midType, string midPath, string remainingPath, string prefix, API::Node source |
114114
Shared::typeModel(type1, midType, midPath) and
115115
partiallyEvaluatedModel(midType, midPath, source, remainingPath) and
116-
source.getAValueReachableFromSource() = node.asSink() and
116+
Specific::sourceFlowsToSink(source, node) and
117117
ExportedGraph::pathToNode(type2, prefix, node) and
118118
path = join(prefix, remainingPath)
119119
)

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,3 +379,28 @@ predicate apiGraphHasEdge(API::Node pred, string path, API::Node succ) {
379379
path = ""
380380
)
381381
}
382+
383+
/**
384+
* Holds if the value of `source` is exposed at `sink`.
385+
*/
386+
bindingset[source]
387+
predicate sourceFlowsToSink(API::Node source, API::Node sink) {
388+
source.getAValueReachableFromSource() = sink.asSink()
389+
or
390+
// Handle the case of an upstream class being the base class of an exposed own class
391+
//
392+
// class Foo extends external.BaseClass {}
393+
//
394+
// Here we want to ensure that `Instance(Foo)` is seen as subtype of `Instance(external.BaseClass)`.
395+
//
396+
// Although we have a dedicated sink node for `Instance(Foo)` we don't have dedicate source node for `Instance(external.BaseClass)`.
397+
//
398+
// However, there is always an `Instance` edge from the base class expression (`external.BaseClass`)
399+
// to the receiver node in subclass constructor (the implicit constructor of `Foo`), which always exists.
400+
// So we use the constructor receiver as the representative for `Instance(external.BaseClass)`.
401+
// (This will get simplified when migrating to Ruby-style API graphs, as both sides will have explicit API nodes).
402+
exists(DataFlow::ClassNode cls |
403+
source.asSource() = cls.getConstructor().getReceiver() and
404+
sink = API::Internal::getClassInstance(cls)
405+
)
406+
}

javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ typeModel
5050
| (subclass).D.prototype.d | (subclass).D.prototype | Member[d] |
5151
| upstream-lib | (reexport).func | ReturnValue |
5252
| upstream-lib | reexport | Member[lib] |
53+
| upstream-lib.Type | (subclass).D.prototype | |
5354
| upstream-lib.XYZ | reexport | Member[x].Member[y].Member[z] |
5455
| upstream-lib.XYZ | reexport | Member[xy].Member[z] |
5556
summaryModel

javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ export class C extends B {
1212

1313
import * as upstream from "upstream-lib";
1414

15-
// TODO: needs to emit type model: [upstream.Type; (subclass).D.prototype; ""]
16-
// The getAValueReachableFromSource() logic does not handle the base class -> instance step
1715
export class D extends upstream.Type {
1816
d() {}
1917
}

0 commit comments

Comments
 (0)