diff --git a/change-notes/1.18/analysis-javascript.md b/change-notes/1.18/analysis-javascript.md index 0ce25027a6e..9b52a90266e 100644 --- a/change-notes/1.18/analysis-javascript.md +++ b/change-notes/1.18/analysis-javascript.md @@ -31,6 +31,8 @@ - [deepmerge](https://github.com/KyleAMathews/deepmerge) - [defaults-deep](https://github.com/jonschlinkert/defaults-deep) - [defaults](https://github.com/tmpvar/defaults) + - [dottie](https://github.com/mickhansen/dottie.js) + - [dotty](https://github.com/deoxxa/dotty) - [ent](https://github.com/substack/node-ent) - [entities](https://github.com/fb55/node-entities) - [escape-goat](https://github.com/sindresorhus/escape-goat) diff --git a/javascript/ql/src/javascript.qll b/javascript/ql/src/javascript.qll index 5438624fc5f..4f2158638ce 100644 --- a/javascript/ql/src/javascript.qll +++ b/javascript/ql/src/javascript.qll @@ -63,6 +63,7 @@ import semmle.javascript.frameworks.Logging import semmle.javascript.frameworks.HttpFrameworks import semmle.javascript.frameworks.NoSQL import semmle.javascript.frameworks.PkgCloud +import semmle.javascript.frameworks.PropertyProjection import semmle.javascript.frameworks.React import semmle.javascript.frameworks.ReactNative import semmle.javascript.frameworks.Request diff --git a/javascript/ql/src/semmle/javascript/frameworks/PropertyProjection.qll b/javascript/ql/src/semmle/javascript/frameworks/PropertyProjection.qll new file mode 100644 index 00000000000..ce2e5796e50 --- /dev/null +++ b/javascript/ql/src/semmle/javascript/frameworks/PropertyProjection.qll @@ -0,0 +1,155 @@ +/** + * Provides classes for modelling property projection functions. + * + * Subclass `PropertyProjection` to refine the behavior of the analysis on existing property projections. + * Subclass `CustomPropertyProjection` to introduce new kinds of property projections. + */ + +import javascript + +/** + * A property projection call such as `_.get(o, 'a.b')`, which is equivalent to `o.a.b`. + */ +abstract class CustomPropertyProjection extends DataFlow::CallNode { + + /** + * Gets the argument for the object to project properties from, such as `o` in `_.get(o, 'a.b')`. + */ + abstract DataFlow::Node getObject(); + + /** + * Gets an argument that selects the properties to project, such as `'a.b'` in `_.get(o, 'a.b')`. + */ + abstract DataFlow::Node getASelector(); + + /** + * Holds if this call returns the value of a single projected property, as opposed to an object that can contain multiple projected properties. + */ + abstract predicate isSingletonProjection(); + +} + +/** + * A property projection call such as `_.get(o, 'a.b')`, which is equivalent to `o.a.b`. + */ +class PropertyProjection extends DataFlow::CallNode { + + CustomPropertyProjection custom; + + PropertyProjection() { this = custom } + + /** + * Gets the argument for the object to project properties from, such as `o` in `_.get(o, 'a.b')`. + */ + DataFlow::Node getObject() { result = custom.getObject() } + + /** + * Gets an argument that selects the properties to project, such as `'a.b'` in `_.get(o, 'a.b')`. + */ + DataFlow::Node getASelector() { result = custom.getASelector() } + + /** + * Holds if this call returns the value of a single projected property, as opposed to an object that can contain multiple projected properties. + * + * Examples: + * - This predicate holds for `_.get({a: 'b'}, 'a')`, which returns `'b'`, + * - This predicate does not hold for `_.pick({a: 'b', c: 'd'}}, 'a')`, which returns `{a: 'b'}`, + */ + predicate isSingletonProjection() { custom.isSingletonProjection() } + +} + +/** + * A simple model of common property projection functions. + */ +private class SimplePropertyProjection extends CustomPropertyProjection { + + int objectIndex; + int selectorIndex; + boolean singleton; + + SimplePropertyProjection() { + exists (DataFlow::SourceNode callee | + this = callee.getACall() | + singleton = false and ( + ( + callee = LodashUnderscore::member("pick") and + objectIndex = 0 and + selectorIndex = [1..getNumArgument()] + ) + or + ( + callee = LodashUnderscore::member("pickBy") and + objectIndex = 0 and + selectorIndex = 1 + ) + or + exists (string name | + name = "pick" or + name = "pickAll" or + name = "pickBy" | + callee = DataFlow::moduleMember("ramda", name) and + objectIndex = 1 and + selectorIndex = 0 + ) + or + ( + callee = DataFlow::moduleMember("dotty", "search") and + objectIndex = 0 and + selectorIndex = 1 + ) + ) + or + singleton = true and ( + ( + callee = LodashUnderscore::member("get") and + objectIndex = 0 and + selectorIndex = 1 + ) + or + ( + callee = DataFlow::moduleMember("ramda", "path") and + objectIndex = 1 and + selectorIndex = 0 + ) + or + ( + callee = DataFlow::moduleMember("dottie", "get") and + objectIndex = 0 and + selectorIndex = 1 + ) + or + ( + callee = DataFlow::moduleMember("dotty", "get") and + objectIndex = 0 and + selectorIndex = 1 + ) + ) + ) + } + + override DataFlow::Node getObject() { result = getArgument(objectIndex) } + + override DataFlow::Node getASelector() { result = getArgument(selectorIndex) } + + override predicate isSingletonProjection() { singleton = true } + +} + +/** + * A taint step for a property projection. + */ +private class PropertyProjectionTaintStep extends TaintTracking::AdditionalTaintStep { + + PropertyProjection projection; + + PropertyProjectionTaintStep() { + projection = this + } + + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + // reading from a tainted object yields a tainted result + this = succ and + pred = projection.getObject() + } +} \ No newline at end of file diff --git a/javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyInjectionTaint.expected b/javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyInjectionTaint.expected new file mode 100644 index 00000000000..9244a0a9491 --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyInjectionTaint.expected @@ -0,0 +1,3 @@ +| tst.js:25:10:25:15 | source | +| tst.js:32:10:32:27 | _.pick(tainted, s) | +| tst.js:33:10:33:26 | _.get(tainted, s) | diff --git a/javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyInjectionTaint.ql b/javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyInjectionTaint.ql new file mode 100644 index 00000000000..c232f182030 --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyInjectionTaint.ql @@ -0,0 +1,22 @@ +import javascript + +class ExampleConfiguration extends TaintTracking::Configuration { + + ExampleConfiguration() { this = "ExampleConfiguration" } + + override predicate isSource(DataFlow::Node source) { + source.asExpr().(CallExpr).getCalleeName() = "SOURCE" + } + + override predicate isSink(DataFlow::Node sink) { + exists (CallExpr callExpr | + callExpr.getCalleeName() = "SINK" and + DataFlow::valueNode(callExpr.getArgument(0)) = sink + ) + } + +} + +from ExampleConfiguration cfg, DataFlow::Node source, DataFlow::Node sink +where cfg.hasFlow(source, sink) +select sink \ No newline at end of file diff --git a/javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyProjection.expected b/javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyProjection.expected new file mode 100644 index 00000000000..fb50e1d7e02 --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyProjection.expected @@ -0,0 +1,15 @@ +| tst.js:6:1:6:17 | _.pick(o, s1, s2) | tst.js:6:8:6:8 | o | tst.js:6:11:6:12 | s1 | false | +| tst.js:6:1:6:17 | _.pick(o, s1, s2) | tst.js:6:8:6:8 | o | tst.js:6:15:6:16 | s2 | false | +| tst.js:7:1:7:14 | _.pickBy(o, s) | tst.js:7:10:7:10 | o | tst.js:7:13:7:13 | s | false | +| tst.js:9:1:9:12 | R.pick(s, o) | tst.js:9:11:9:11 | o | tst.js:9:8:9:8 | s | false | +| tst.js:10:1:10:14 | R.pickBy(s, o) | tst.js:10:13:10:13 | o | tst.js:10:10:10:10 | s | false | +| tst.js:11:1:11:15 | R.pickAll(s, o) | tst.js:11:14:11:14 | o | tst.js:11:11:11:11 | s | false | +| tst.js:13:1:13:11 | _.get(o, s) | tst.js:13:7:13:7 | o | tst.js:13:10:13:10 | s | true | +| tst.js:15:1:15:12 | R.path(s, o) | tst.js:15:11:15:11 | o | tst.js:15:8:15:8 | s | true | +| tst.js:17:1:17:16 | dottie.get(o, s) | tst.js:17:12:17:12 | o | tst.js:17:15:17:15 | s | true | +| tst.js:19:1:19:15 | dotty.get(o, s) | tst.js:19:11:19:11 | o | tst.js:19:14:19:14 | s | true | +| tst.js:20:1:20:18 | dotty.search(o, s) | tst.js:20:14:20:14 | o | tst.js:20:17:20:17 | s | false | +| tst.js:27:10:27:30 | _.pick( ... ted, s) | tst.js:27:17:27:26 | notTainted | tst.js:27:29:27:29 | s | false | +| tst.js:28:10:28:29 | _.get(notTainted, s) | tst.js:28:16:28:25 | notTainted | tst.js:28:28:28:28 | s | true | +| tst.js:32:10:32:27 | _.pick(tainted, s) | tst.js:32:17:32:23 | tainted | tst.js:32:26:32:26 | s | false | +| tst.js:33:10:33:26 | _.get(tainted, s) | tst.js:33:16:33:22 | tainted | tst.js:33:25:33:25 | s | true | diff --git a/javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyProjection.ql b/javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyProjection.ql new file mode 100644 index 00000000000..6f27bbc0628 --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyProjection.ql @@ -0,0 +1,5 @@ +import javascript + +from PropertyProjection p, boolean singleton +where if p.isSingletonProjection() then singleton = true else singleton = false +select p, p.getObject(), p.getASelector(), singleton \ No newline at end of file diff --git a/javascript/ql/test/library-tests/frameworks/PropertyProjection/tst.js b/javascript/ql/test/library-tests/frameworks/PropertyProjection/tst.js new file mode 100644 index 00000000000..db141b6eb3c --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/PropertyProjection/tst.js @@ -0,0 +1,34 @@ +var _ = require("lodash"), + dotty = require("dotty"), + dottie = require("dottie"), + R = require("ramda"); + +_.pick(o, s1, s2); +_.pickBy(o, s); + +R.pick(s, o); +R.pickBy(s, o); +R.pickAll(s, o); + +_.get(o, s); + +R.path(s, o); + +dottie.get(o, s); + +dotty.get(o, s); +dotty.search(o, s); + +(function(){ + var source = SOURCE(); + + SINK(source); + + SINK(_.pick(notTainted, s)); + SINK(_.get(notTainted, s)); + + var tainted = {}; + tainted[x] = source; + SINK(_.pick(tainted, s)); + SINK(_.get(tainted, s)); +});