Skip to content

JS: recognize library inputs when the library exports "through" a function #7137

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
erik-krogh merged 1 commit into from
Nov 17, 2021
Merged

JS: recognize library inputs when the library exports "through" a function #7137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 13 additions & 0 deletions javascript/ql/lib/semmle/javascript/PackageExports.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,19 @@ private DataFlow::Node getAValueExportedByPackage() {
result = unique( | | call.getCalleeNode().getAFunctionValue()).getAReturn()
)
or
// the exported value is a function that returns another import.
Copy link
Copy Markdown
Contributor

@esbena esbena Nov 16, 2021

Choose a reason for hiding this comment

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

Do you know if this pattern is common in this syntactic form? If so, it would probably be worth it to support this kind of reexport directly in our import graph as well.

Copy link
Copy Markdown
Contributor Author

@erik-krogh erik-krogh Nov 17, 2021

Choose a reason for hiding this comment

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

I'm not sure if we can put this kind of "reexport" directly into the import graph.

At the import site it requires a user to write something like:

var lib = require("lib")(conf);

So it's only "reexported" when the client calls the function.

// ```JavaScript
// module.exports = function foo() {
// return require("./other-module.js");
// }
// ```
exists(DataFlow::FunctionNode func, Module mod |
func = getAValueExportedByPackage().getABoundFunctionValue(_)
|
mod = func.getAReturn().getALocalSource().getEnclosingExpr().(Import).getImportedModule() and
result = getAnExportFromModule(mod)
)
or
// *****
// Common styles of transforming exported objects.
// *****
Expand Down
1 change: 1 addition & 0 deletions javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
| jsonschema.js:15:23:15:29 | (a?a?)* | Strings with many repetitions of 'a' can start matching anywhere after the start of the preceeding (a?a?)*b |
| jsonschema.js:20:18:20:24 | (a?a?)* | Strings with many repetitions of 'a' can start matching anywhere after the start of the preceeding (a?a?)*b |
| 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/indirect.js:2:6:2:7 | k* | Strings with many repetitions of 'k' can start matching anywhere after the start of the preceeding k*h |
| 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/moduleLib/moduleLib.js:2:3:2:4 | a* | Strings with many repetitions of 'a' can start matching anywhere after the start of the preceeding a*b |
Expand Down
9 changes: 9 additions & 0 deletions javascript/ql/test/query-tests/Performance/ReDoS/PolynomialReDoS.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ nodes
| lib/closure.js:3:21:3:21 | x |
| lib/closure.js:4:16:4:16 | x |
| lib/closure.js:4:16:4:16 | x |
| lib/indirect.js:1:32:1:32 | x |
| lib/indirect.js:1:32:1:32 | x |
| lib/indirect.js:2:16:2:16 | x |
| lib/indirect.js:2:16:2:16 | x |
| lib/lib.js:3:28:3:31 | name |
| lib/lib.js:3:28:3:31 | name |
| lib/lib.js:4:14:4:17 | name |
Expand Down Expand Up @@ -170,6 +174,10 @@ edges
| lib/closure.js:3:21:3:21 | x | lib/closure.js:4:16:4:16 | x |
| lib/closure.js:3:21:3:21 | x | lib/closure.js:4:16:4:16 | x |
| lib/closure.js:3:21:3:21 | x | lib/closure.js:4:16:4:16 | x |
| lib/indirect.js:1:32:1:32 | x | lib/indirect.js:2:16:2:16 | x |
| lib/indirect.js:1:32:1:32 | x | lib/indirect.js:2:16:2:16 | x |
| lib/indirect.js:1:32:1:32 | x | lib/indirect.js:2:16:2:16 | x |
| lib/indirect.js:1:32:1:32 | x | lib/indirect.js:2:16:2:16 | x |
| lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name |
| lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name |
| lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name |
Expand Down Expand Up @@ -329,6 +337,7 @@ edges
| polynomial-redos.js:123:13:123:20 | replaced | polynomial-redos.js:123:3:123:20 | result |
#select
| 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/indirect.js:2:5:2:17 | /k*h/.test(x) | lib/indirect.js:1:32:1:32 | x | lib/indirect.js:2:16:2:16 | x | This $@ that depends on $@ may run slow on strings with many repetitions of 'k'. | lib/indirect.js:2:6:2:7 | k* | regular expression | lib/indirect.js:1:32:1:32 | 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/moduleLib/moduleLib.js:2:2:2:17 | /a*b/.test(name) | lib/moduleLib/moduleLib.js:1:28:1:31 | name | lib/moduleLib/moduleLib.js:2:13:2:16 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'a'. | lib/moduleLib/moduleLib.js:2:3:2:4 | a* | regular expression | lib/moduleLib/moduleLib.js:1:28:1:31 | name | library input |
Expand Down
3 changes: 3 additions & 0 deletions javascript/ql/test/query-tests/Performance/ReDoS/lib/indirect.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports.foo = function (x) {
/k*h/.test(x); // NOT OK
}
6 changes: 5 additions & 1 deletion javascript/ql/test/query-tests/Performance/ReDoS/lib/lib.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,8 @@ if (typeof define !== 'undefined' && define.amd) { // AMD
define([], function () {return bar});
}

module.exports.closure = require("./closure")
module.exports.closure = require("./closure")

module.exports.func = function (conf) {
return require("./indirect")
}