Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 55 additions & 1 deletion javascript/ql/src/semmle/javascript/PackageExports.qll
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,23 @@ DataFlow::ParameterNode getALibraryInputParameter() {
)
}

private import NodeModuleResolutionImpl as NodeModule

/**
* Gets a value exported by the main module from a named `package.json` file.
* The value is either directly the `module.exports` value, a nested property of `module.exports`, or a method on an exported class.
*/
private DataFlow::Node getAValueExportedByPackage() {
// The base case, an export from a named `package.json` file.
result =
getAnExportFromModule(any(PackageJSON pack | exists(pack.getPackageName())).getMainModule())
or
// module.exports.bar.baz = result;
result = getAValueExportedByPackage().(DataFlow::PropWrite).getRhs()
or
// class Foo {
// bar() {} // <- result
// };
// module.exports = new Foo();
exists(DataFlow::SourceNode callee |
callee = getAValueExportedByPackage().(DataFlow::NewNode).getCalleeNode().getALocalSource()
|
Expand All @@ -36,20 +43,67 @@ private DataFlow::Node getAValueExportedByPackage() {
or
result = getAValueExportedByPackage().getALocalSource()
or
// Nested property reads.
result = getAValueExportedByPackage().(DataFlow::SourceNode).getAPropertyReference()
or
// module.exports.foo = require("./other-module.js");
exists(Module mod |
mod = getAValueExportedByPackage().getEnclosingExpr().(Import).getImportedModule()
|
result = getAnExportFromModule(mod)
)
or
// module.exports = class Foo {
// bar() {} // <- result
// static baz() {} // <- result
// constructor() {} // <- result
// };
exists(DataFlow::ClassNode cla | cla = getAValueExportedByPackage() |
result = cla.getAnInstanceMethod() or
result = cla.getAStaticMethod() or
result = cla.getConstructor()
)
or
// One shot closures that define a "factory" function.
// Recognizes the following pattern:
// ```Javascript
// (function (root, factory) {
// if (typeof define === 'function' && define.amd) {
// define('library-name', factory);
// } else if (typeof exports === 'object') {
// module.exports = factory();
// } else {
// root.libraryName = factory();
// }
// }(this, function () {
// ....
// }));
// ```
// Such files are not recognized as modules, so we manually use `NodeModule::resolveMainModule` to resolve the file against a `package.json` file.
exists(ImmediatelyInvokedFunctionExpr func, DataFlow::ParameterNode prev, int i |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be related to the define.amd check? I think name-matching on "factory" is a bit too heuristic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've related it to the define(..) call.

I also missed a check that the file was actually exported 😬
That is added now.

prev.getName() = "factory" and
func.getParameter(i) = prev.getParameter() and
result = func.getInvocation().getArgument(i).flow().getAFunctionValue().getAReturn() and
DataFlow::globalVarRef("define").getACall().getArgument(1) = prev.getALocalUse() and
func.getFile() =
min(int j, File f |
f = NodeModule::resolveMainModule(any(PackageJSON pack | exists(pack.getPackageName())), j)
|
f order by j
)
)
or
// the exported value is a call to a unique callee
// ```JavaScript
// module.exports = foo();
// function foo() {
// return result;
// }
// ```
exists(DataFlow::CallNode call | call = getAValueExportedByPackage() |
result = unique( | | call.getCalleeNode().getAFunctionValue()).getAReturn()
)
or
// *****
// Common styles of transforming exported objects.
// *****
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
| lib/closure.js:4:6:4:7 | u* | Strings with many repetitions of 'u' can start matching anywhere after the start of the preceeding u*o |
| lib/lib.js:1:15:1:16 | a* | Strings with many repetitions of 'a' can start matching anywhere after the start of the preceeding a*b |
| lib/lib.js:8:3:8:4 | f* | Strings with many repetitions of 'f' can start matching anywhere after the start of the preceeding f*g |
| lib/sublib/factory.js:13:14:13:15 | f* | Strings with many repetitions of 'f' can start matching anywhere after the start of the preceeding f*g |
| polynomial-redos.js:7:24:7:26 | \\s+ | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding \\s+$ |
| polynomial-redos.js:8:17:8:18 | * | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding *, * |
| polynomial-redos.js:9:19:9:21 | \\s* | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding \\s*\\n\\s* |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ nodes
| lib/lib.js:7:19:7:22 | name |
| lib/lib.js:8:13:8:16 | name |
| lib/lib.js:8:13:8:16 | name |
| lib/sublib/factory.js:12:26:12:29 | name |
| lib/sublib/factory.js:12:26:12:29 | name |
| lib/sublib/factory.js:13:24:13:27 | name |
| lib/sublib/factory.js:13:24:13:27 | name |
| polynomial-redos.js:5:6:5:32 | tainted |
| polynomial-redos.js:5:16:5:32 | req.query.tainted |
| polynomial-redos.js:5:16:5:32 | req.query.tainted |
Expand Down Expand Up @@ -166,6 +170,10 @@ edges
| lib/lib.js:7:19:7:22 | name | lib/lib.js:8:13:8:16 | name |
| lib/lib.js:7:19:7:22 | name | lib/lib.js:8:13:8:16 | name |
| lib/lib.js:7:19:7:22 | name | lib/lib.js:8:13:8:16 | name |
| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
| polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:7:2:7:8 | tainted |
| polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:7:2:7:8 | tainted |
| polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:8:2:8:8 | tainted |
Expand Down Expand Up @@ -307,6 +315,7 @@ edges
| lib/closure.js:4:5:4:17 | /u*o/.test(x) | lib/closure.js:3:21:3:21 | x | lib/closure.js:4:16:4:16 | x | This $@ that depends on $@ may run slow on strings with many repetitions of 'u'. | lib/closure.js:4:6:4:7 | u* | regular expression | lib/closure.js:3:21:3:21 | x | library input |
| lib/lib.js:4:2:4:18 | regexp.test(name) | lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'a'. | lib/lib.js:1:15:1:16 | a* | regular expression | lib/lib.js:3:28:3:31 | name | library input |
| lib/lib.js:8:2:8:17 | /f*g/.test(name) | lib/lib.js:7:19:7:22 | name | lib/lib.js:8:13:8:16 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'f'. | lib/lib.js:8:3:8:4 | f* | regular expression | lib/lib.js:7:19:7:22 | name | library input |
| lib/sublib/factory.js:13:13:13:28 | /f*g/.test(name) | lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'f'. | lib/sublib/factory.js:13:14:13:15 | f* | regular expression | lib/sublib/factory.js:12:26:12:29 | name | library input |
| polynomial-redos.js:7:2:7:34 | tainted ... /g, '') | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:7:2:7:8 | tainted | This $@ that depends on $@ may run slow on strings with many repetitions of ' '. | polynomial-redos.js:7:24:7:26 | \\s+ | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
| polynomial-redos.js:8:2:8:23 | tainted ... *, */) | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:8:2:8:8 | tainted | This $@ that depends on $@ may run slow on strings with many repetitions of ' '. | polynomial-redos.js:8:17:8:18 | * | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
| polynomial-redos.js:9:2:9:34 | tainted ... g, ' ') | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:9:2:9:8 | tainted | This $@ that depends on $@ may run slow on strings with many repetitions of ' '. | polynomial-redos.js:9:19:9:21 | \\s* | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@

(function (root, factory) {
if (typeof define === 'function' && define.amd) {
define('my-sub-library', factory);
} else if (typeof exports === 'object') {
module.exports = factory();
} else {
root.mySubLibrary = factory();
}
}(this, function () {
function create() {
return function (name) {
/f*g/.test(name); // NOT OK
}
}
return create()
}));
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "my-sub-lib",
"version": "0.0.7",
"main": "./factory.js"
}