Skip to content

Commit ac29d5f

Browse files
codeql-cicodeql-ci
authored
Merge pull request #8523 from asgerf/js/api-graph-receiver-label
Approved by erik-krogh
2 parents 8b8f0ca + f228570 commit ac29d5f

File tree

6 files changed

+45
-35
lines changed

6 files changed

+45
-35
lines changed

‎javascript/ql/lib/change-notes/2022-03-23-api-graph-receiver-label.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
category: fix
3+
---
4+
* The following predicates on `API::Node` have been changed so as not to include the receiver. The receiver should now only be accessed via `getReceiver()`.
5+
- `getParameter(int i)` previously included the receiver when `i = -1`
6+
- `getAParameter()` previously included the receiver
7+
- `getLastParameter()` previously included the receiver for calls with no arguments

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

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,7 @@ module API {
187187
}
188188

189189
/**
190-
* Gets a node representing a parameter or the receiver of the function represented by this
191-
* node.
190+
* Gets a node representing a parameter of the function represented by this node.
192191
*
193192
* This predicate may result in a mix of parameters from different call sites in cases where
194193
* there are multiple invocations of this API component.
@@ -198,8 +197,6 @@ module API {
198197
Node getAParameter() {
199198
Stages::ApiStage::ref() and
200199
result = this.getParameter(_)
201-
or
202-
result = this.getReceiver()
203200
}
204201

205202
/**
@@ -561,9 +558,10 @@ module API {
561558
rhs = f.getExceptionalReturn()
562559
)
563560
or
564-
exists(int i |
565-
lbl = Label::parameter(i) and
566-
argumentPassing(base, i, rhs)
561+
exists(int i | argumentPassing(base, i, rhs) |
562+
lbl = Label::parameter(i)
563+
or
564+
i = -1 and lbl = Label::receiver()
567565
)
568566
or
569567
exists(DataFlow::SourceNode src, DataFlow::PropWrite pw |
@@ -1096,8 +1094,8 @@ module API {
10961094
*/
10971095
LabelParameter parameter(int i) { result.getIndex() = i }
10981096

1099-
/** Gets the `parameter` edge label for the receiver. */
1100-
LabelParameter receiver() { result = parameter(-1) }
1097+
/** Gets the edge label for the receiver. */
1098+
LabelReceiver receiver() { any() }
11011099

11021100
/** Gets the `return` edge label. */
11031101
LabelReturn return() { any() }
@@ -1132,12 +1130,13 @@ module API {
11321130
MkLabelUnknownMember() or
11331131
MkLabelParameter(int i) {
11341132
i =
1135-
[-1 .. max(int args |
1133+
[0 .. max(int args |
11361134
args = any(InvokeExpr invk).getNumArgument() or
11371135
args = any(Function f).getNumParameter()
11381136
)] or
11391137
i = [0 .. 10]
11401138
} or
1139+
MkLabelReceiver() or
11411140
MkLabelReturn() or
11421141
MkLabelPromised() or
11431142
MkLabelPromisedError() or
@@ -1225,6 +1224,11 @@ module API {
12251224
/** Gets the index of the parameter for this label. */
12261225
int getIndex() { result = i }
12271226
}
1227+
1228+
/** A label for the receiver of call, that is, the value passed as `this`. */
1229+
class LabelReceiver extends ApiLabel, MkLabelReceiver {
1230+
override string toString() { result = "receiver" }
1231+
}
12281232
}
12291233
}
12301234
}

