From 418f841749ddbad9173a534563eb978aeba92db5 Mon Sep 17 00:00:00 2001 From: Asger Feldthaus Date: Wed, 5 Feb 2020 12:04:58 +0000 Subject: [PATCH 1/6] JS: Handle imports through lazy-cache --- javascript/ql/src/javascript.qll | 1 + .../javascript/frameworks/LazyCache.qll | 55 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 javascript/ql/src/semmle/javascript/frameworks/LazyCache.qll diff --git a/javascript/ql/src/javascript.qll b/javascript/ql/src/javascript.qll index 00ec17dc70b..6f84584aa47 100644 --- a/javascript/ql/src/javascript.qll +++ b/javascript/ql/src/javascript.qll @@ -78,6 +78,7 @@ import semmle.javascript.frameworks.Files import semmle.javascript.frameworks.Firebase import semmle.javascript.frameworks.jQuery import semmle.javascript.frameworks.Handlebars +import semmle.javascript.frameworks.LazyCache import semmle.javascript.frameworks.LodashUnderscore import semmle.javascript.frameworks.Logging import semmle.javascript.frameworks.HttpFrameworks diff --git a/javascript/ql/src/semmle/javascript/frameworks/LazyCache.qll b/javascript/ql/src/semmle/javascript/frameworks/LazyCache.qll new file mode 100644 index 00000000000..b8c1c1e0c1f --- /dev/null +++ b/javascript/ql/src/semmle/javascript/frameworks/LazyCache.qll @@ -0,0 +1,55 @@ +/** + * Models imports through the NPM `lazy-cache` package. + */ + +import javascript + +module LazyCache { + /** + * A lazy-cache object, usually created through an expression of form `require('lazy-cache')(require)`. + */ + class LazyCacheObject extends DataFlow::SourceNode { + LazyCacheObject() { + // Use `require` directly instead of `moduleImport` to avoid recursion. + // For the same reason, avoid `Import.getImportedPath`. + exists(Require req | + req.getArgument(0).getStringValue() = "lazy-cache" and + this = req.flow().(DataFlow::SourceNode).getAnInvocation() + ) + } + } + + /** + * An import through `lazy-cache`. + */ + class LazyCacheImport extends CallExpr, Import { + LazyCacheObject cache; + + LazyCacheImport() { this = cache.getACall().asExpr() } + + /** Gets the name of the package as it's exposed on the lazy-cache object. */ + string getLocalAlias() { + result = getArgument(1).getStringValue() + or + not exists(getArgument(1)) and + result = getArgument(0).getStringValue() + } + + override Module getEnclosingModule() { result = getTopLevel() } + + override PathExpr getImportedPath() { result = getArgument(0) } + + override DataFlow::Node getImportedModuleNode() { + result = this.flow() + or + result = cache.getAPropertyRead(getLocalAlias()) + } + } + + /** A constant path element appearing in a call to a lazy-cache object. */ + private class LazyCachePathExpr extends PathExprInModule, ConstantString { + LazyCachePathExpr() { this = any(LazyCacheImport rp).getArgument(0) } + + override string getValue() { result = getStringValue() } + } +} From 34a9dce33d72e1bf452bfeaa10c2e7a6cd078f82 Mon Sep 17 00:00:00 2001 From: Asger Feldthaus Date: Wed, 5 Feb 2020 12:19:26 +0000 Subject: [PATCH 2/6] JS: Detect property enumeration through for-own --- .../CWE-400/PrototypePollutionUtility.ql | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql b/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql index 978fb78b11e..dbe0511ecf2 100644 --- a/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql +++ b/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql @@ -120,6 +120,30 @@ class EntriesEnumeratedPropName extends EnumeratedPropName { } } +/** + * Property enumeration through the `for-own` package. + */ +class ForOwnEnumeratedPropName extends EnumeratedPropName { + CallNode call; + FunctionNode callback; + + ForOwnEnumeratedPropName() { + call = moduleImport("for-own").getACall() and + callback = call.getCallback(1) and + this = callback.getParameter(1) + } + + override Node getSourceObject() { + result = call.getArgument(0) + } + + override SourceNode getASourceProp() { + result = super.getASourceProp() + or + result = callback.getParameter(0) + } +} + /** * Holds if the properties of `node` are enumerated locally. */ From c559ab13e770b380d0ba0db998e91ac1450b553e Mon Sep 17 00:00:00 2001 From: Asger Feldthaus Date: Wed, 5 Feb 2020 12:32:13 +0000 Subject: [PATCH 3/6] JS: Add test and handle parameter with source object --- .../CWE-400/PrototypePollutionUtility.ql | 8 +- .../PrototypePollutionUtility.expected | 161 ++++++++++++++++++ .../PrototypePollutionUtility/tests.js | 14 ++ 3 files changed, 182 insertions(+), 1 deletion(-) diff --git a/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql b/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql index dbe0511ecf2..b6d81e5054c 100644 --- a/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql +++ b/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql @@ -42,7 +42,7 @@ SourceNode getAnEnumeratedArrayElement(SourceNode array) { */ abstract class EnumeratedPropName extends DataFlow::Node { /** - * Gets the object whose properties are being enumerated. + * Gets the data flow node holding the object whose properties are being enumerated. * * For example, gets `src` in `for (var key in src)`. */ @@ -137,6 +137,12 @@ class ForOwnEnumeratedPropName extends EnumeratedPropName { result = call.getArgument(0) } + override SourceNode getASourceObjectRef() { + result = super.getASourceObjectRef() + or + result = callback.getParameter(2) + } + override SourceNode getASourceProp() { result = super.getASourceProp() or diff --git a/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility.expected b/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility.expected index 9d80066ebfb..808f35e087f 100644 --- a/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility.expected +++ b/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility.expected @@ -953,6 +953,70 @@ nodes | PrototypePollutionUtility/tests.js:437:24:437:28 | value | | PrototypePollutionUtility/tests.js:437:24:437:28 | value | | PrototypePollutionUtility/tests.js:437:24:437:28 | value | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | +| PrototypePollutionUtility/tests.js:442:31:442:33 | src | +| PrototypePollutionUtility/tests.js:442:31:442:33 | src | +| PrototypePollutionUtility/tests.js:444:18:444:22 | value | +| PrototypePollutionUtility/tests.js:444:18:444:22 | value | +| PrototypePollutionUtility/tests.js:444:18:444:22 | value | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | +| PrototypePollutionUtility/tests.js:446:29:446:31 | dst | +| PrototypePollutionUtility/tests.js:446:29:446:31 | dst | +| PrototypePollutionUtility/tests.js:446:29:446:36 | dst[key] | +| PrototypePollutionUtility/tests.js:446:29:446:36 | dst[key] | +| PrototypePollutionUtility/tests.js:446:29:446:36 | dst[key] | +| PrototypePollutionUtility/tests.js:446:29:446:36 | dst[key] | +| PrototypePollutionUtility/tests.js:446:33:446:35 | key | +| PrototypePollutionUtility/tests.js:446:33:446:35 | key | +| PrototypePollutionUtility/tests.js:446:39:446:41 | src | +| PrototypePollutionUtility/tests.js:446:39:446:41 | src | +| PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | +| PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | +| PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | +| PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | +| PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | +| PrototypePollutionUtility/tests.js:446:43:446:45 | key | +| PrototypePollutionUtility/tests.js:446:43:446:45 | key | +| PrototypePollutionUtility/tests.js:449:30:449:32 | dst | +| PrototypePollutionUtility/tests.js:449:30:449:32 | dst | +| PrototypePollutionUtility/tests.js:449:30:449:32 | dst | +| PrototypePollutionUtility/tests.js:449:34:449:36 | key | +| PrototypePollutionUtility/tests.js:449:34:449:36 | key | +| PrototypePollutionUtility/tests.js:449:34:449:36 | key | +| PrototypePollutionUtility/tests.js:449:41:449:43 | src | +| PrototypePollutionUtility/tests.js:449:41:449:43 | src | +| PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:45:449:47 | key | +| PrototypePollutionUtility/tests.js:449:45:449:47 | key | +| PrototypePollutionUtility/tests.js:450:30:450:32 | dst | +| PrototypePollutionUtility/tests.js:450:30:450:32 | dst | +| PrototypePollutionUtility/tests.js:450:30:450:32 | dst | +| PrototypePollutionUtility/tests.js:450:34:450:36 | key | +| PrototypePollutionUtility/tests.js:450:34:450:36 | key | +| PrototypePollutionUtility/tests.js:450:34:450:36 | key | +| PrototypePollutionUtility/tests.js:450:41:450:46 | o[key] | +| PrototypePollutionUtility/tests.js:450:41:450:46 | o[key] | +| PrototypePollutionUtility/tests.js:450:41:450:46 | o[key] | +| PrototypePollutionUtility/tests.js:450:41:450:46 | o[key] | +| PrototypePollutionUtility/tests.js:450:43:450:45 | key | +| PrototypePollutionUtility/tests.js:450:43:450:45 | key | +| PrototypePollutionUtility/tests.js:451:30:451:32 | dst | +| PrototypePollutionUtility/tests.js:451:30:451:32 | dst | +| PrototypePollutionUtility/tests.js:451:30:451:32 | dst | +| PrototypePollutionUtility/tests.js:451:34:451:36 | key | +| PrototypePollutionUtility/tests.js:451:34:451:36 | key | +| PrototypePollutionUtility/tests.js:451:34:451:36 | key | +| PrototypePollutionUtility/tests.js:451:41:451:45 | value | +| PrototypePollutionUtility/tests.js:451:41:451:45 | value | +| PrototypePollutionUtility/tests.js:451:41:451:45 | value | | examples/PrototypePollutionUtility.js:1:16:1:18 | dst | | examples/PrototypePollutionUtility.js:1:16:1:18 | dst | | examples/PrototypePollutionUtility.js:1:21:1:23 | src | @@ -2242,6 +2306,100 @@ edges | PrototypePollutionUtility/tests.js:435:39:435:43 | value | PrototypePollutionUtility/tests.js:430:33:430:35 | src | | PrototypePollutionUtility/tests.js:435:39:435:43 | value | PrototypePollutionUtility/tests.js:430:33:430:35 | src | | PrototypePollutionUtility/tests.js:435:39:435:43 | value | PrototypePollutionUtility/tests.js:430:33:430:35 | src | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:446:29:446:31 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:446:29:446:31 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:449:30:449:32 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:449:30:449:32 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:449:30:449:32 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:449:30:449:32 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:450:30:450:32 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:450:30:450:32 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:450:30:450:32 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:450:30:450:32 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:451:30:451:32 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:451:30:451:32 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:451:30:451:32 | dst | +| PrototypePollutionUtility/tests.js:442:26:442:28 | dst | PrototypePollutionUtility/tests.js:451:30:451:32 | dst | +| PrototypePollutionUtility/tests.js:442:31:442:33 | src | PrototypePollutionUtility/tests.js:446:39:446:41 | src | +| PrototypePollutionUtility/tests.js:442:31:442:33 | src | PrototypePollutionUtility/tests.js:446:39:446:41 | src | +| PrototypePollutionUtility/tests.js:442:31:442:33 | src | PrototypePollutionUtility/tests.js:449:41:449:43 | src | +| PrototypePollutionUtility/tests.js:442:31:442:33 | src | PrototypePollutionUtility/tests.js:449:41:449:43 | src | +| PrototypePollutionUtility/tests.js:444:18:444:22 | value | PrototypePollutionUtility/tests.js:451:41:451:45 | value | +| PrototypePollutionUtility/tests.js:444:18:444:22 | value | PrototypePollutionUtility/tests.js:451:41:451:45 | value | +| PrototypePollutionUtility/tests.js:444:18:444:22 | value | PrototypePollutionUtility/tests.js:451:41:451:45 | value | +| PrototypePollutionUtility/tests.js:444:18:444:22 | value | PrototypePollutionUtility/tests.js:451:41:451:45 | value | +| PrototypePollutionUtility/tests.js:444:18:444:22 | value | PrototypePollutionUtility/tests.js:451:41:451:45 | value | +| PrototypePollutionUtility/tests.js:444:18:444:22 | value | PrototypePollutionUtility/tests.js:451:41:451:45 | value | +| PrototypePollutionUtility/tests.js:444:18:444:22 | value | PrototypePollutionUtility/tests.js:451:41:451:45 | value | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:446:33:446:35 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:446:33:446:35 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:446:33:446:35 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:446:33:446:35 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:446:43:446:45 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:446:43:446:45 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:446:43:446:45 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:446:43:446:45 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:449:34:449:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:449:34:449:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:449:34:449:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:449:34:449:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:449:34:449:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:449:34:449:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:449:34:449:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:449:45:449:47 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:449:45:449:47 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:449:45:449:47 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:449:45:449:47 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:450:34:450:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:450:34:450:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:450:34:450:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:450:34:450:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:450:34:450:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:450:34:450:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:450:34:450:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:450:43:450:45 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:450:43:450:45 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:450:43:450:45 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:450:43:450:45 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:451:34:451:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:451:34:451:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:451:34:451:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:451:34:451:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:451:34:451:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:451:34:451:36 | key | +| PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:451:34:451:36 | key | +| PrototypePollutionUtility/tests.js:446:29:446:31 | dst | PrototypePollutionUtility/tests.js:446:29:446:36 | dst[key] | +| PrototypePollutionUtility/tests.js:446:29:446:31 | dst | PrototypePollutionUtility/tests.js:446:29:446:36 | dst[key] | +| PrototypePollutionUtility/tests.js:446:29:446:36 | dst[key] | PrototypePollutionUtility/tests.js:442:26:442:28 | dst | +| PrototypePollutionUtility/tests.js:446:29:446:36 | dst[key] | PrototypePollutionUtility/tests.js:442:26:442:28 | dst | +| PrototypePollutionUtility/tests.js:446:29:446:36 | dst[key] | PrototypePollutionUtility/tests.js:442:26:442:28 | dst | +| PrototypePollutionUtility/tests.js:446:29:446:36 | dst[key] | PrototypePollutionUtility/tests.js:442:26:442:28 | dst | +| PrototypePollutionUtility/tests.js:446:33:446:35 | key | PrototypePollutionUtility/tests.js:446:29:446:36 | dst[key] | +| PrototypePollutionUtility/tests.js:446:33:446:35 | key | PrototypePollutionUtility/tests.js:446:29:446:36 | dst[key] | +| PrototypePollutionUtility/tests.js:446:39:446:41 | src | PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | +| PrototypePollutionUtility/tests.js:446:39:446:41 | src | PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | +| PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | PrototypePollutionUtility/tests.js:442:31:442:33 | src | +| PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | PrototypePollutionUtility/tests.js:442:31:442:33 | src | +| PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | PrototypePollutionUtility/tests.js:442:31:442:33 | src | +| PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | PrototypePollutionUtility/tests.js:442:31:442:33 | src | +| PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | PrototypePollutionUtility/tests.js:442:31:442:33 | src | +| PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | PrototypePollutionUtility/tests.js:442:31:442:33 | src | +| PrototypePollutionUtility/tests.js:446:43:446:45 | key | PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | +| PrototypePollutionUtility/tests.js:446:43:446:45 | key | PrototypePollutionUtility/tests.js:446:39:446:46 | src[key] | +| PrototypePollutionUtility/tests.js:449:41:449:43 | src | PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:41:449:43 | src | PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:41:449:43 | src | PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:41:449:43 | src | PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:45:449:47 | key | PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:45:449:47 | key | PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:45:449:47 | key | PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:449:45:449:47 | key | PrototypePollutionUtility/tests.js:449:41:449:48 | src[key] | +| PrototypePollutionUtility/tests.js:450:41:450:46 | o[key] | PrototypePollutionUtility/tests.js:450:41:450:46 | o[key] | +| PrototypePollutionUtility/tests.js:450:43:450:45 | key | PrototypePollutionUtility/tests.js:450:41:450:46 | o[key] | +| PrototypePollutionUtility/tests.js:450:43:450:45 | key | PrototypePollutionUtility/tests.js:450:41:450:46 | o[key] | +| PrototypePollutionUtility/tests.js:450:43:450:45 | key | PrototypePollutionUtility/tests.js:450:41:450:46 | o[key] | +| PrototypePollutionUtility/tests.js:450:43:450:45 | key | PrototypePollutionUtility/tests.js:450:41:450:46 | o[key] | | examples/PrototypePollutionUtility.js:1:16:1:18 | dst | examples/PrototypePollutionUtility.js:5:19:5:21 | dst | | examples/PrototypePollutionUtility.js:1:16:1:18 | dst | examples/PrototypePollutionUtility.js:5:19:5:21 | dst | | examples/PrototypePollutionUtility.js:1:16:1:18 | dst | examples/PrototypePollutionUtility.js:7:13:7:15 | dst | @@ -2364,4 +2522,7 @@ edges | PrototypePollutionUtility/tests.js:387:13:387:15 | dst | PrototypePollutionUtility/tests.js:365:14:365:16 | key | PrototypePollutionUtility/tests.js:387:13:387:15 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | PrototypePollutionUtility/tests.js:365:21:365:23 | obj | obj | PrototypePollutionUtility/tests.js:387:13:387:15 | dst | dst | | PrototypePollutionUtility/tests.js:403:13:403:15 | dst | PrototypePollutionUtility/tests.js:397:14:397:16 | key | PrototypePollutionUtility/tests.js:403:13:403:15 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | PrototypePollutionUtility/tests.js:397:21:397:23 | src | src | PrototypePollutionUtility/tests.js:403:13:403:15 | dst | dst | | PrototypePollutionUtility/tests.js:420:13:420:15 | dst | PrototypePollutionUtility/tests.js:414:14:414:16 | key | PrototypePollutionUtility/tests.js:420:13:420:15 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | PrototypePollutionUtility/tests.js:414:21:414:23 | src | src | PrototypePollutionUtility/tests.js:420:13:420:15 | dst | dst | +| PrototypePollutionUtility/tests.js:449:30:449:32 | dst | PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:449:30:449:32 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | PrototypePollutionUtility/tests.js:444:12:444:14 | src | src | PrototypePollutionUtility/tests.js:449:30:449:32 | dst | dst | +| PrototypePollutionUtility/tests.js:450:30:450:32 | dst | PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:450:30:450:32 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | PrototypePollutionUtility/tests.js:444:12:444:14 | src | src | PrototypePollutionUtility/tests.js:450:30:450:32 | dst | dst | +| PrototypePollutionUtility/tests.js:451:30:451:32 | dst | PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:451:30:451:32 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | PrototypePollutionUtility/tests.js:444:12:444:14 | src | src | PrototypePollutionUtility/tests.js:451:30:451:32 | dst | dst | | examples/PrototypePollutionUtility.js:7:13:7:15 | dst | examples/PrototypePollutionUtility.js:2:14:2:16 | key | examples/PrototypePollutionUtility.js:7:13:7:15 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | examples/PrototypePollutionUtility.js:2:21:2:23 | src | src | examples/PrototypePollutionUtility.js:7:13:7:15 | dst | dst | diff --git a/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility/tests.js b/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility/tests.js index 0c59890bd9c..29f28b57905 100644 --- a/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility/tests.js +++ b/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility/tests.js @@ -438,3 +438,17 @@ function copyUsingSafeRead(dst, src) { } } } + +function copyUsingForOwn(dst, src) { + let forOwn = import('for-own'); + forOwn(src, (value, key, o) => { + if (dst[key]) { + copyUsingForOwn(dst[key], src[key]); + } else { + // Handle a few different ways to access src[key] + if (something()) dst[key] = src[key]; // NOT OK + if (something()) dst[key] = o[key]; // NOT OK + if (something()) dst[key] = value; // NOT OK + } + }); +} From f84af74d1d5af208e8eb05db98f286de5309aa0e Mon Sep 17 00:00:00 2001 From: Asger Feldthaus Date: Wed, 5 Feb 2020 12:34:51 +0000 Subject: [PATCH 4/6] JS: Handle more libraries --- .../CWE-400/PrototypePollutionUtility.ql | 24 ++++++-- .../PrototypePollutionUtility.expected | 58 +++++++++++++++++++ .../PrototypePollutionUtility/tests.js | 10 ++++ 3 files changed, 88 insertions(+), 4 deletions(-) diff --git a/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql b/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql index b6d81e5054c..515c5d5752e 100644 --- a/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql +++ b/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql @@ -121,14 +121,30 @@ class EntriesEnumeratedPropName extends EnumeratedPropName { } /** - * Property enumeration through the `for-own` package. + * Gets a function that enumerates object properties when invoked. + * + * Invocations takes the following form: + * ```js + * fn(obj, (value, key, o) => { ... }) + * ``` */ -class ForOwnEnumeratedPropName extends EnumeratedPropName { +SourceNode propertyEnumerator() { + result = moduleImport("for-own") or + result = moduleImport("for-in") or + result = moduleMember("ramda", "forEachObjIndexed") or + result = LodashUnderscore::member("forEach") or + result = LodashUnderscore::member("each") +} + +/** + * Property enumeration through the `for-own` or `for-in` package. + */ +class LibraryCallbackEnumeratedPropName extends EnumeratedPropName { CallNode call; FunctionNode callback; - ForOwnEnumeratedPropName() { - call = moduleImport("for-own").getACall() and + LibraryCallbackEnumeratedPropName() { + call = propertyEnumerator().getACall() and callback = call.getCallback(1) and this = callback.getParameter(1) } diff --git a/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility.expected b/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility.expected index 808f35e087f..88fd5914d69 100644 --- a/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility.expected +++ b/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility.expected @@ -1017,6 +1017,31 @@ nodes | PrototypePollutionUtility/tests.js:451:41:451:45 | value | | PrototypePollutionUtility/tests.js:451:41:451:45 | value | | PrototypePollutionUtility/tests.js:451:41:451:45 | value | +| PrototypePollutionUtility/tests.js:456:38:456:40 | dst | +| PrototypePollutionUtility/tests.js:456:38:456:40 | dst | +| PrototypePollutionUtility/tests.js:457:18:457:22 | value | +| PrototypePollutionUtility/tests.js:457:18:457:22 | value | +| PrototypePollutionUtility/tests.js:457:18:457:22 | value | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | +| PrototypePollutionUtility/tests.js:459:41:459:43 | dst | +| PrototypePollutionUtility/tests.js:459:41:459:43 | dst | +| PrototypePollutionUtility/tests.js:459:41:459:48 | dst[key] | +| PrototypePollutionUtility/tests.js:459:41:459:48 | dst[key] | +| PrototypePollutionUtility/tests.js:459:41:459:48 | dst[key] | +| PrototypePollutionUtility/tests.js:459:41:459:48 | dst[key] | +| PrototypePollutionUtility/tests.js:459:45:459:47 | key | +| PrototypePollutionUtility/tests.js:459:45:459:47 | key | +| PrototypePollutionUtility/tests.js:461:13:461:15 | dst | +| PrototypePollutionUtility/tests.js:461:13:461:15 | dst | +| PrototypePollutionUtility/tests.js:461:13:461:15 | dst | +| PrototypePollutionUtility/tests.js:461:17:461:19 | key | +| PrototypePollutionUtility/tests.js:461:17:461:19 | key | +| PrototypePollutionUtility/tests.js:461:17:461:19 | key | +| PrototypePollutionUtility/tests.js:461:24:461:28 | value | +| PrototypePollutionUtility/tests.js:461:24:461:28 | value | +| PrototypePollutionUtility/tests.js:461:24:461:28 | value | | examples/PrototypePollutionUtility.js:1:16:1:18 | dst | | examples/PrototypePollutionUtility.js:1:16:1:18 | dst | | examples/PrototypePollutionUtility.js:1:21:1:23 | src | @@ -2400,6 +2425,38 @@ edges | PrototypePollutionUtility/tests.js:450:43:450:45 | key | PrototypePollutionUtility/tests.js:450:41:450:46 | o[key] | | PrototypePollutionUtility/tests.js:450:43:450:45 | key | PrototypePollutionUtility/tests.js:450:41:450:46 | o[key] | | PrototypePollutionUtility/tests.js:450:43:450:45 | key | PrototypePollutionUtility/tests.js:450:41:450:46 | o[key] | +| PrototypePollutionUtility/tests.js:456:38:456:40 | dst | PrototypePollutionUtility/tests.js:459:41:459:43 | dst | +| PrototypePollutionUtility/tests.js:456:38:456:40 | dst | PrototypePollutionUtility/tests.js:459:41:459:43 | dst | +| PrototypePollutionUtility/tests.js:456:38:456:40 | dst | PrototypePollutionUtility/tests.js:461:13:461:15 | dst | +| PrototypePollutionUtility/tests.js:456:38:456:40 | dst | PrototypePollutionUtility/tests.js:461:13:461:15 | dst | +| PrototypePollutionUtility/tests.js:456:38:456:40 | dst | PrototypePollutionUtility/tests.js:461:13:461:15 | dst | +| PrototypePollutionUtility/tests.js:456:38:456:40 | dst | PrototypePollutionUtility/tests.js:461:13:461:15 | dst | +| PrototypePollutionUtility/tests.js:457:18:457:22 | value | PrototypePollutionUtility/tests.js:461:24:461:28 | value | +| PrototypePollutionUtility/tests.js:457:18:457:22 | value | PrototypePollutionUtility/tests.js:461:24:461:28 | value | +| PrototypePollutionUtility/tests.js:457:18:457:22 | value | PrototypePollutionUtility/tests.js:461:24:461:28 | value | +| PrototypePollutionUtility/tests.js:457:18:457:22 | value | PrototypePollutionUtility/tests.js:461:24:461:28 | value | +| PrototypePollutionUtility/tests.js:457:18:457:22 | value | PrototypePollutionUtility/tests.js:461:24:461:28 | value | +| PrototypePollutionUtility/tests.js:457:18:457:22 | value | PrototypePollutionUtility/tests.js:461:24:461:28 | value | +| PrototypePollutionUtility/tests.js:457:18:457:22 | value | PrototypePollutionUtility/tests.js:461:24:461:28 | value | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | PrototypePollutionUtility/tests.js:459:45:459:47 | key | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | PrototypePollutionUtility/tests.js:459:45:459:47 | key | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | PrototypePollutionUtility/tests.js:459:45:459:47 | key | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | PrototypePollutionUtility/tests.js:459:45:459:47 | key | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | PrototypePollutionUtility/tests.js:461:17:461:19 | key | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | PrototypePollutionUtility/tests.js:461:17:461:19 | key | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | PrototypePollutionUtility/tests.js:461:17:461:19 | key | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | PrototypePollutionUtility/tests.js:461:17:461:19 | key | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | PrototypePollutionUtility/tests.js:461:17:461:19 | key | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | PrototypePollutionUtility/tests.js:461:17:461:19 | key | +| PrototypePollutionUtility/tests.js:457:25:457:27 | key | PrototypePollutionUtility/tests.js:461:17:461:19 | key | +| PrototypePollutionUtility/tests.js:459:41:459:43 | dst | PrototypePollutionUtility/tests.js:459:41:459:48 | dst[key] | +| PrototypePollutionUtility/tests.js:459:41:459:43 | dst | PrototypePollutionUtility/tests.js:459:41:459:48 | dst[key] | +| PrototypePollutionUtility/tests.js:459:41:459:48 | dst[key] | PrototypePollutionUtility/tests.js:456:38:456:40 | dst | +| PrototypePollutionUtility/tests.js:459:41:459:48 | dst[key] | PrototypePollutionUtility/tests.js:456:38:456:40 | dst | +| PrototypePollutionUtility/tests.js:459:41:459:48 | dst[key] | PrototypePollutionUtility/tests.js:456:38:456:40 | dst | +| PrototypePollutionUtility/tests.js:459:41:459:48 | dst[key] | PrototypePollutionUtility/tests.js:456:38:456:40 | dst | +| PrototypePollutionUtility/tests.js:459:45:459:47 | key | PrototypePollutionUtility/tests.js:459:41:459:48 | dst[key] | +| PrototypePollutionUtility/tests.js:459:45:459:47 | key | PrototypePollutionUtility/tests.js:459:41:459:48 | dst[key] | | examples/PrototypePollutionUtility.js:1:16:1:18 | dst | examples/PrototypePollutionUtility.js:5:19:5:21 | dst | | examples/PrototypePollutionUtility.js:1:16:1:18 | dst | examples/PrototypePollutionUtility.js:5:19:5:21 | dst | | examples/PrototypePollutionUtility.js:1:16:1:18 | dst | examples/PrototypePollutionUtility.js:7:13:7:15 | dst | @@ -2525,4 +2582,5 @@ edges | PrototypePollutionUtility/tests.js:449:30:449:32 | dst | PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:449:30:449:32 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | PrototypePollutionUtility/tests.js:444:12:444:14 | src | src | PrototypePollutionUtility/tests.js:449:30:449:32 | dst | dst | | PrototypePollutionUtility/tests.js:450:30:450:32 | dst | PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:450:30:450:32 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | PrototypePollutionUtility/tests.js:444:12:444:14 | src | src | PrototypePollutionUtility/tests.js:450:30:450:32 | dst | dst | | PrototypePollutionUtility/tests.js:451:30:451:32 | dst | PrototypePollutionUtility/tests.js:444:25:444:27 | key | PrototypePollutionUtility/tests.js:451:30:451:32 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | PrototypePollutionUtility/tests.js:444:12:444:14 | src | src | PrototypePollutionUtility/tests.js:451:30:451:32 | dst | dst | +| PrototypePollutionUtility/tests.js:461:13:461:15 | dst | PrototypePollutionUtility/tests.js:457:25:457:27 | key | PrototypePollutionUtility/tests.js:461:13:461:15 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | PrototypePollutionUtility/tests.js:457:12:457:14 | src | src | PrototypePollutionUtility/tests.js:461:13:461:15 | dst | dst | | examples/PrototypePollutionUtility.js:7:13:7:15 | dst | examples/PrototypePollutionUtility.js:2:14:2:16 | key | examples/PrototypePollutionUtility.js:7:13:7:15 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | examples/PrototypePollutionUtility.js:2:21:2:23 | src | src | examples/PrototypePollutionUtility.js:7:13:7:15 | dst | dst | diff --git a/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility/tests.js b/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility/tests.js index 29f28b57905..2de68103c63 100644 --- a/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility/tests.js +++ b/javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility/tests.js @@ -452,3 +452,13 @@ function copyUsingForOwn(dst, src) { } }); } + +function copyUsingUnderscoreOrLodash(dst, src) { + _.each(src, (value, key, o) => { + if (dst[key]) { + copyUsingUnderscoreOrLodash(dst[key], src[key]); + } else { + dst[key] = value; // NOT OK + } + }); +} From a628f787e8370e3eeaeb6f8f4ba158926d654b53 Mon Sep 17 00:00:00 2001 From: Asger Feldthaus Date: Wed, 5 Feb 2020 14:22:01 +0000 Subject: [PATCH 5/6] JS: Fix qldoc comment --- javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql b/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql index 515c5d5752e..2fd16055484 100644 --- a/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql +++ b/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql @@ -137,7 +137,7 @@ SourceNode propertyEnumerator() { } /** - * Property enumeration through the `for-own` or `for-in` package. + * Property enumeration through a library function taking a callback. */ class LibraryCallbackEnumeratedPropName extends EnumeratedPropName { CallNode call; From 91a5385e7fa7620737b561a04c5b5ef540c7841d Mon Sep 17 00:00:00 2001 From: Asger Feldthaus Date: Wed, 5 Feb 2020 14:37:00 +0000 Subject: [PATCH 6/6] JS: Add libraries to change note --- change-notes/1.24/analysis-javascript.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/change-notes/1.24/analysis-javascript.md b/change-notes/1.24/analysis-javascript.md index 97298e0738f..003a19d1182 100644 --- a/change-notes/1.24/analysis-javascript.md +++ b/change-notes/1.24/analysis-javascript.md @@ -21,6 +21,9 @@ - [ws](https://github.com/websockets/ws) - [WebSocket](https://developer.mozilla.org/en-US/docs/Web/API/WebSockets_API) - [Koa](https://www.npmjs.com/package/koa) + - [lazy-cache](https://www.npmjs.com/package/lazy-cache) + - [for-in](https://www.npmjs.com/package/for-in) + - [for-own](https://www.npmjs.com/package/for-own) ## New queries