Skip to content

Commit 4c1c0b7

Browse files
asgerfasgerf
committed
JS: Make API-graphs use Content internally, and use steps from flow summaries
1 parent cc95c77 commit 4c1c0b7

File tree

7 files changed

+131
-40
lines changed

7 files changed

+131
-40
lines changed

‎javascript/ql/lib/semmle/javascript/ApiGraphs.qll‎

Lines changed: 95 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
import javascript
99
private import semmle.javascript.dataflow.internal.FlowSteps as FlowSteps
1010
private import semmle.javascript.dataflow.internal.PreCallGraphStep
11+
private import semmle.javascript.dataflow.internal.StepSummary
12+
private import semmle.javascript.dataflow.internal.sharedlib.SummaryTypeTracker as SummaryTypeTracker
1113
private import internal.CachedStages
1214

1315
/**
@@ -229,6 +231,40 @@ module API {
229231
result = this.getASuccessor(Label::unknownMember())
230232
}
231233

234+
cached
235+
private Node getContentRaw(DataFlow::Content content) {
236+
Stages::ApiStage::ref() and
237+
result = this.getASuccessor(Label::content(content))
238+
}
239+
240+
/**
241+
* Gets a representative for the `content` of this value.
242+
*
243+
* When possible, it is preferrable to use one of the specialized variants of this predicate, such as `getMember`.
244+
*/
245+
pragma[inline]
246+
Node getContent(DataFlow::Content content) {
247+
result = this.getContentRaw(content)
248+
or
249+
result = this.getMember(content.asPropertyName())
250+
}
251+
252+
/**
253+
* Gets a representative for the `contents` of this value.
254+
*/
255+
bindingset[this, contents]
256+
pragma[inline_late]
257+
Node getContents(DataFlow::ContentSet contents) {
258+
// We always use getAStoreContent when generating content edges, and we always use getAReadContent when querying the graph.
259+
result = this.getContent(contents.getAReadContent())
260+
}
261+
262+
/**
263+
* Gets a node representing an arbitrary array element in the array represented by this node.
264+
*/
265+
cached
266+
Node getArrayElement() { result = this.getContents(DataFlow::ContentSet::arrayElement()) }
267+
232268
/**
233269
* Gets a node representing a member of this API component where the name of the member may
234270
* or may not be known statically.
@@ -790,6 +826,11 @@ module API {
790826
not DataFlow::PseudoProperties::isPseudoProperty(prop)
791827
)
792828
or
829+
exists(DataFlow::ContentSet contents |
830+
SummaryTypeTracker::basicStoreStep(rhs, pred.getALocalUse(), contents) and
831+
lbl = Label::content(contents.getAStoreContent())
832+
)
833+
or
793834
exists(DataFlow::FunctionNode fn |
794835
fn = pred and
795836
lbl = Label::return()
@@ -982,6 +1023,11 @@ module API {
9821023
// avoid generating member edges like "$arrayElement$"
9831024
not DataFlow::PseudoProperties::isPseudoProperty(prop)
9841025
)
1026+
or
1027+
exists(DataFlow::ContentSet contents |
1028+
SummaryTypeTracker::basicLoadStep(pred.getALocalUse(), ref, contents) and
1029+
lbl = Label::content(contents.getAStoreContent())
1030+
)
9851031
)
9861032
or
9871033
exists(DataFlow::Node def, DataFlow::FunctionNode fn |
@@ -1199,8 +1245,6 @@ module API {
11991245
t = useStep(nd, promisified, boundArgs, prop, result)
12001246
}
12011247

1202-
private import semmle.javascript.dataflow.internal.StepSummary
1203-
12041248
/**
12051249
* Holds if `nd`, which is a use of an API-graph node, flows in zero or more potentially
12061250
* inter-procedural steps to some intermediate node, and then from that intermediate node to
@@ -1458,8 +1502,11 @@ module API {
14581502
bindingset[result]
14591503
LabelMember member(string m) { result.getProperty() = m }
14601504

1505+
/** Gets the `content` edge label for content `c`. */
1506+
LabelContent content(ContentPrivate::Content c) { result.getContent() = c }
1507+
14611508
/** Gets the `member` edge label for the unknown member. */
1462-
LabelUnknownMember unknownMember() { any() }
1509+
LabelContent unknownMember() { result.getContent().isUnknownArrayElement() }
14631510

14641511
/**
14651512
* Gets a property name referred to by the given dynamic property access,
@@ -1516,10 +1563,10 @@ module API {
15161563
LabelForwardingFunction forwardingFunction() { any() }
15171564

15181565
/** Gets the `promised` edge label connecting a promise to its contained value. */
1519-
LabelPromised promised() { any() }
1566+
LabelContent promised() { result.getContent() = ContentPrivate::MkPromiseValue() }
15201567