‎javascript/ql/lib/semmle/javascript/security/dataflow/ExternalAPIUsedWithUntrustedDataCustomizations.qll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,6 @@ module ExternalApiUsedWithUntrustedData {
219219
or
220220
exists(string callbackName, int index |
221221
node = getNamedParameter(base.getParameter(index).getMember(callbackName), paramName) and
222-
index != -1 and // ignore receiver
223222
result =
224223
basename + ".[callback " + index + " '" + callbackName + "'].[param '" + paramName +
225224
"']"

‎javascript/ql/test/ApiGraphs/bound-args/index.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
import bar from 'foo';
22

33
let boundbar = bar.bind(
4-
"receiver", // def (parameter -1 (member default (member exports (module foo))))
4+
"receiver", // def (receiver (member default (member exports (module foo))))
55
"firstarg" // def (parameter 0 (member default (member exports (module foo))))
66
);
77
boundbar(
88
"secondarg" // def (parameter 1 (member default (member exports (module foo))))
99
)
1010

1111
let boundbar2 = boundbar.bind(
12-
"ignored", // !def (parameter -1 (member default (member exports (module foo))))
12+
"ignored", // !def (receiver (member default (member exports (module foo))))
1313
"othersecondarg" // def (parameter 1 (member default (member exports (module foo))))
1414
)
1515
boundbar2(

‎javascript/ql/test/ApiGraphs/partial-invoke/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ const cp = require('child_process');
22

33
module.exports = function () {
44
return cp.spawn.bind(
5-
cp, // def (parameter -1 (member spawn (member exports (module child_process))))
5+
cp, // def (receiver (member spawn (member exports (module child_process))))
66
"cat" // def (parameter 0 (member spawn (member exports (module child_process))))
77
);
88
};

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

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@ knexObject
4242
| tst.js:17:1:17:23 | use (return (member avg (return (member exports (module knex))))) |
4343
| tst.js:17:1:19:4 | use (return (member from (return (member avg (return (member exports (module knex))))))) |
4444
| tst.js:17:1:19:24 | use (return (member as (return (member from (return (member avg (return (member exports (module knex))))))))) |
45-
| tst.js:17:30:17:29 | use (parameter -1 (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))) |
46-
| tst.js:18:5:18:38 | use (return (member sum (parameter -1 (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))))) |
47-
| tst.js:18:5:18:49 | use (return (member from (return (member sum (parameter -1 (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))))))) |
48-
| tst.js:18:5:18:68 | use (return (member groupBy (return (member from (return (member sum (parameter -1 (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))))))))) |
49-
| tst.js:18:5:18:77 | use (return (member as (return (member groupBy (return (member from (return (member sum (parameter -1 (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))))))))))) |
45+
| tst.js:17:30:17:29 | use (receiver (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))) |
46+
| tst.js:18:5:18:38 | use (return (member sum (receiver (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))))) |
47+
| tst.js:18:5:18:49 | use (return (member from (return (member sum (receiver (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))))))) |
48+
| tst.js:18:5:18:68 | use (return (member groupBy (return (member from (return (member sum (receiver (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))))))))) |
49+
| tst.js:18:5:18:77 | use (return (member as (return (member groupBy (return (member from (return (member sum (receiver (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))))))))))) |
5050
| tst.js:21:1:21:38 | use (return (member column (return (member exports (module knex))))) |
5151
| tst.js:21:1:21:47 | use (return (member select (return (member column (return (member exports (module knex))))))) |
5252
| tst.js:21:1:21:61 | use (return (member from (return (member select (return (member column (return (member exports (module knex))))))))) |
@@ -70,14 +70,14 @@ knexObject
7070
| tst.js:42:1:42:13 | use (return (return (member exports (module knex)))) |
7171
| tst.js:42:1:45:3 | use (return (member where (return (return (member exports (module knex)))))) |
7272
| tst.js:42:1:48:4 | use (return (member andWhere (return (member where (return (return (member exports (module knex)))))))) |
73-
| tst.js:46:13:46:12 | use (parameter -1 (parameter 0 (member andWhere (return (member where (return (return (member exports (module knex))))))))) |
74-
| tst.js:47:5:47:29 | use (return (member where (parameter -1 (parameter 0 (member andWhere (return (member where (return (return (member exports (module knex))))))))))) |
73+
| tst.js:46:13:46:12 | use (receiver (parameter 0 (member andWhere (return (member where (return (return (member exports (module knex))))))))) |
74+
| tst.js:47:5:47:29 | use (return (member where (receiver (parameter 0 (member andWhere (return (member where (return (return (member exports (module knex))))))))))) |
7575
| tst.js:50:1:50:13 | use (return (return (member exports (module knex)))) |
7676
| tst.js:50:1:52:2 | use (return (member where (return (return (member exports (module knex)))))) |
7777
| tst.js:50:1:52:28 | use (return (member orWhere (return (member where (return (return (member exports (module knex)))))))) |
78-
| tst.js:50:21:50:20 | use (parameter -1 (parameter 0 (member where (return (return (member exports (module knex))))))) |
79-
| tst.js:51:3:51:21 | use (return (member where (parameter -1 (parameter 0 (member where (return (return (member exports (module knex))))))))) |
80-
| tst.js:51:3:51:44 | use (return (member orWhere (return (member where (parameter -1 (parameter 0 (member where (return (return (member exports (module knex))))))))))) |
78+
| tst.js:50:21:50:20 | use (receiver (parameter 0 (member where (return (return (member exports (module knex))))))) |
79+
| tst.js:51:3:51:21 | use (return (member where (receiver (parameter 0 (member where (return (return (member exports (module knex))))))))) |
80+
| tst.js:51:3:51:44 | use (return (member orWhere (return (member where (receiver (parameter 0 (member where (return (return (member exports (module knex))))))))))) |
8181
| tst.js:54:1:54:13 | use (return (return (member exports (module knex)))) |
8282
| tst.js:54:1:54:56 | use (return (member where (return (return (member exports (module knex)))))) |
8383
| tst.js:56:1:56:13 | use (return (return (member exports (module knex)))) |
@@ -100,9 +100,9 @@ knexObject
100100
| tst.js:70:1:70:13 | use (return (return (member exports (module knex)))) |
101101
| tst.js:70:1:72:2 | use (return (member whereNot (return (return (member exports (module knex)))))) |
102102
| tst.js:70:1:72:31 | use (return (member orWhereNot (return (member whereNot (return (return (member exports (module knex)))))))) |
103-
| tst.js:70:24:70:23 | use (parameter -1 (parameter 0 (member whereNot (return (return (member exports (module knex))))))) |
104-
| tst.js:71:3:71:21 | use (return (member where (parameter -1 (parameter 0 (member whereNot (return (return (member exports (module knex))))))))) |
105-
| tst.js:71:3:71:47 | use (return (member orWhereNot (return (member where (parameter -1 (parameter 0 (member whereNot (return (return (member exports (module knex))))))))))) |
103+
| tst.js:70:24:70:23 | use (receiver (parameter 0 (member whereNot (return (return (member exports (module knex))))))) |
104+
| tst.js:71:3:71:21 | use (return (member where (receiver (parameter 0 (member whereNot (return (return (member exports (module knex))))))))) |
105+
| tst.js:71:3:71:47 | use (return (member orWhereNot (return (member where (receiver (parameter 0 (member whereNot (return (return (member exports (module knex))))))))))) |
106106
| tst.js:74:19:74:31 | use (return (return (member exports (module knex)))) |
107107
| tst.js:74:19:75:30 | use (return (member whereNot (return (return (member exports (module knex)))))) |
108108
| tst.js:74:19:76:31 | use (return (member andWhere (return (member whereNot (return (return (member exports (module knex)))))))) |
@@ -128,21 +128,21 @@ knexObject
128128
| tst.js:97:1:97:40 | use (return (member whereNotNull (return (return (member exports (module knex)))))) |
129129
| tst.js:99:1:99:13 | use (return (return (member exports (module knex)))) |
130130
| tst.js:99:1:101:2 | use (return (member whereExists (return (return (member exports (module knex)))))) |
131-
| tst.js:99:27:99:26 | use (parameter -1 (parameter 0 (member whereExists (return (return (member exports (module knex))))))) |
132-
| tst.js:100:3:100:18 | use (return (member select (parameter -1 (parameter 0 (member whereExists (return (return (member exports (module knex))))))))) |
133-
| tst.js:100:3:100:35 | use (return (member from (return (member select (parameter -1 (parameter 0 (member whereExists (return (return (member exports (module knex))))))))))) |
134-
| tst.js:100:3:100:78 | use (return (member whereRaw (return (member from (return (member select (parameter -1 (parameter 0 (member whereExists (return (return (member exports (module knex))))))))))))) |
131+
| tst.js:99:27:99:26 | use (receiver (parameter 0 (member whereExists (return (return (member exports (module knex))))))) |
132+
| tst.js:100:3:100:18 | use (return (member select (receiver (parameter 0 (member whereExists (return (return (member exports (module knex))))))))) |
133+
| tst.js:100:3:100:35 | use (return (member from (return (member select (receiver (parameter 0 (member whereExists (return (return (member exports (module knex))))))))))) |
134+
| tst.js:100:3:100:78 | use (return (member whereRaw (return (member from (return (member select (receiver (parameter 0 (member whereExists (return (return (member exports (module knex))))))))))))) |
135135
| tst.js:103:1:103:13 | use (return (return (member exports (module knex)))) |
136136
| tst.js:103:1:103:103 | use (return (member whereExists (return (return (member exports (module knex)))))) |
137137
| tst.js:103:27:103:42 | use (return (member select (return (member exports (module knex))))) |
138138
| tst.js:103:27:103:59 | use (return (member from (return (member select (return (member exports (module knex))))))) |
139139
| tst.js:103:27:103:102 | use (return (member whereRaw (return (member from (return (member select (return (member exports (module knex))))))))) |
140140
| tst.js:105:1:105:13 | use (return (return (member exports (module knex)))) |
141141
| tst.js:105:1:107:2 | use (return (member whereNotExists (return (return (member exports (module knex)))))) |
142-
| tst.js:105:30:105:29 | use (parameter -1 (parameter 0 (member whereNotExists (return (return (member exports (module knex))))))) |
143-
| tst.js:106:3:106:18 | use (return (member select (parameter -1 (parameter 0 (member whereNotExists (return (return (member exports (module knex))))))))) |
144-
| tst.js:106:3:106:35 | use (return (member from (return (member select (parameter -1 (parameter 0 (member whereNotExists (return (return (member exports (module knex))))))))))) |
145-
| tst.js:106:3:106:78 | use (return (member whereRaw (return (member from (return (member select (parameter -1 (parameter 0 (member whereNotExists (return (return (member exports (module knex))))))))))))) |
142+
| tst.js:105:30:105:29 | use (receiver (parameter 0 (member whereNotExists (return (return (member exports (module knex))))))) |
143+
| tst.js:106:3:106:18 | use (return (member select (receiver (parameter 0 (member whereNotExists (return (return (member exports (module knex))))))))) |
144+
| tst.js:106:3:106:35 | use (return (member from (return (member select (receiver (parameter 0 (member whereNotExists (return (return (member exports (module knex))))))))))) |
145+
| tst.js:106:3:106:78 | use (return (member whereRaw (return (member from (return (member select (receiver (parameter 0 (member whereNotExists (return (return (member exports (module knex))))))))))))) |
146146
| tst.js:109:1:109:13 | use (return (return (member exports (module knex)))) |
147147
| tst.js:109:1:109:45 | use (return (member whereBetween (return (return (member exports (module knex)))))) |
148148
| tst.js:111:1:111:13 | use (return (return (member exports (module knex)))) |

0 commit comments

Comments
 (0)