Skip to content

JS: Add dedicated API graph label for the receiver, instead of parameter -1 #8523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 23, 2022
Merged

JS: Add dedicated API graph label for the receiver, instead of parameter -1 #8523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6bef5a7
JS: Add dedicated API graph label for receiver, instead of parameter -1
asgerf Mar 22, 2022
73071bd
JS: Change getAParameter to not return the receiver
asgerf Mar 22, 2022
59d5c54
JS: Update test output from knex
asgerf Mar 22, 2022
f228570
JS: Change note
asgerf Mar 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions javascript/ql/lib/change-notes/2022-03-23-api-graph-receiver-label.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
category: fix
---
* 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()`.
- `getParameter(int i)` previously included the receiver when `i = -1`
- `getAParameter()` previously included the receiver
- `getLastParameter()` previously included the receiver for calls with no arguments
24 changes: 14 additions & 10 deletions javascript/ql/lib/semmle/javascript/ApiGraphs.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,7 @@ module API {
}

/**
* Gets a node representing a parameter or the receiver of the function represented by this
* node.
* Gets a node representing a parameter of the function represented by this node.
*
* This predicate may result in a mix of parameters from different call sites in cases where
* there are multiple invocations of this API component.
Expand All @@ -198,8 +197,6 @@ module API {
Node getAParameter() {
Stages::ApiStage::ref() and
result = this.getParameter(_)
or
result = this.getReceiver()
}

/**
Expand Down Expand Up @@ -561,9 +558,10 @@ module API {
rhs = f.getExceptionalReturn()
)
or
exists(int i |
lbl = Label::parameter(i) and
argumentPassing(base, i, rhs)
exists(int i | argumentPassing(base, i, rhs) |
lbl = Label::parameter(i)
or
i = -1 and lbl = Label::receiver()
)
or
exists(DataFlow::SourceNode src, DataFlow::PropWrite pw |
Expand Down Expand Up @@ -1096,8 +1094,8 @@ module API {
*/
LabelParameter parameter(int i) { result.getIndex() = i }

/** Gets the `parameter` edge label for the receiver. */
LabelParameter receiver() { result = parameter(-1) }
/** Gets the edge label for the receiver. */
LabelReceiver receiver() { any() }

/** Gets the `return` edge label. */
LabelReturn return() { any() }
Expand Down Expand Up @@ -1132,12 +1130,13 @@ module API {
MkLabelUnknownMember() or
MkLabelParameter(int i) {
i =
[-1 .. max(int args |
[0 .. max(int args |
args = any(InvokeExpr invk).getNumArgument() or
args = any(Function f).getNumParameter()
)] or
i = [0 .. 10]
} or
MkLabelReceiver() or
MkLabelReturn() or
MkLabelPromised() or
MkLabelPromisedError() or
Expand Down Expand Up @@ -1225,6 +1224,11 @@ module API {
/** Gets the index of the parameter for this label. */
int getIndex() { result = i }
}

/** A label for the receiver of call, that is, the value passed as `this`. */
class LabelReceiver extends ApiLabel, MkLabelReceiver {
override string toString() { result = "receiver" }
}
}
}
}
Expand Down
1 change: 0 additions & 1 deletion ...ib/semmle/javascript/security/dataflow/ExternalAPIUsedWithUntrustedDataCustomizations.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,6 @@ module ExternalApiUsedWithUntrustedData {
or
exists(string callbackName, int index |
node = getNamedParameter(base.getParameter(index).getMember(callbackName), paramName) and
index != -1 and // ignore receiver
result =
basename + ".[callback " + index + " '" + callbackName + "'].[param '" + paramName +
"']"
Expand Down
4 changes: 2 additions & 2 deletions javascript/ql/test/ApiGraphs/bound-args/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import bar from 'foo';

let boundbar = bar.bind(
"receiver", // def (parameter -1 (member default (member exports (module foo))))
"receiver", // def (receiver (member default (member exports (module foo))))
"firstarg" // def (parameter 0 (member default (member exports (module foo))))
);
boundbar(
"secondarg" // def (parameter 1 (member default (member exports (module foo))))
)

let boundbar2 = boundbar.bind(
"ignored", // !def (parameter -1 (member default (member exports (module foo))))
"ignored", // !def (receiver (member default (member exports (module foo))))
"othersecondarg" // def (parameter 1 (member default (member exports (module foo))))
)
boundbar2(
Expand Down
2 changes: 1 addition & 1 deletion javascript/ql/test/ApiGraphs/partial-invoke/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const cp = require('child_process');

module.exports = function () {
return cp.spawn.bind(
cp, // def (parameter -1 (member spawn (member exports (module child_process))))
cp, // def (receiver (member spawn (member exports (module child_process))))
"cat" // def (parameter 0 (member spawn (member exports (module child_process))))
);
};
42 changes: 21 additions & 21 deletions javascript/ql/test/library-tests/frameworks/Knex/test.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ knexObject
| tst.js:17:1:17:23 | use (return (member avg (return (member exports (module knex))))) |
| tst.js:17:1:19:4 | use (return (member from (return (member avg (return (member exports (module knex))))))) |
| tst.js:17:1:19:24 | use (return (member as (return (member from (return (member avg (return (member exports (module knex))))))))) |
| tst.js:17:30:17:29 | use (parameter -1 (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))) |
| tst.js:18:5:18:38 | use (return (member sum (parameter -1 (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))))) |
| 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)))))))))))) |
| 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)))))))))))))) |
| 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)))))))))))))))) |
| tst.js:17:30:17:29 | use (receiver (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))) |
| tst.js:18:5:18:38 | use (return (member sum (receiver (parameter 0 (member from (return (member avg (return (member exports (module knex)))))))))) |
| 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)))))))))))) |
| 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)))))))))))))) |
| 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)))))))))))))))) |
| tst.js:21:1:21:38 | use (return (member column (return (member exports (module knex))))) |
| tst.js:21:1:21:47 | use (return (member select (return (member column (return (member exports (module knex))))))) |
| tst.js:21:1:21:61 | use (return (member from (return (member select (return (member column (return (member exports (module knex))))))))) |
Expand All @@ -70,14 +70,14 @@ knexObject
| tst.js:42:1:42:13 | use (return (return (member exports (module knex)))) |
| tst.js:42:1:45:3 | use (return (member where (return (return (member exports (module knex)))))) |
| tst.js:42:1:48:4 | use (return (member andWhere (return (member where (return (return (member exports (module knex)))))))) |
| tst.js:46:13:46:12 | use (parameter -1 (parameter 0 (member andWhere (return (member where (return (return (member exports (module knex))))))))) |
| 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))))))))))) |
| tst.js:46:13:46:12 | use (receiver (parameter 0 (member andWhere (return (member where (return (return (member exports (module knex))))))))) |
| tst.js:47:5:47:29 | use (return (member where (receiver (parameter 0 (member andWhere (return (member where (return (return (member exports (module knex))))))))))) |
| tst.js:50:1:50:13 | use (return (return (member exports (module knex)))) |
| tst.js:50:1:52:2 | use (return (member where (return (return (member exports (module knex)))))) |
| tst.js:50:1:52:28 | use (return (member orWhere (return (member where (return (return (member exports (module knex)))))))) |
| tst.js:50:21:50:20 | use (parameter -1 (parameter 0 (member where (return (return (member exports (module knex))))))) |
| tst.js:51:3:51:21 | use (return (member where (parameter -1 (parameter 0 (member where (return (return (member exports (module knex))))))))) |
| 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))))))))))) |
| tst.js:50:21:50:20 | use (receiver (parameter 0 (member where (return (return (member exports (module knex))))))) |
| tst.js:51:3:51:21 | use (return (member where (receiver (parameter 0 (member where (return (return (member exports (module knex))))))))) |
| tst.js:51:3:51:44 | use (return (member orWhere (return (member where (receiver (parameter 0 (member where (return (return (member exports (module knex))))))))))) |
| tst.js:54:1:54:13 | use (return (return (member exports (module knex)))) |
| tst.js:54:1:54:56 | use (return (member where (return (return (member exports (module knex)))))) |
| tst.js:56:1:56:13 | use (return (return (member exports (module knex)))) |
Expand All @@ -100,9 +100,9 @@ knexObject
| tst.js:70:1:70:13 | use (return (return (member exports (module knex)))) |
| tst.js:70:1:72:2 | use (return (member whereNot (return (return (member exports (module knex)))))) |
| tst.js:70:1:72:31 | use (return (member orWhereNot (return (member whereNot (return (return (member exports (module knex)))))))) |
| tst.js:70:24:70:23 | use (parameter -1 (parameter 0 (member whereNot (return (return (member exports (module knex))))))) |
| tst.js:71:3:71:21 | use (return (member where (parameter -1 (parameter 0 (member whereNot (return (return (member exports (module knex))))))))) |
| 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))))))))))) |
| tst.js:70:24:70:23 | use (receiver (parameter 0 (member whereNot (return (return (member exports (module knex))))))) |
| tst.js:71:3:71:21 | use (return (member where (receiver (parameter 0 (member whereNot (return (return (member exports (module knex))))))))) |
| tst.js:71:3:71:47 | use (return (member orWhereNot (return (member where (receiver (parameter 0 (member whereNot (return (return (member exports (module knex))))))))))) |
| tst.js:74:19:74:31 | use (return (return (member exports (module knex)))) |
| tst.js:74:19:75:30 | use (return (member whereNot (return (return (member exports (module knex)))))) |
| tst.js:74:19:76:31 | use (return (member andWhere (return (member whereNot (return (return (member exports (module knex)))))))) |
Expand All @@ -128,21 +128,21 @@ knexObject
| tst.js:97:1:97:40 | use (return (member whereNotNull (return (return (member exports (module knex)))))) |
| tst.js:99:1:99:13 | use (return (return (member exports (module knex)))) |
| tst.js:99:1:101:2 | use (return (member whereExists (return (return (member exports (module knex)))))) |
| tst.js:99:27:99:26 | use (parameter -1 (parameter 0 (member whereExists (return (return (member exports (module knex))))))) |
| tst.js:100:3:100:18 | use (return (member select (parameter -1 (parameter 0 (member whereExists (return (return (member exports (module knex))))))))) |
| 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))))))))))) |
| 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))))))))))))) |
| tst.js:99:27:99:26 | use (receiver (parameter 0 (member whereExists (return (return (member exports (module knex))))))) |
| tst.js:100:3:100:18 | use (return (member select (receiver (parameter 0 (member whereExists (return (return (member exports (module knex))))))))) |
| tst.js:100:3:100:35 | use (return (member from (return (member select (receiver (parameter 0 (member whereExists (return (return (member exports (module knex))))))))))) |
| 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))))))))))))) |
| tst.js:103:1:103:13 | use (return (return (member exports (module knex)))) |
| tst.js:103:1:103:103 | use (return (member whereExists (return (return (member exports (module knex)))))) |
| tst.js:103:27:103:42 | use (return (member select (return (member exports (module knex))))) |
| tst.js:103:27:103:59 | use (return (member from (return (member select (return (member exports (module knex))))))) |
| tst.js:103:27:103:102 | use (return (member whereRaw (return (member from (return (member select (return (member exports (module knex))))))))) |
| tst.js:105:1:105:13 | use (return (return (member exports (module knex)))) |
| tst.js:105:1:107:2 | use (return (member whereNotExists (return (return (member exports (module knex)))))) |
| tst.js:105:30:105:29 | use (parameter -1 (parameter 0 (member whereNotExists (return (return (member exports (module knex))))))) |
| tst.js:106:3:106:18 | use (return (member select (parameter -1 (parameter 0 (member whereNotExists (return (return (member exports (module knex))))))))) |
| 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))))))))))) |
| 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))))))))))))) |
| tst.js:105:30:105:29 | use (receiver (parameter 0 (member whereNotExists (return (return (member exports (module knex))))))) |
| tst.js:106:3:106:18 | use (return (member select (receiver (parameter 0 (member whereNotExists (return (return (member exports (module knex))))))))) |
| tst.js:106:3:106:35 | use (return (member from (return (member select (receiver (parameter 0 (member whereNotExists (return (return (member exports (module knex))))))))))) |
| 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))))))))))))) |
| tst.js:109:1:109:13 | use (return (return (member exports (module knex)))) |
| tst.js:109:1:109:45 | use (return (member whereBetween (return (return (member exports (module knex)))))) |
| tst.js:111:1:111:13 | use (return (return (member exports (module knex)))) |
Expand Down