15211568
/** Gets the `promisedError` edge label connecting a promise to its rejected value. */
1522-
LabelPromisedError promisedError() { any() }
1569+
LabelContent promisedError() { result.getContent() = ContentPrivate::MkPromiseError() }
15231570

15241571
/** Gets the label for an edge leading from a value `D` to any class that has `D` as a decorator. */
15251572
LabelDecoratedClass decoratedClass() { any() }
@@ -1533,6 +1580,7 @@ module API {
15331580
/** Gets an entry-point label for the entry-point `e`. */
15341581
LabelEntryPoint entryPoint(API::EntryPoint e) { result.getEntryPoint() = e }
15351582

1583+
private import semmle.javascript.dataflow.internal.Contents::Private as ContentPrivate
15361584
private import LabelImpl
15371585

15381586
private module LabelImpl {
@@ -1542,18 +1590,12 @@ module API {
15421590
exists(Impl::MkModuleImport(mod))
15431591
} or
15441592
MkLabelInstance() or
1545-
MkLabelMember(string prop) {
1546-
exports(_, prop, _) or
1547-
exists(any(DataFlow::ClassNode c).getInstanceMethod(prop)) or
1548-
prop = "exports" or
1549-
prop = any(CanonicalName c).getName() or
1550-
prop = any(DataFlow::PropRef p).getPropertyName() or
1551-
exists(Impl::MkTypeUse(_, prop)) or
1552-
exists(any(Module m).getAnExportedValue(prop)) or
1553-
PreCallGraphStep::loadStep(_, _, prop) or
1554-
PreCallGraphStep::storeStep(_, _, prop)
1593+
MkLabelContent(DataFlow::Content content) or
1594+
MkLabelMember(string name) {
1595+
name instanceof PropertyName
1596+
or
1597+
exists(Impl::MkTypeUse(_, name))
15551598
} or
1556-
MkLabelUnknownMember() or
15571599
MkLabelParameter(int i) {
15581600
i =
15591601
[0 .. max(int args |
@@ -1564,8 +1606,6 @@ module API {
15641606
} or
15651607
MkLabelReceiver() or
15661608
MkLabelReturn() or
1567-
MkLabelPromised() or
1568-
MkLabelPromisedError() or
15691609
MkLabelDecoratedClass() or
15701610
MkLabelDecoratedMember() or
15711611
MkLabelDecoratedParameter() or
@@ -1585,13 +1625,13 @@ module API {
15851625
}
15861626

15871627
/** A label that gets a promised value. */
1588-
class LabelPromised extends ApiLabel, MkLabelPromised {
1589-
override string toString() { result = "getPromised()" }
1628+
deprecated class LabelPromised extends ApiLabel {
1629+
LabelPromised() { this = MkLabelContent(ContentPrivate::MkPromiseValue()) }
15901630
}
15911631

15921632
/** A label that gets a rejected promise. */
1593-
class LabelPromisedError extends ApiLabel, MkLabelPromisedError {
1594-
override string toString() { result = "getPromisedError()" }
1633+
deprecated class LabelPromisedError extends ApiLabel {
1634+
LabelPromisedError() { this = MkLabelContent(ContentPrivate::MkPromiseError()) }
15951635
}
15961636

15971637
/** A label that gets the return value of a function. */
@@ -1617,9 +1657,39 @@ module API {
16171657
override string toString() { result = "getInstance()" }
16181658
}
16191659

1660+
/** A label for a content. */
1661+
class LabelContent extends ApiLabel, MkLabelContent {
1662+
private DataFlow::Content content;
1663+
1664+
LabelContent() {
1665+
this = MkLabelContent(content) and
1666+
// Property names are represented by LabelMember to ensure additional property
1667+
// names from PreCallGraph step are included, as well as those from MkTypeUse.
1668+
not content instanceof ContentPrivate::MkPropertyContent
1669+
}
1670+
1671+
/** Gets the content associated with this label. */
1672+
DataFlow::Content getContent() { result = content }
1673+
1674+
private string specialisedToString() {
1675+
content instanceof ContentPrivate::MkPromiseValue and result = "getPromised()"
1676+
or
1677+
content instanceof ContentPrivate::MkPromiseError and result = "getPromisedError()"
1678+
or
1679+
content instanceof ContentPrivate::MkArrayElementUnknown and result = "getUnknownMember()"
1680+
}
1681+
1682+
override string toString() {
1683+
result = this.specialisedToString()
1684+
or
1685+
not exists(this.specialisedToString()) and
1686+
result = "getContent(" + content + ")"
1687+
}
1688+
}
1689+
16201690
/** A label for the member named `prop`. */
16211691
class LabelMember extends ApiLabel, MkLabelMember {
1622-
string prop;
1692+
private string prop;
16231693

16241694
LabelMember() { this = MkLabelMember(prop) }
16251695

@@ -1630,10 +1700,8 @@ module API {
16301700
}
16311701

16321702
/** A label for a member with an unknown name. */
1633-
class LabelUnknownMember extends ApiLabel, MkLabelUnknownMember {
1634-
LabelUnknownMember() { this = MkLabelUnknownMember() }
1635-
1636-
override string toString() { result = "getUnknownMember()" }
1703+
deprecated class LabelUnknownMember extends LabelContent {
1704+
LabelUnknownMember() { this.getContent().isUnknownArrayElement() }
16371705
}
16381706

16391707
/** A label for parameter `i`. */

‎javascript/ql/lib/semmle/javascript/dataflow/internal/Contents.qll‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,16 @@ module Private {
5757
this = getAPreciseArrayIndex().toString()
5858
or
5959
isAccessPathTokenPresent("Member", this)
60+
or
61+
this = any(ImportSpecifier spec).getImportedName()
62+
or
63+
this = any(ExportSpecifier n).getExportedName()
64+
or
65+
this = any(ExportNamedDeclaration d).getAnExportedDecl().getName()
66+
or
67+
this = any(MemberDefinition m).getName()
68+
or
69+
this = ["exports", "default"]
6070
}
6171

6272
/** Gets the array index corresponding to this property name. */

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,8 @@ API::Node getExtraSuccessorFromNode(API::Node node, AccessPathTokenBase token) {
162162
token.getName() = "Awaited" and
163163
result = node.getPromised()
164164
or
165-
token.getName() = "ArrayElement" and
166-
result = node.getMember(DataFlow::PseudoProperties::arrayElement())
165+
token.getName() = ["ArrayElement", "Element"] and
166+
result = node.getArrayElement()
167167
or
168168
token.getName() = "Element" and
169169
result = node.getMember(DataFlow::PseudoProperties::arrayLikeElement())
@@ -172,11 +172,6 @@ API::Node getExtraSuccessorFromNode(API::Node node, AccessPathTokenBase token) {
172172
token.getName() = "MapValue" and
173173
result = node.getMember(DataFlow::PseudoProperties::mapValueAll())
174174
or
175-
// Currently we need to include the "unknown member" for ArrayElement and Element since
176-
// API graphs do not use store/load steps for arrays
177-
token.getName() = ["ArrayElement", "Element"] and
178-
result = node.getUnknownMember()
179-
or
180175
token.getName() = "Parameter" and
181176
token.getAnArgument() = "this" and
182177
result = node.getReceiver()

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ taintFlow
8181
| test.js:272:6:272:40 | new MyS ... ource() | test.js:272:6:272:40 | new MyS ... ource() |
8282
| test.js:274:6:274:39 | testlib ... eName() | test.js:274:6:274:39 | testlib ... eName() |
8383
| test.js:277:8:277:31 | "danger ... .danger | test.js:277:8:277:31 | "danger ... .danger |
84+
| test.js:284:8:284:16 | source[0] | test.js:284:8:284:16 | source[0] |
85+
| test.js:285:8:285:19 | source.pop() | test.js:285:8:285:19 | source.pop() |
86+
| test.js:286:18:286:18 | e | test.js:286:28:286:28 | e |
87+
| test.js:287:14:287:14 | e | test.js:287:24:287:24 | e |
8488
isSink
8589
| test.js:54:18:54:25 | source() | test-sink |
8690
| test.js:55:22:55:29 | source() | test-sink |

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,8 @@ function dangerConstant() {
281281

282282
function arraySource() {
283283
const source = testlib.getSourceArray();
284-
sink(source[0]); // NOT OK [INCONSISTENCY]
285-
sink(source.pop()); // NOT OK [INCONSISTENCY]
286-
source.forEach(e => sink(e)); // NOT OK [INCONSISTENCY]
287-
source.map(e => sink(e)); // NOT OK [INCONSISTENCY]
284+
sink(source[0]); // NOT OK
285+
sink(source.pop()); // NOT OK
286+
source.forEach(e => sink(e)); // NOT OK
287+
source.map(e => sink(e)); // NOT OK
288288
}

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXssWithResponseThreat/Xss.expected‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
| testReactUseQueries.jsx:37:25:37:38 | repoQuery.data | testReactUseQueries.jsx:4:26:4:53 | fetch(' ... e.com') | testReactUseQueries.jsx:37:25:37:38 | repoQuery.data | Cross-site scripting vulnerability due to $@. | testReactUseQueries.jsx:4:26:4:53 | fetch(' ... e.com') | user-provided value |
1717
| testUseQueries2.vue:40:10:40:23 | v-html=data3 | testUseQueries2.vue:6:28:6:63 | fetch(" ... ntent") | testUseQueries2.vue:40:10:40:23 | v-html=data3 | Cross-site scripting vulnerability due to $@. | testUseQueries2.vue:6:28:6:63 | fetch(" ... ntent") | user-provided value |
1818
| testUseQueries2.vue:40:10:40:23 | v-html=data3 | testUseQueries2.vue:12:28:12:41 | fetch("${id}") | testUseQueries2.vue:40:10:40:23 | v-html=data3 | Cross-site scripting vulnerability due to $@. | testUseQueries2.vue:12:28:12:41 | fetch("${id}") | user-provided value |
19+
| testUseQueries.vue:25:10:25:23 | v-html=data2 | testUseQueries.vue:11:36:11:49 | fetch("${id}") | testUseQueries.vue:25:10:25:23 | v-html=data2 | Cross-site scripting vulnerability due to $@. | testUseQueries.vue:11:36:11:49 | fetch("${id}") | user-provided value |
1920
edges
2021
| test.jsx:5:11:5:63 | response | test.jsx:6:24:6:31 | response | provenance | |
2122
| test.jsx:5:22:5:63 | await f ... ntent") | test.jsx:5:11:5:63 | response | provenance | |
@@ -88,6 +89,12 @@ edges
8889
| testUseQueries2.vue:13:12:13:19 | response | testUseQueries2.vue:13:12:13:26 | response.json() | provenance | |
8990
| testUseQueries2.vue:13:12:13:26 | response.json() | testUseQueries2.vue:33:22:33:36 | results[0].data | provenance | |
9091
| testUseQueries2.vue:33:22:33:36 | results[0].data | testUseQueries2.vue:40:10:40:23 | v-html=data3 | provenance | |
92+
| testUseQueries.vue:11:19:11:49 | response | testUseQueries.vue:12:20:12:27 | response | provenance | |
93+
| testUseQueries.vue:11:30:11:49 | await fetch("${id}") | testUseQueries.vue:11:19:11:49 | response | provenance | |
94+
| testUseQueries.vue:11:36:11:49 | fetch("${id}") | testUseQueries.vue:11:30:11:49 | await fetch("${id}") | provenance | |
95+
| testUseQueries.vue:12:20:12:27 | response | testUseQueries.vue:12:20:12:34 | response.json() | provenance | |
96+
| testUseQueries.vue:12:20:12:34 | response.json() | testUseQueries.vue:18:22:18:36 | results[0].data | provenance | |
97+
| testUseQueries.vue:18:22:18:36 | results[0].data | testUseQueries.vue:25:10:25:23 | v-html=data2 | provenance | |
9198
nodes
9299
| test.jsx:5:11:5:63 | response | semmle.label | response |
93100
| test.jsx:5:22:5:63 | await f ... ntent") | semmle.label | await f ... ntent") |
@@ -174,4 +181,11 @@ nodes
174181
| testUseQueries2.vue:13:12:13:26 | response.json() | semmle.label | response.json() |
175182
| testUseQueries2.vue:33:22:33:36 | results[0].data | semmle.label | results[0].data |
176183
| testUseQueries2.vue:40:10:40:23 | v-html=data3 | semmle.label | v-html=data3 |
184+
| testUseQueries.vue:11:19:11:49 | response | semmle.label | response |
185+
| testUseQueries.vue:11:30:11:49 | await fetch("${id}") | semmle.label | await fetch("${id}") |
186+
| testUseQueries.vue:11:36:11:49 | fetch("${id}") | semmle.label | fetch("${id}") |
187+
| testUseQueries.vue:12:20:12:27 | response | semmle.label | response |
188+
| testUseQueries.vue:12:20:12:34 | response.json() | semmle.label | response.json() |
189+
| testUseQueries.vue:18:22:18:36 | results[0].data | semmle.label | results[0].data |
190+
| testUseQueries.vue:25:10:25:23 | v-html=data2 | semmle.label | v-html=data2 |
177191
subpaths

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXssWithResponseThreat/testUseQueries.vue‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export default {
88
queries: ids.map((id) => ({
99
queryKey: ['post', id],
1010
queryFn: async () => {
11-
const response = await fetch("${id}"); // $ MISSING: Source
11+
const response = await fetch("${id}"); // $ Source
1212
return response.json();
1313
},
1414
staleTime: Infinity,
@@ -22,6 +22,6 @@ export default {
2222

2323
<template>
2424
<VueQueryClientProvider :client="queryClient">
25-
<div v-html="data2"></div> <!--$ MISSING: Alert -->
25+
<div v-html="data2"></div> <!--$ Alert -->
2626
</VueQueryClientProvider>
2727
</template>

0 commit comments

Comments
 (0)