Skip to content

Commit 9050f99

Browse files
committed
recognize functions that return object of methods as library input
1 parent e0c74d4 commit 9050f99

File tree

3 files changed

+34
-9
lines changed

3 files changed

+34
-9
lines changed

javascript/ql/lib/semmle/javascript/PackageExports.qll

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -156,17 +156,22 @@ private DataFlow::Node getAValueExportedByPackage() {
156156
result = unique( | | call.getCalleeNode().getAFunctionValue()).getAReturn()
157157
)
158158
or
159-
// the exported value is a function that returns another import.
160-
// ```JavaScript
161-
// module.exports = function foo() {
162-
// return require("./other-module.js");
163-
// }
164-
// ```
165-
exists(DataFlow::FunctionNode func, Module mod |
159+
exists(DataFlow::FunctionNode func |
166160
func = getAValueExportedByPackage().getABoundFunctionValue(_)
167161
|
168-
mod = func.getAReturn().getALocalSource().getEnclosingExpr().(Import).getImportedModule() and
169-
result = getAnExportFromModule(mod)
162+
// the exported value is a function that returns another import.
163+
// ```JavaScript
164+
// module.exports = function foo() {
165+
// return require("./other-module.js");
166+
// }
167+
// ```
168+
exists(Module mod |
169+
mod = func.getAReturn().getALocalSource().getEnclosingExpr().(Import).getImportedModule() and
170+
result = getAnExportFromModule(mod)
171+
)
172+
or
173+
// a function that returns an object of methods. This acts a bit like a class.
174+
result = func.getAReturn().getALocalSource().getAPropertySource().(DataFlow::FunctionNode)
170175
)
171176
or
172177
// *****

javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingAssignment/PrototypePollutingAssignment.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,12 @@ nodes
9494
| lib.js:108:3:108:10 | obj[one] |
9595
| lib.js:108:3:108:10 | obj[one] |
9696
| lib.js:108:7:108:9 | one |
97+
| lib.js:118:29:118:32 | path |
98+
| lib.js:118:29:118:32 | path |
99+
| lib.js:119:13:119:24 | obj[path[0]] |
100+
| lib.js:119:13:119:24 | obj[path[0]] |
101+
| lib.js:119:17:119:20 | path |
102+
| lib.js:119:17:119:23 | path[0] |
97103
| tst.js:5:9:5:38 | taint |
98104
| tst.js:5:17:5:38 | String( ... y.data) |
99105
| tst.js:5:24:5:37 | req.query.data |
@@ -230,6 +236,11 @@ edges
230236
| lib.js:104:13:104:24 | arguments[1] | lib.js:104:7:104:24 | one |
231237
| lib.js:108:7:108:9 | one | lib.js:108:3:108:10 | obj[one] |
232238
| lib.js:108:7:108:9 | one | lib.js:108:3:108:10 | obj[one] |
239+
| lib.js:118:29:118:32 | path | lib.js:119:17:119:20 | path |
240+
| lib.js:118:29:118:32 | path | lib.js:119:17:119:20 | path |
241+
| lib.js:119:17:119:20 | path | lib.js:119:17:119:23 | path[0] |
242+
| lib.js:119:17:119:23 | path[0] | lib.js:119:13:119:24 | obj[path[0]] |
243+
| lib.js:119:17:119:23 | path[0] | lib.js:119:13:119:24 | obj[path[0]] |
233244
| tst.js:5:9:5:38 | taint | tst.js:8:12:8:16 | taint |
234245
| tst.js:5:9:5:38 | taint | tst.js:9:12:9:16 | taint |
235246
| tst.js:5:9:5:38 | taint | tst.js:12:25:12:29 | taint |
@@ -284,6 +295,7 @@ edges
284295
| lib.js:70:13:70:24 | obj[path[0]] | lib.js:59:18:59:18 | s | lib.js:70:13:70:24 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:59:18:59:18 | s | library input |
285296
| lib.js:87:10:87:14 | proto | lib.js:83:14:83:25 | arguments[1] | lib.js:87:10:87:14 | proto | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:83:14:83:25 | arguments[1] | library input |
286297
| lib.js:108:3:108:10 | obj[one] | lib.js:104:13:104:24 | arguments[1] | lib.js:108:3:108:10 | obj[one] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:104:13:104:24 | arguments[1] | library input |
298+
| lib.js:119:13:119:24 | obj[path[0]] | lib.js:118:29:118:32 | path | lib.js:119:13:119:24 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:118:29:118:32 | path | library input |
287299
| tst.js:8:5:8:17 | object[taint] | tst.js:5:24:5:37 | req.query.data | tst.js:8:5:8:17 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |
288300
| tst.js:9:5:9:17 | object[taint] | tst.js:5:24:5:37 | req.query.data | tst.js:9:5:9:17 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |
289301
| tst.js:14:5:14:32 | unsafeG ... taint) | tst.js:5:24:5:37 | req.query.data | tst.js:14:5:14:32 | unsafeG ... taint) | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |

javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingAssignment/lib.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,3 +112,11 @@ module.exports.sanWithFcuntion = function() {
112112
}
113113
obj[one][two] = value; // OK
114114
}
115+
116+
module.exports.returnsObj = function () {
117+
return {
118+
set: function (obj, path, value) {
119+
obj[path[0]][path[1]] = value; // NOT OK
120+
}
121+
}
122+
}

0 commit comments

Comments
 (0)