From 9fc5c0bdb87db5967d35547dc61c7e9d8e41e625 Mon Sep 17 00:00:00 2001 From: Asger Feldthaus Date: Tue, 27 Oct 2020 14:31:50 +0000 Subject: [PATCH] JS: Update ComposedFunctions --- .../frameworks/ComposedFunctions.qll | 138 ++++++++++++++---- .../ComposedFunctions/compose.expected | 20 +-- .../frameworks/ComposedFunctions/tst.js | 35 +++-- 3 files changed, 138 insertions(+), 55 deletions(-) diff --git a/javascript/ql/src/semmle/javascript/frameworks/ComposedFunctions.qll b/javascript/ql/src/semmle/javascript/frameworks/ComposedFunctions.qll index 7f56771b8a8..9427581b9b8 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/ComposedFunctions.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/ComposedFunctions.qll @@ -5,31 +5,110 @@ import javascript /** - * A function composed from a collection of functions. + * A call to a function that constructs a function composition `f(g(h(...)))` from a + * series functions `f, g, h, ...`. */ -private class ComposedFunction extends DataFlow::CallNode { - ComposedFunction() { - exists(string name | - name = "just-compose" or - name = "compose-function" - | - this = DataFlow::moduleImport(name).getACall() - ) - or - this = LodashUnderscore::member("flow").getACall() +class FunctionCompositionCall extends DataFlow::CallNode { + FunctionCompositionCall::Range range; + + FunctionCompositionCall() { this = range } + + /** + * Gets the `i`th function in the composition `f(g(h(...)))`, counting from left to right. + * + * Note that this is the opposite of the order in which the function are invoked, + * that is, `g` occurs later than `f` in `f(g(...))` but is invoked before `f`. + */ + DataFlow::Node getOperandNode(int i) { result = range.getOperandNode(i) } + + /** Gets a node holding one of the functions to be composed. */ + final DataFlow::Node getAnOperandNode() { result = getOperandNode(_) } + + /** + * Gets the function flowing into the `i`th function in the composition `f(g(h(...)))`. + * + * Note that this is the opposite of the order in which the function are invoked, + * that is, `g` occurs later than `f` in `f(g(...))` but is invoked before `f`. + */ + final DataFlow::FunctionNode getOperandFunction(int i) { + result = getOperandNode(i).getALocalSource() + } + + /** Gets any of the functions being composed. */ + final DataFlow::Node getAnOperandFunction() { result = getOperandFunction(_) } + + /** Gets the number of functions being composed. */ + int getNumOperand() { result = range.getNumOperand() } +} + +module FunctionCompositionCall { + abstract class Range extends DataFlow::CallNode { + /** + * Gets the function flowing into the `i`th function in the composition `f(g(h(...)))`. + */ + abstract DataFlow::Node getOperandNode(int i); + + /** Gets the number of functions being composed. */ + abstract int getNumOperand(); } /** - * Gets the ith function in this composition. + * A function composition call that accepts its operands in an array or + * via the arguments list. + * + * For simplicity, we model every composition function as if it supported this. */ - DataFlow::FunctionNode getFunction(int i) { result.flowsTo(getArgument(i)) } + private abstract class WithArrayOverloading extends Range { + /** Gets the `i`th argument to the call or the `i`th array element passed into the call. */ + DataFlow::Node getEffectiveArgument(int i) { + result = getArgument(0).(DataFlow::ArrayCreationNode).getElement(i) + or + not getArgument(0) instanceof DataFlow::ArrayCreationNode and + result = getArgument(i) + } + + override int getNumOperand() { + result = getArgument(0).(DataFlow::ArrayCreationNode).getSize() + or + not getArgument(0) instanceof DataFlow::ArrayCreationNode and + result = getNumArgument() + } + } + + /** A call whose arguments are functions `f,g,h` which are composed into `f(g(h(...))` */ + private class RightToLeft extends WithArrayOverloading { + RightToLeft() { + this = DataFlow::moduleImport(["compose-function"]).getACall() + or + this = DataFlow::moduleMember(["redux", "ramda"], "compose").getACall() + or + this = LodashUnderscore::member("flowRight").getACall() + } + + override DataFlow::Node getOperandNode(int i) { + result = getEffectiveArgument(i) + } + } + + /** A call whose arguments are functions `f,g,h` which are composed into `f(g(h(...))` */ + private class LeftToRight extends WithArrayOverloading { + LeftToRight() { + this = DataFlow::moduleImport("just-compose").getACall() + or + this = LodashUnderscore::member("flow").getACall() + } + + override DataFlow::Node getOperandNode(int i) { + result = getEffectiveArgument(getNumOperand() - i - 1) + } + } } /** * A taint step for a composed function. */ private class ComposedFunctionTaintStep extends TaintTracking::AdditionalTaintStep { - ComposedFunction composed; + FunctionCompositionCall composed; DataFlow::CallNode call; ComposedFunctionTaintStep() { @@ -38,25 +117,24 @@ private class ComposedFunctionTaintStep extends TaintTracking::AdditionalTaintSt } override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - exists(int fnIndex, DataFlow::FunctionNode fn | fn = composed.getFunction(fnIndex) | + exists(int fnIndex, DataFlow::FunctionNode fn | fn = composed.getOperandFunction(fnIndex) | // flow out of the composed call - fnIndex = composed.getNumArgument() - 1 and - pred = fn.getAReturn() and + fnIndex = 0 and + pred = fn.getReturnNode() and succ = this or - if fnIndex = 0 - then - // flow into the first composed function - exists(int callArgIndex | - pred = call.getArgument(callArgIndex) and - succ = fn.getParameter(callArgIndex) - ) - else - // flow through the composed functions - exists(DataFlow::FunctionNode predFn | predFn = composed.getFunction(fnIndex - 1) | - pred = predFn.getAReturn() and - succ = fn.getParameter(0) - ) + // flow into the first function + fnIndex = composed.getNumOperand() - 1 and + exists(int callArgIndex | + pred = call.getArgument(callArgIndex) and + succ = fn.getParameter(callArgIndex) + ) + or + // flow through the composed functions + exists(DataFlow::FunctionNode predFn | predFn = composed.getOperandFunction(fnIndex + 1) | + pred = predFn.getReturnNode() and + succ = fn.getParameter(0) + ) ) } } diff --git a/javascript/ql/test/library-tests/frameworks/ComposedFunctions/compose.expected b/javascript/ql/test/library-tests/frameworks/ComposedFunctions/compose.expected index 52b552b3d38..932f4ea6d43 100644 --- a/javascript/ql/test/library-tests/frameworks/ComposedFunctions/compose.expected +++ b/javascript/ql/test/library-tests/frameworks/ComposedFunctions/compose.expected @@ -1,13 +1,13 @@ | tst.js:10:10:10:15 | source | | tst.js:15:10:15:13 | f1() | -| tst.js:20:10:20:23 | compose1(f2)() | -| tst.js:28:10:28:27 | compose1(f3, f4)() | -| tst.js:33:10:33:28 | compose1(o.f, f5)() | -| tst.js:41:10:41:27 | compose1(f6, f7)() | -| tst.js:49:10:49:33 | compose ... source) | -| tst.js:61:10:61:40 | compose ... source) | -| tst.js:66:10:66:30 | compose ... source) | +| tst.js:20:10:20:24 | lcompose1(f2)() | +| tst.js:28:10:28:28 | lcompose1(f3, f4)() | +| tst.js:33:10:33:29 | lcompose1(o.f, f5)() | +| tst.js:41:10:41:28 | lcompose1(f6, f7)() | +| tst.js:49:10:49:34 | lcompos ... source) | +| tst.js:61:10:61:41 | lcompos ... source) | +| tst.js:66:10:66:31 | lcompos ... source) | | tst.js:89:10:89:31 | f18(und ... source) | -| tst.js:94:10:94:24 | compose2(f19)() | -| tst.js:99:10:99:24 | compose3(f20)() | -| tst.js:104:10:104:24 | compose4(f21)() | \ No newline at end of file +| tst.js:94:10:94:30 | rcompos ... o.f)() | +| tst.js:99:10:99:30 | lcompos ... f20)() | +| tst.js:104:10:104:30 | lcompos ... f21)() | diff --git a/javascript/ql/test/library-tests/frameworks/ComposedFunctions/tst.js b/javascript/ql/test/library-tests/frameworks/ComposedFunctions/tst.js index e6c7a6804d4..95426079a03 100644 --- a/javascript/ql/test/library-tests/frameworks/ComposedFunctions/tst.js +++ b/javascript/ql/test/library-tests/frameworks/ComposedFunctions/tst.js @@ -1,8 +1,8 @@ -import compose1 from 'just-compose'; -import compose2 from 'compose-function'; -import compose3 from 'lodash.flow'; +import lcompose1 from 'just-compose'; +import rcompose2 from 'compose-function'; +import lcompose3 from 'lodash.flow'; import _ from 'lodash'; -var compose4 = _.flow; +var lcompose4 = _.flow; (function(){ var source = SOURCE(); @@ -17,7 +17,7 @@ var compose4 = _.flow; function f2(){ return source; } - SINK(compose1(f2)()); + SINK(lcompose1(f2)()); function f3(){ @@ -25,12 +25,12 @@ var compose4 = _.flow; function f4(){ return source; } - SINK(compose1(f3, f4)()); + SINK(lcompose1(f3, f4)()); function f5(){ return source; } - SINK(compose1(o.f, f5)()); + SINK(lcompose1(o.f, f5)()); function f6(){ return source; @@ -38,7 +38,7 @@ var compose4 = _.flow; function f7(x){ return x; } - SINK(compose1(f6, f7)()); + SINK(lcompose1(f6, f7)()); function f8(x){ return x; @@ -46,7 +46,7 @@ var compose4 = _.flow; function f9(x){ return x; } - SINK(compose1(f8, f9)(source)); + SINK(lcompose1(f8, f9)(source)); function f10(x){ @@ -58,12 +58,12 @@ var compose4 = _.flow; function f12(x){ return x; } - SINK(compose1(f10, f11, f12)(source)); + SINK(lcompose1(f10, f11, f12)(source)); function f13(x){ return x + 'foo' ; } - SINK(compose1(f13)(source)); + SINK(lcompose1(f13)(source)); function f14(){ return undefined; @@ -76,7 +76,7 @@ var compose4 = _.flow; function f16(){ return undefined; } - SINK(compose1(f15, f16)()); // NO FLOW + SINK(lcompose1(f15, f16)()); // NO FLOW function f17(x, y){ return y; @@ -91,16 +91,21 @@ var compose4 = _.flow; function f19(){ return source; } - SINK(compose2(f19)()); + SINK(rcompose2(f19, o.f)()); function f20(){ return source; } - SINK(compose3(f20)()); + SINK(lcompose3(f16, f20)()); function f21(){ return source; } - SINK(compose4(f21)()); + SINK(lcompose4(f16, f21)()); + + function f22(){ + return source; + } + SINK(lcompose3(f22, f16)()); // NO FLOW })();