Skip to content

Commit 28c060e

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#6113 from erik-krogh/promise
Approved by esbena
2 parents 61c8936 + f53955f commit 28c060e

File tree

5 files changed

+193
-10
lines changed

5 files changed

+193
-10
lines changed

‎javascript/change-notes/2021-06-18-promises.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
lgtm,codescanning
2+
* The security queries now track flow through various `Promise` polyfills.
3+
Affected packages are
4+
[kew](https://npmjs.com/package/kew),
5+
[promise](https://npmjs.com/package/promise),
6+
[promise-polyfill](https://npmjs.com/package/promise-polyfill),
7+
[rsvp](https://npmjs.com/package/rsvp),
8+
[es6-promise](https://npmjs.com/package/es6-promise),
9+
[native-promise-only](https://npmjs.com/package/native-promise-only),
10+
[when](https://npmjs.com/package/when),
11+
[pinkie-promise](https://npmjs.com/package/pinkie-promise),
12+
[pinkie](https://npmjs.com/package/pinkie),
13+
[synchronous-promise](https://npmjs.com/package/synchronous-promise),
14+
[any-promise](https://npmjs.com/package/any-promise),
15+
[lie](https://npmjs.com/package/lie),
16+
[promise.allsettled](https://npmjs.com/package/promise.allsettled)

‎javascript/ql/src/semmle/javascript/Promises.qll

Lines changed: 48 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,43 @@ private predicate hasHandler(DataFlow::InvokeNode promise, string m, int i) {
5858
exists(promise.getAMethodCall(m).getCallback(i))
5959
}
6060

61+
/**
62+
* Gets a reference to the `Promise` object.
63+
* Either from the standard library, a polyfill import, or a polyfill that defines the global `Promise` variable.
64+
*/
65+
private DataFlow::SourceNode getAPromiseObject() {
66+
// Standard library, or polyfills like [es6-shim](https://npmjs.org/package/es6-shim).
67+
result = DataFlow::globalVarRef("Promise")
68+
or
69+
// polyfills from the [`promise`](https://npmjs.org/package/promise) library.
70+
result =
71+
DataFlow::moduleImport([
72+
"promise", "promise/domains", "promise/setimmediate", "promise/lib/es6-extensions",
73+
"promise/domains/es6-extensions", "promise/setimmediate/es6-extensions"
74+
])
75+
or
76+
// polyfill from the [`promise-polyfill`](https://npmjs.org/package/promise-polyfill) library.
77+
result = DataFlow::moduleMember(["promise-polyfill", "promise-polyfill/src/polyfill"], "default")
78+
or
79+
result = DataFlow::moduleImport(["promise-polyfill", "promise-polyfill/src/polyfill"])
80+
or
81+
result = DataFlow::moduleMember(["es6-promise", "rsvp"], "Promise")
82+
or
83+
result = DataFlow::moduleImport("native-promise-only")
84+
or
85+
result = DataFlow::moduleImport("when")
86+
or
87+
result = DataFlow::moduleImport("pinkie-promise")
88+
or
89+
result = DataFlow::moduleImport("pinkie")
90+
or
91+
result = DataFlow::moduleMember("synchronous-promise", "SynchronousPromise")
92+
or
93+
result = DataFlow::moduleImport("any-promise")
94+
or
95+
result = DataFlow::moduleImport("lie")
96+
}
97+
6198
/**
6299
* A call that looks like a Promise.
63100
*
@@ -72,10 +109,11 @@ class PromiseCandidate extends DataFlow::InvokeNode {
72109
}
73110

74111
/**
75-
* A promise object created by the standard ECMAScript 2015 `Promise` constructor.
112+
* A promise object created by the standard ECMAScript 2015 `Promise` constructor,
113+
* or a polyfill implementing a superset of the ECMAScript 2015 `Promise` API.
76114
*/
77-
private class ES2015PromiseDefinition extends PromiseDefinition, DataFlow::NewNode {
78-
ES2015PromiseDefinition() { this = DataFlow::globalVarRef("Promise").getAnInstantiation() }
115+
private class ES2015PromiseDefinition extends PromiseDefinition, DataFlow::InvokeNode {
116+
ES2015PromiseDefinition() { this = getAPromiseObject().getAnInvocation() }
79117

80118
override DataFlow::FunctionNode getExecutor() { result = getCallback(0) }
81119
}
@@ -109,9 +147,7 @@ abstract class PromiseAllCreation extends PromiseCreationCall {
109147
* A resolved promise created by the standard ECMAScript 2015 `Promise.resolve` function.
110148
*/
111149
class ResolvedES2015PromiseDefinition extends ResolvedPromiseDefinition {
112-
ResolvedES2015PromiseDefinition() {
113-
this = DataFlow::globalVarRef("Promise").getAMemberCall("resolve")
114-
}
150+
ResolvedES2015PromiseDefinition() { this = getAPromiseObject().getAMemberCall("resolve") }
115151

116152
override DataFlow::Node getValue() { result = getArgument(0) }
117153
}
@@ -121,9 +157,11 @@ class ResolvedES2015PromiseDefinition extends ResolvedPromiseDefinition {
121157
*/
122158
class AggregateES2015PromiseDefinition extends PromiseCreationCall {
123159
AggregateES2015PromiseDefinition() {
124-
exists(string m | m = "all" or m = "race" or m = "any" |
125-
this = DataFlow::globalVarRef("Promise").getAMemberCall(m)
160+
exists(string m | m = "all" or m = "race" or m = "any" or m = "allSettled" |
161+
this = getAPromiseObject().getAMemberCall(m)
126162
)
163+
or
164+
this = DataFlow::moduleImport("promise.allsettled").getACall()
127165
}
128166

129167
override DataFlow::Node getValue() {
@@ -562,14 +600,14 @@ module Bluebird {
562600
}
563601

564602
/**
565-
* Provides classes for working with the `q` library (https://github.com/kriskowal/q).
603+
* Provides classes for working with the `q` library (https://github.com/kriskowal/q) and the compatible `kew` library (https://github.com/Medium/kew).
566604
*/
567605
module Q {
568606
/**
569607
* A promise object created by the q `Promise` constructor.
570608
*/
571609
private class QPromiseDefinition extends PromiseDefinition, DataFlow::CallNode {
572-
QPromiseDefinition() { this = DataFlow::moduleMember("q", "Promise").getACall() }
610+
QPromiseDefinition() { this = DataFlow::moduleMember(["q", "kew"], "Promise").getACall() }
573611

574612
override DataFlow::FunctionNode getExecutor() { result = getCallback(0) }
575613
}

‎javascript/ql/test/library-tests/Promises/AdditionalPromises.expected

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,9 @@
7979
| promises.js:71:5:71:27 | Promise ... source) |
8080
| promises.js:72:5:72:41 | new Pro ... ource)) |
8181
| promises.js:79:19:79:41 | Promise ... source) |
82+
| promises.js:88:17:90:4 | Q.Promi ... );\\n }) |
83+
| promises.js:112:17:112:62 | new RSV ... ct) {}) |
84+
| promises.js:124:19:124:30 | when(source) |
85+
| promises.js:130:14:130:69 | new Pro ... s'); }) |
86+
| promises.js:135:3:137:4 | new Pro ... );\\n }) |
87+
| promises.js:148:10:148:49 | new Pro ... ect){}) |

‎javascript/ql/test/library-tests/Promises/promises.js

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,75 @@
8080
promise.then(function (val) {
8181
var sink = val;
8282
});
83+
})();
84+
85+
86+
(function() {
87+
var Q = require("kew");
88+
var promise = Q.Promise(function (resolve, reject) {
89+
resolve(source);
90+
});
91+
promise.then(function (val) {
92+
var sink = val;
93+
});
94+
})();
95+
96+
(function() {
97+
var PromiseA = require('promise');
98+
var PromiseB = require('promise/domains');
99+
PromiseA.resolve(source);
100+
PromiseB.resolve(source);
101+
})();
102+
103+
(function() {
104+
var PromiseA = require('promise-polyfill').default;
105+
import PromiseB from 'promise-polyfill';
106+
PromiseA.resolve(source);
107+
PromiseB.resolve(source);
108+
})();
109+
110+
(function() {
111+
var RSVP = require('rsvp');
112+
var promise = new RSVP.Promise(function(resolve, reject) {});
113+
var Promise = require('es6-promise').Promise;
114+
Promise.resolve(source);
115+
})();
116+
117+
(function() {
118+
var Promise = require('native-promise-only');
119+
Promise.resolve(source);
120+
})();
121+
122+
(function() {
123+
const when = require('when');
124+
const promise = when(source);
125+
const promise2 = when.resolve(source);
126+
})();
127+
128+
(function() {
129+
var Promise = require('pinkie-promise');
130+
var prom = new Promise(function (resolve) { resolve('unicorns'); });
131+
})();
132+
133+
(function() {
134+
var Promise = require('pinkie');
135+
new Promise(function (resolve, reject) {
136+
resolve(data);
137+
});
138+
})();
139+
140+
(function() {
141+
import { SynchronousPromise } from 'synchronous-promise';
142+
// is technically not a promise, but behaves like one.
143+
var promise = SynchronousPromise.resolve(source);
144+
})();
145+
146+
(function() {
147+
var Promise = require('any-promise');
148+
return new Promise(function(resolve, reject){})
149+
})();
150+
151+
(function() {
152+
var Promise = require('lie');
153+
var promise = Promise.resolve(source);
83154
})();

‎javascript/ql/test/library-tests/Promises/tests.expected

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,15 @@ test_ResolvedPromiseDefinition
3636
| promises.js:62:19:62:41 | Promise ... source) | promises.js:62:35:62:40 | source |
3737
| promises.js:71:5:71:27 | Promise ... source) | promises.js:71:21:71:26 | source |
3838
| promises.js:79:19:79:41 | Promise ... source) | promises.js:79:35:79:40 | source |
39+
| promises.js:99:3:99:26 | Promise ... source) | promises.js:99:20:99:25 | source |
40+
| promises.js:100:3:100:26 | Promise ... source) | promises.js:100:20:100:25 | source |
41+
| promises.js:106:3:106:26 | Promise ... source) | promises.js:106:20:106:25 | source |
42+
| promises.js:107:3:107:26 | Promise ... source) | promises.js:107:20:107:25 | source |
43+
| promises.js:114:3:114:25 | Promise ... source) | promises.js:114:19:114:24 | source |
44+
| promises.js:119:3:119:25 | Promise ... source) | promises.js:119:19:119:24 | source |
45+
| promises.js:125:20:125:39 | when.resolve(source) | promises.js:125:33:125:38 | source |
46+
| promises.js:143:17:143:50 | Synchro ... source) | promises.js:143:44:143:49 | source |
47+
| promises.js:153:17:153:39 | Promise ... source) | promises.js:153:33:153:38 | source |
3948
test_PromiseDefinition_getARejectHandler
4049
| flow.js:26:2:26:49 | new Pro ... ource)) | flow.js:26:69:26:80 | y => sink(y) |
4150
| flow.js:32:2:32:49 | new Pro ... ource)) | flow.js:32:57:32:68 | x => sink(x) |
@@ -82,6 +91,11 @@ test_PromiseDefinition_getExecutor
8291
| promises.js:10:18:17:4 | new Pro ... );\\n }) | promises.js:10:30:17:3 | (res, r ... e);\\n } |
8392
| promises.js:33:19:35:6 | new Pro ... \\n }) | promises.js:33:31:35:5 | functio ... ;\\n } |
8493
| promises.js:43:19:45:6 | Q.Promi ... \\n }) | promises.js:43:29:45:5 | functio ... ;\\n } |
94+
| promises.js:88:17:90:4 | Q.Promi ... );\\n }) | promises.js:88:27:90:3 | functio ... e);\\n } |
95+
| promises.js:112:17:112:62 | new RSV ... ct) {}) | promises.js:112:34:112:61 | functio ... ect) {} |
96+
| promises.js:130:14:130:69 | new Pro ... s'); }) | promises.js:130:26:130:68 | functio ... ns'); } |
97+
| promises.js:135:3:137:4 | new Pro ... );\\n }) | promises.js:135:15:137:3 | functio ... a);\\n } |
98+
| promises.js:148:10:148:49 | new Pro ... ect){}) | promises.js:148:22:148:48 | functio ... ject){} |
8599
test_PromiseDefinition_getAFinallyHandler
86100
| flow.js:105:2:105:48 | new Pro ... "BLA")) | flow.js:105:58:105:76 | x => {throw source} |
87101
| flow.js:109:2:109:48 | new Pro ... "BLA")) | flow.js:109:58:109:70 | x => rejected |
@@ -117,6 +131,12 @@ test_PromiseDefinition
117131
| promises.js:10:18:17:4 | new Pro ... );\\n }) |
118132
| promises.js:33:19:35:6 | new Pro ... \\n }) |
119133
| promises.js:43:19:45:6 | Q.Promi ... \\n }) |
134+
| promises.js:88:17:90:4 | Q.Promi ... );\\n }) |
135+
| promises.js:112:17:112:62 | new RSV ... ct) {}) |
136+
| promises.js:124:19:124:30 | when(source) |
137+
| promises.js:130:14:130:69 | new Pro ... s'); }) |
138+
| promises.js:135:3:137:4 | new Pro ... );\\n }) |
139+
| promises.js:148:10:148:49 | new Pro ... ect){}) |
120140
test_PromiseDefinition_getAResolveHandler
121141
| flow.js:24:2:24:49 | new Pro ... ource)) | flow.js:24:56:24:67 | x => sink(x) |
122142
| flow.js:26:2:26:49 | new Pro ... ource)) | flow.js:26:56:26:66 | x => foo(x) |
@@ -134,6 +154,7 @@ test_PromiseDefinition_getAResolveHandler
134154
| promises.js:10:18:17:4 | new Pro ... );\\n }) | promises.js:26:20:28:3 | (v) => ... v;\\n } |
135155
| promises.js:33:19:35:6 | new Pro ... \\n }) | promises.js:36:18:38:5 | functio ... ;\\n } |
136156
| promises.js:43:19:45:6 | Q.Promi ... \\n }) | promises.js:46:18:48:5 | functio ... ;\\n } |
157+
| promises.js:88:17:90:4 | Q.Promi ... );\\n }) | promises.js:91:16:93:3 | functio ... al;\\n } |
137158
test_PromiseDefinition_getRejectParameter
138159
| flow.js:7:11:7:59 | new Pro ... ource)) | flow.js:7:33:7:38 | reject |
139160
| flow.js:10:11:10:58 | new Pro ... ource)) | flow.js:10:33:10:38 | reject |
@@ -164,6 +185,10 @@ test_PromiseDefinition_getRejectParameter
164185
| promises.js:10:18:17:4 | new Pro ... );\\n }) | promises.js:10:36:10:38 | rej |
165186
| promises.js:33:19:35:6 | new Pro ... \\n }) | promises.js:33:50:33:55 | reject |
166187
| promises.js:43:19:45:6 | Q.Promi ... \\n }) | promises.js:43:48:43:53 | reject |
188+
| promises.js:88:17:90:4 | Q.Promi ... );\\n }) | promises.js:88:46:88:51 | reject |
189+
| promises.js:112:17:112:62 | new RSV ... ct) {}) | promises.js:112:52:112:57 | reject |
190+
| promises.js:135:3:137:4 | new Pro ... );\\n }) | promises.js:135:34:135:39 | reject |
191+
| promises.js:148:10:148:49 | new Pro ... ect){}) | promises.js:148:40:148:45 | reject |
167192
test_PromiseDefinition_getResolveParameter
168193
| flow.js:7:11:7:59 | new Pro ... ource)) | flow.js:7:24:7:30 | resolve |
169194
| flow.js:10:11:10:58 | new Pro ... ource)) | flow.js:10:24:10:30 | resolve |
@@ -194,6 +219,11 @@ test_PromiseDefinition_getResolveParameter
194219
| promises.js:10:18:17:4 | new Pro ... );\\n }) | promises.js:10:31:10:33 | res |
195220
| promises.js:33:19:35:6 | new Pro ... \\n }) | promises.js:33:41:33:47 | resolve |
196221
| promises.js:43:19:45:6 | Q.Promi ... \\n }) | promises.js:43:39:43:45 | resolve |
222+
| promises.js:88:17:90:4 | Q.Promi ... );\\n }) | promises.js:88:37:88:43 | resolve |
223+
| promises.js:112:17:112:62 | new RSV ... ct) {}) | promises.js:112:43:112:49 | resolve |
224+
| promises.js:130:14:130:69 | new Pro ... s'); }) | promises.js:130:36:130:42 | resolve |
225+
| promises.js:135:3:137:4 | new Pro ... );\\n }) | promises.js:135:25:135:31 | resolve |
226+
| promises.js:148:10:148:49 | new Pro ... ect){}) | promises.js:148:31:148:37 | resolve |
197227
test_PromiseDefinition_getACatchHandler
198228
| flow.js:32:2:32:49 | new Pro ... ource)) | flow.js:32:57:32:68 | x => sink(x) |
199229
| flow.js:48:2:48:36 | new Pro ... urce }) | flow.js:48:44:48:55 | x => sink(x) |
@@ -400,3 +430,25 @@ typetrack
400430
| promises.js:71:34:71:36 | val | promises.js:71:5:71:27 | Promise ... source) | load $PromiseResolveField$ |
401431
| promises.js:72:48:72:50 | val | promises.js:72:5:72:41 | new Pro ... ource)) | load $PromiseResolveField$ |
402432
| promises.js:75:27:75:29 | val | promises.js:75:5:75:20 | resolver.promise | load $PromiseResolveField$ |
433+
| promises.js:88:17:90:4 | Q.Promi ... );\\n }) | promises.js:89:15:89:20 | source | copy $PromiseResolveField$ |
434+
| promises.js:88:17:90:4 | Q.Promi ... );\\n }) | promises.js:89:15:89:20 | source | store $PromiseResolveField$ |
435+
| promises.js:99:3:99:26 | Promise ... source) | promises.js:99:20:99:25 | source | copy $PromiseResolveField$ |
436+
| promises.js:99:3:99:26 | Promise ... source) | promises.js:99:20:99:25 | source | store $PromiseResolveField$ |
437+
| promises.js:100:3:100:26 | Promise ... source) | promises.js:100:20:100:25 | source | copy $PromiseResolveField$ |
438+
| promises.js:100:3:100:26 | Promise ... source) | promises.js:100:20:100:25 | source | store $PromiseResolveField$ |
439+
| promises.js:106:3:106:26 | Promise ... source) | promises.js:106:20:106:25 | source | copy $PromiseResolveField$ |
440+
| promises.js:106:3:106:26 | Promise ... source) | promises.js:106:20:106:25 | source | store $PromiseResolveField$ |
441+
| promises.js:107:3:107:26 | Promise ... source) | promises.js:107:20:107:25 | source | copy $PromiseResolveField$ |
442+
| promises.js:107:3:107:26 | Promise ... source) | promises.js:107:20:107:25 | source | store $PromiseResolveField$ |
443+
| promises.js:114:3:114:25 | Promise ... source) | promises.js:114:19:114:24 | source | copy $PromiseResolveField$ |
444+
| promises.js:114:3:114:25 | Promise ... source) | promises.js:114:19:114:24 | source | store $PromiseResolveField$ |
445+
| promises.js:119:3:119:25 | Promise ... source) | promises.js:119:19:119:24 | source | copy $PromiseResolveField$ |
446+
| promises.js:119:3:119:25 | Promise ... source) | promises.js:119:19:119:24 | source | store $PromiseResolveField$ |
447+
| promises.js:125:20:125:39 | when.resolve(source) | promises.js:125:33:125:38 | source | copy $PromiseResolveField$ |
448+
| promises.js:125:20:125:39 | when.resolve(source) | promises.js:125:33:125:38 | source | store $PromiseResolveField$ |
449+
| promises.js:135:3:137:4 | new Pro ... );\\n }) | promises.js:136:13:136:16 | data | copy $PromiseResolveField$ |
450+
| promises.js:135:3:137:4 | new Pro ... );\\n }) | promises.js:136:13:136:16 | data | store $PromiseResolveField$ |
451+
| promises.js:143:17:143:50 | Synchro ... source) | promises.js:143:44:143:49 | source | copy $PromiseResolveField$ |
452+
| promises.js:143:17:143:50 | Synchro ... source) | promises.js:143:44:143:49 | source | store $PromiseResolveField$ |
453+
| promises.js:153:17:153:39 | Promise ... source) | promises.js:153:33:153:38 | source | copy $PromiseResolveField$ |
454+
| promises.js:153:17:153:39 | Promise ... source) | promises.js:153:33:153:38 | source | store $PromiseResolveField$ |

0 commit comments

Comments
 (0)