diff --git a/javascript/ql/src/semmle/javascript/Arrays.qll b/javascript/ql/src/semmle/javascript/Arrays.qll index 5bbcc00a0c1..226558e0fca 100644 --- a/javascript/ql/src/semmle/javascript/Arrays.qll +++ b/javascript/ql/src/semmle/javascript/Arrays.qll @@ -9,12 +9,9 @@ module ArrayTaintTracking { /** * A taint propagating data flow edge caused by the builtin array functions. */ - private class ArrayFunctionTaintStep extends TaintTracking::AdditionalTaintStep, - DataFlow::CallNode { - ArrayFunctionTaintStep() { arrayFunctionTaintStep(_, _, this) } - + private class ArrayFunctionTaintStep extends TaintTracking::AdditionalTaintStep{ override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - arrayFunctionTaintStep(pred, succ, this) + arrayFunctionTaintStep(pred, succ, _) } } diff --git a/javascript/ql/src/semmle/javascript/Base64.qll b/javascript/ql/src/semmle/javascript/Base64.qll index bda1c07b953..a135e57b226 100644 --- a/javascript/ql/src/semmle/javascript/Base64.qll +++ b/javascript/ql/src/semmle/javascript/Base64.qll @@ -68,13 +68,11 @@ module Base64 { * to avoid false positives. */ private class Base64DecodingStep extends TaintTracking::AdditionalTaintStep { - Decode dec; - - Base64DecodingStep() { this = dec } - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = dec.getInput() and - succ = dec.getOutput() + exists(Decode dec | + pred = dec.getInput() and + succ = dec.getOutput() + ) } } } diff --git a/javascript/ql/src/semmle/javascript/Extend.qll b/javascript/ql/src/semmle/javascript/Extend.qll index 5943aa78320..0ee242f0e84 100644 --- a/javascript/ql/src/semmle/javascript/Extend.qll +++ b/javascript/ql/src/semmle/javascript/Extend.qll @@ -166,13 +166,11 @@ private class FunctionalExtendCallShallow extends ExtendCall { * and to the source of the destination object. */ private class ExtendCallTaintStep extends TaintTracking::AdditionalTaintStep { - ExtendCall extend; - - ExtendCallTaintStep() { this = extend } - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = extend.getASourceOperand() and succ = extend.getDestinationOperand().getALocalSource() - or - pred = extend.getAnOperand() and succ = extend + exists(ExtendCall extend | + pred = extend.getASourceOperand() and succ = extend.getDestinationOperand().getALocalSource() + or + pred = extend.getAnOperand() and succ = extend + ) } } diff --git a/javascript/ql/src/semmle/javascript/Promises.qll b/javascript/ql/src/semmle/javascript/Promises.qll index 95e78a219ca..58b581e2675 100644 --- a/javascript/ql/src/semmle/javascript/Promises.qll +++ b/javascript/ql/src/semmle/javascript/Promises.qll @@ -460,13 +460,7 @@ predicate promiseTaintStep(DataFlow::Node pred, DataFlow::Node succ) { * An additional taint step that involves promises. */ private class PromiseTaintStep extends TaintTracking::AdditionalTaintStep { - DataFlow::Node source; - - PromiseTaintStep() { promiseTaintStep(source, this) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = source and succ = this - } + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { promiseTaintStep(pred, succ) } } /** @@ -500,14 +494,13 @@ private module AsyncReturnSteps { /** * A data-flow step for ordinary return from an async function in a taint configuration. */ - private class AsyncTaintReturn extends TaintTracking::AdditionalTaintStep, DataFlow::FunctionNode { - Function f; - - AsyncTaintReturn() { this.getFunction() = f and f.isAsync() } - + private class AsyncTaintReturn extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - returnExpr(f, pred, _) and - succ.(DataFlow::FunctionReturnNode).getFunction() = f + exists(Function f | + f.isAsync() and + returnExpr(f, pred, _) and + succ.(DataFlow::FunctionReturnNode).getFunction() = f + ) } } } @@ -617,13 +610,11 @@ private module ClosurePromise { * Taint steps through closure promise methods. */ private class ClosurePromiseTaintStep extends TaintTracking::AdditionalTaintStep { - DataFlow::Node pred; - - ClosurePromiseTaintStep() { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { // static methods in goog.Promise exists(DataFlow::CallNode call, string name | call = Closure::moduleImport("goog.Promise." + name).getACall() and - this = call and + succ = call and pred = call.getAnArgument() | name = "all" or @@ -635,12 +626,10 @@ private module ClosurePromise { // promise created through goog.promise.withResolver() exists(DataFlow::CallNode resolver | resolver = Closure::moduleImport("goog.Promise.withResolver").getACall() and - this = resolver.getAPropertyRead("promise") and + succ = resolver.getAPropertyRead("promise") and pred = resolver.getAMethodCall("resolve").getArgument(0) ) } - - override predicate step(DataFlow::Node src, DataFlow::Node dst) { src = pred and dst = this } } } diff --git a/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll b/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll index 204524a0173..ef056905d33 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll @@ -206,6 +206,8 @@ module TaintTracking { override predicate sanitizes(boolean outcome, Expr e) { none() } } + private newtype TUnit = TUnitInjector() + /** * A taint-propagating data flow edge that should be added to all taint tracking * configurations in addition to standard data flow edges. @@ -214,14 +216,15 @@ module TaintTracking { * of the standard library. Override `Configuration::isAdditionalTaintStep` * for analysis-specific taint steps. */ - cached - abstract class AdditionalTaintStep extends DataFlow::Node { + class AdditionalTaintStep extends TUnit { /** * Holds if `pred` → `succ` should be considered a taint-propagating * data flow edge. */ cached abstract predicate step(DataFlow::Node pred, DataFlow::Node succ); + + string toString() { result = "Additional taint step class" } } /** @@ -229,11 +232,7 @@ module TaintTracking { * promises. */ private class HeapTaintStep extends AdditionalTaintStep { - HeapTaintStep() { heapStep(_, this) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - heapStep(pred, succ) and succ = this - } + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { heapStep(pred, succ) } } /** @@ -280,16 +279,15 @@ module TaintTracking { * A taint propagating data flow edge through persistent storage. */ class PersistentStorageTaintStep extends AdditionalTaintStep { - PersistentReadAccess read; - - PersistentStorageTaintStep() { this = read } - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = read.getAWrite().getValue() and - succ = read + persistentStorageTaintStep(pred, succ) } } + predicate persistentStorageTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + pred = succ.(PersistentReadAccess).getAWrite().getValue() + } + predicate arrayFunctionTaintStep = ArrayTaintTracking::arrayFunctionTaintStep/3; /** @@ -303,10 +301,7 @@ module TaintTracking { * map to be tainted as soon as one of its entries is. */ private class DictionaryTaintStep extends AdditionalTaintStep { - DictionaryTaintStep() { dictionaryTaintStep(_, this) } - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - succ = this and dictionaryTaintStep(pred, succ) } } @@ -328,9 +323,7 @@ module TaintTracking { * also is an instance of `C`. */ private class ReactComponentStateTaintStep extends AdditionalTaintStep { - DataFlow::Node source; - - ReactComponentStateTaintStep() { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { exists(ReactComponent c, DataFlow::PropRead prn, DataFlow::PropWrite pwn | ( c.getACandidateStateSource().flowsTo(pwn.getBase()) or @@ -342,14 +335,10 @@ module TaintTracking { ) | prn.getPropertyName() = pwn.getPropertyName() and - this = prn and - source = pwn.getRhs() + succ = prn and + pred = pwn.getRhs() ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = source and succ = this - } } /** @@ -359,21 +348,15 @@ module TaintTracking { * also is an instance of `C`. */ private class ReactComponentPropsTaintStep extends AdditionalTaintStep { - DataFlow::Node source; - - ReactComponentPropsTaintStep() { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { exists(ReactComponent c, string name, DataFlow::PropRead prn | prn = c.getAPropRead(name) or prn = c.getAPreviousPropsSource().getAPropertyRead(name) | - source = c.getACandidatePropsValue(name) and - this = prn + pred = c.getACandidatePropsValue(name) and + succ = prn ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = source and succ = this - } } /** @@ -383,10 +366,7 @@ module TaintTracking { * we consider any `+` operation to propagate taint. */ class StringConcatenationTaintStep extends AdditionalTaintStep { - StringConcatenationTaintStep() { StringConcatenation::taintStep(_, this) } - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - succ = this and StringConcatenation::taintStep(pred, succ) } } @@ -395,11 +375,8 @@ module TaintTracking { * A taint propagating data flow edge arising from string manipulation * functions defined in the standard library. */ - private class StringManipulationTaintStep extends AdditionalTaintStep, DataFlow::ValueNode { - StringManipulationTaintStep() { stringManipulationStep(_, this) } - + private class StringManipulationTaintStep extends AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - succ = this and stringManipulationStep(pred, succ) } } @@ -488,16 +465,8 @@ module TaintTracking { * A taint propagating data flow edge arising from string formatting. */ private class StringFormattingTaintStep extends AdditionalTaintStep { - PrintfStyleCall call; - - StringFormattingTaintStep() { - this = call and - call.returnsFormatted() - } - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - succ = this and - ( + exists(PrintfStyleCall call | call.returnsFormatted() and succ = call | pred = call.getFormatString() or pred = call.getFormatArgument(_) @@ -510,59 +479,50 @@ module TaintTracking { * `RegExp.prototype.exec` to its result. */ private class RegExpExecTaintStep extends AdditionalTaintStep { - DataFlow::MethodCallNode self; - - RegExpExecTaintStep() { - this = self and - self.getReceiver().analyze().getAType() = TTRegExp() and - self.getMethodName() = "exec" and - self.getNumArgument() = 1 - } - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = self.getArgument(0) and - succ = this + exists(DataFlow::MethodCallNode call | + call.getReceiver().analyze().getAType() = TTRegExp() and + call.getMethodName() = "exec" and + call.getNumArgument() = 1 and + pred = call.getArgument(0) and + succ = call + ) } } /** * A taint propagating data flow edge arising from calling `String.prototype.match()`. */ - private class StringMatchTaintStep extends AdditionalTaintStep, DataFlow::MethodCallNode { - StringMatchTaintStep() { - this.getMethodName() = "match" and - this.getNumArgument() = 1 and - this.getArgument(0).analyze().getAType() = TTRegExp() - } - + private class StringMatchTaintStep extends AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = this.getReceiver() and - succ = this + exists(DataFlow::MethodCallNode call | + pred = call.getReceiver() and + succ = call and + call.getMethodName() = "match" and + call.getNumArgument() = 1 and + call.getArgument(0).analyze().getAType() = TTRegExp() + ) } } /** * A taint propagating data flow edge arising from JSON unparsing. */ - private class JsonStringifyTaintStep extends AdditionalTaintStep, DataFlow::CallNode { - JsonStringifyTaintStep() { this instanceof JsonStringifyCall } - + private class JsonStringifyTaintStep extends AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getArgument(0) and succ = this + pred = succ.(JsonStringifyCall).getArgument(0) } } /** * A taint propagating data flow edge arising from JSON parsing. */ - private class JsonParserTaintStep extends AdditionalTaintStep, DataFlow::CallNode { - JsonParserCall call; - - JsonParserTaintStep() { this = call } - + private class JsonParserTaintStep extends AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = call.getInput() and - succ = call.getOutput() + exists(JsonParserCall call | + pred = call.getInput() and + succ = call.getOutput() + ) } } @@ -664,33 +624,29 @@ module TaintTracking { /** * A taint propagating data flow edge arising from sorting. */ - private class SortTaintStep extends AdditionalTaintStep, DataFlow::MethodCallNode { - SortTaintStep() { getMethodName() = "sort" } - + private class SortTaintStep extends AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getReceiver() and succ = this + exists(DataFlow::MethodCallNode call | call.getMethodName() = "sort" | + pred = call.getReceiver() and succ = call + ) } } /** * A taint step through an exception constructor, such as `x` to `new Error(x)`. */ - class ErrorConstructorTaintStep extends AdditionalTaintStep, DataFlow::InvokeNode { - ErrorConstructorTaintStep() { - exists(string name | this = DataFlow::globalVarRef(name).getAnInvocation() | - name = "Error" or - name = "EvalError" or - name = "RangeError" or - name = "ReferenceError" or - name = "SyntaxError" or - name = "TypeError" or - name = "URIError" - ) - } - + class ErrorConstructorTaintStep extends AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getArgument(0) and - succ = this + exists(DataFlow::InvokeNode call | + call = + DataFlow::globalVarRef([ + "Error", "EvalError", "RangeError", "ReferenceError", "SyntaxError", "TypeError", + "URIError" + ]).getAnInvocation() + | + pred = call.getArgument(0) and + succ = call + ) } } @@ -756,11 +712,8 @@ module TaintTracking { } private class StaticRegExpCaptureStep extends AdditionalTaintStep { - StaticRegExpCaptureStep() { staticRegExpCaptureStep(this, _) } - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = this and - staticRegExpCaptureStep(this, succ) + staticRegExpCaptureStep(pred, succ) } } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/Angular2.qll b/javascript/ql/src/semmle/javascript/frameworks/Angular2.qll index e418df6704c..79e9b22a536 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Angular2.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Angular2.qll @@ -166,8 +166,6 @@ module Angular2 { } private class AngularTaintStep extends TaintTracking::AdditionalTaintStep { - AngularTaintStep() { taintStep(_, this) } - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { taintStep(pred, succ) } } @@ -469,13 +467,11 @@ module Angular2 { * a step from `array` to each access to `elem`. */ private class ForLoopStep extends TaintTracking::AdditionalTaintStep { - ForLoopAttribute attrib; - - ForLoopStep() { this = attrib.getIterationDomain() } - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = this and - succ = attrib.getAnIteratorAccess() + exists(ForLoopAttribute attrib | + pred = attrib.getIterationDomain() and + succ = attrib.getAnIteratorAccess() + ) } } @@ -498,27 +494,23 @@ module Angular2 { result.getCalleeNode().asExpr().(PipeRefExpr).getName() = name } - private class BuiltinPipeStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - string name; - - BuiltinPipeStep() { this = getAPipeCall(name) } - + private class BuiltinPipeStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - succ = this and - exists(int i | pred = getArgument(i) | - i = 0 and - name = - [ - "async", "i18nPlural", "json", "keyvalue", "lowercase", "uppercase", "titlecase", - "slice" - ] + exists(DataFlow::CallNode call, string name | call = getAPipeCall(name) and succ = call | + exists(int i | pred = call.getArgument(i) | + i = 0 and + name = + [ + "async", "i18nPlural", "json", "keyvalue", "lowercase", "uppercase", "titlecase", + "slice" + ] + or + i = 1 and name = "date" // date format string + ) or - i = 1 and name = "date" // date format string + name = "translate" and + pred = [call.getArgument(1), call.getOptionArgument(1, _)] ) - or - name = "translate" and - succ = this and - pred = [getArgument(1), getOptionArgument(1, _)] } } @@ -568,26 +560,25 @@ module Angular2 { * ``` */ private class MatTableTaintStep extends TaintTracking::AdditionalTaintStep { - MatTableElement table; - - MatTableTaintStep() { this = table.getDataSourceNode() } - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = this and - succ = table.getARowRef() + exists(MatTableElement table | + pred = table.getDataSourceNode() and + succ = table.getARowRef() + ) } } /** A taint step into the data array of a `MatTableDataSource` instance. */ - private class MatTableDataSourceStep extends TaintTracking::AdditionalTaintStep, DataFlow::NewNode { - MatTableDataSourceStep() { - this = - DataFlow::moduleMember("@angular/material/table", "MatTableDataSource").getAnInstantiation() - } - + private class MatTableDataSourceStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = [getArgument(0), getAPropertyWrite("data").getRhs()] and - succ = this + exists(DataFlow::NewNode new | + new = + DataFlow::moduleMember("@angular/material/table", "MatTableDataSource") + .getAnInstantiation() + | + pred = [new.getArgument(0), new.getAPropertyWrite("data").getRhs()] and + succ = new + ) } } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/AsyncPackage.qll b/javascript/ql/src/semmle/javascript/frameworks/AsyncPackage.qll index aaa8d57f49b..d9fe489fdf1 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/AsyncPackage.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/AsyncPackage.qll @@ -150,11 +150,11 @@ module AsyncPackage { * * For example: `data -> item` in `async.each(data, (item, cb) => {})`. */ - private class IterationInputTaintStep extends TaintTracking::AdditionalTaintStep, IterationCall { + private class IterationInputTaintStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - exists(DataFlow::FunctionNode iteratee | - iteratee = getIteratorCallback() and // Require a closure to avoid spurious call/return mismatch. - pred = getCollection() and + exists(IterationCall iteration, DataFlow::FunctionNode iteratee | + iteratee = iteration.getIteratorCallback() and // Require a closure to avoid spurious call/return mismatch. + pred = iteration.getCollection() and succ = iteratee.getParameter(0) ) } @@ -166,18 +166,15 @@ module AsyncPackage { * * For example: `item + taint()` -> result` in `async.map(data, (item, cb) => cb(null, item + taint()), (err, result) => {})`. */ - private class IterationOutputTaintStep extends TaintTracking::AdditionalTaintStep, IterationCall { - IterationOutputTaintStep() { - name = "concat" or - name = "map" or - name = "reduce" or - name = "reduceRight" - } - + private class IterationOutputTaintStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - exists(DataFlow::FunctionNode iteratee, DataFlow::FunctionNode final, int i | - iteratee = getIteratorCallback().getALocalSource() and - final = getFinalCallback() and // Require a closure to avoid spurious call/return mismatch. + exists( + IterationCall iteration, DataFlow::FunctionNode iteratee, DataFlow::FunctionNode final, + int i + | + iteration.getName() = ["concat", "map", "reduce", "reduceRight"] and + iteratee = iteration.getIteratorCallback().getALocalSource() and + final = iteration.getFinalCallback() and // Require a closure to avoid spurious call/return mismatch. pred = getLastParameter(iteratee).getACall().getArgument(i) and succ = final.getParameter(i) ) @@ -189,16 +186,13 @@ module AsyncPackage { * * For example: `data -> result` in `async.sortBy(data, orderingFn, (err, result) => {})`. */ - private class IterationPreserveTaintStep extends TaintTracking::AdditionalTaintStep, IterationCall { - IterationPreserveTaintStep() { - name = "sortBy" - // We don't currently include `filter` and `reject` as they could act as sanitizers. - } - + private class IterationPreserveTaintStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - exists(DataFlow::FunctionNode final | - final = getFinalCallback() and // Require a closure to avoid spurious call/return mismatch. - pred = getCollection() and + exists(IterationCall iteration, DataFlow::FunctionNode final | + // We don't currently include `filter` and `reject` as they could act as sanitizers. + iteration.getName() = "sortBy" and + final = iteration.getFinalCallback() and // Require a closure to avoid spurious call/return mismatch. + pred = iteration.getCollection() and succ = final.getParameter(1) ) } diff --git a/javascript/ql/src/semmle/javascript/frameworks/Classnames.qll b/javascript/ql/src/semmle/javascript/frameworks/Classnames.qll index 729aa448309..1ef42458cfa 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Classnames.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Classnames.qll @@ -8,32 +8,25 @@ private DataFlow::SourceNode classnames() { result = DataFlow::moduleImport(["classnames", "classnames/dedupe", "classnames/bind"]) } -private class PlainStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - PlainStep() { - this = classnames().getACall() - or - this = DataFlow::moduleImport("clsx").getACall() - } - +private class PlainStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getAnArgument() and - succ = this + exists(DataFlow::CallNode call | + call = classnames().getACall() or call = DataFlow::moduleImport("clsx").getACall() + | + pred = call.getAnArgument() and + succ = call + ) } } /** * Step from `x` or `y` to the result of `classnames.bind(x)(y)`. */ -private class BindStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - DataFlow::CallNode bind; - - BindStep() { - bind = classnames().getAMemberCall("bind") and - this = bind.getACall() - } - +private class BindStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = [getAnArgument(), bind.getAnArgument(), bind.getOptionArgument(_, _)] and - succ = this + exists(DataFlow::CallNode bind | bind = classnames().getAMemberCall("bind") | + pred = [succ.(DataFlow::CallNode).getAnArgument(), bind.getAnArgument(), bind.getOptionArgument(_, _)] and + succ = bind.getACall() + ) } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/ClosureLibrary.qll b/javascript/ql/src/semmle/javascript/frameworks/ClosureLibrary.qll index b220c05dbc2..9e2a7ce7761 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/ClosureLibrary.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/ClosureLibrary.qll @@ -7,12 +7,13 @@ import javascript module ClosureLibrary { private import DataFlow - private class StringStep extends TaintTracking::AdditionalTaintStep, CallNode { - Node pred; - - StringStep() { - exists(string name | this = Closure::moduleImport("goog.string." + name).getACall() | - pred = getAnArgument() and + private class StringStep extends TaintTracking::AdditionalTaintStep { + override predicate step(Node pred, Node succ) { + exists(CallNode call, string name | + call = Closure::moduleImport("goog.string." + name).getACall() and + succ = call + | + pred = call.getAnArgument() and ( name = "canonicalizeNewlines" or name = "capitalize" or @@ -39,7 +40,7 @@ module ClosureLibrary { name = "whitespaceEscape" ) or - pred = getArgument(0) and + pred = call.getArgument(0) and ( name = "truncate" or name = "truncateMiddle" or @@ -47,10 +48,5 @@ module ClosureLibrary { ) ) } - - override predicate step(Node src, Node dst) { - src = pred and - dst = this - } } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/ComposedFunctions.qll b/javascript/ql/src/semmle/javascript/frameworks/ComposedFunctions.qll index ab12f202c4f..647330cf5d6 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/ComposedFunctions.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/ComposedFunctions.qll @@ -114,33 +114,27 @@ module FunctionCompositionCall { * A taint step for a composed function. */ private class ComposedFunctionTaintStep extends TaintTracking::AdditionalTaintStep { - FunctionCompositionCall composed; - DataFlow::CallNode call; - - ComposedFunctionTaintStep() { - call = composed.getACall() and - this = call - } - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - exists(int fnIndex, DataFlow::FunctionNode fn | fn = composed.getOperandFunction(fnIndex) | - // flow into the first function - fnIndex = composed.getNumOperand() - 1 and - exists(int callArgIndex | - pred = call.getArgument(callArgIndex) and - succ = fn.getParameter(callArgIndex) + exists(FunctionCompositionCall composed, DataFlow::CallNode call | call = composed.getACall() | + exists(int fnIndex, DataFlow::FunctionNode fn | fn = composed.getOperandFunction(fnIndex) | + // 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) + ) + or + // flow out of the composed call + fnIndex = 0 and + pred = fn.getReturnNode() and + succ = call ) - or - // flow through the composed functions - exists(DataFlow::FunctionNode predFn | predFn = composed.getOperandFunction(fnIndex + 1) | - pred = predFn.getReturnNode() and - succ = fn.getParameter(0) - ) - or - // flow out of the composed call - fnIndex = 0 and - pred = fn.getReturnNode() and - succ = this ) } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/DateFunctions.qll b/javascript/ql/src/semmle/javascript/frameworks/DateFunctions.qll index bb80abbe904..9f369d2c3c7 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/DateFunctions.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/DateFunctions.qll @@ -29,24 +29,24 @@ private module DateFns { * * A format string can use single-quotes to include mostly arbitrary text. */ - private class FormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - FormatStep() { this = formatFunction().getACall() } - + private class FormatStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getArgument(1) and - succ = this + exists(DataFlow::CallNode call | call = formatFunction().getACall() | + pred = call.getArgument(1) and + succ = call + ) } } /** * Taint step of form: `f -> format(f)(date)` */ - private class CurriedFormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - CurriedFormatStep() { this = curriedFormatFunction().getACall() } - + private class CurriedFormatStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getArgument(0) and - succ = getACall() + exists(DataFlow::CallNode call | call = curriedFormatFunction().getACall() | + pred = call.getArgument(0) and + succ = call.getACall() + ) } } } @@ -66,12 +66,12 @@ private module Moment { * * The format string can use backslash-escaping to include mostly arbitrary text. */ - private class MomentFormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - MomentFormatStep() { this = moment().getMember("format").getACall() } - + private class MomentFormatStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getArgument(0) and - succ = this + exists(DataFlow::CallNode call | call = moment().getMember("format").getACall() | + pred = call.getArgument(0) and + succ = call + ) } } } @@ -82,12 +82,12 @@ private module DateFormat { * * The format string can use single-quotes to include mostly arbitrary text. */ - private class DateFormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - DateFormatStep() { this = DataFlow::moduleImport("dateformat").getACall() } - + private class DateFormatStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getArgument(1) and - succ = this + exists(DataFlow::CallNode call | call = DataFlow::moduleImport("dateformat").getACall() | + pred = call.getArgument(1) and + succ = call + ) } } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/JWT.qll b/javascript/ql/src/semmle/javascript/frameworks/JWT.qll index 9c7c29189be..ebf4b6a5353 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/JWT.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/JWT.qll @@ -11,12 +11,12 @@ private module JwtDecode { /** * A taint-step for `succ = require("jwt-decode")(pred)`. */ - private class JwtDecodeStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - JwtDecodeStep() { this = DataFlow::moduleImport("jwt-decode").getACall() } - + private class JwtDecodeStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = this.getArgument(0) and - succ = this + exists(DataFlow::CallNode call | call = DataFlow::moduleImport("jwt-decode").getACall() | + pred = call.getArgument(0) and + succ = call + ) } } } @@ -28,12 +28,14 @@ private module JsonWebToken { /** * A taint-step for `require("jsonwebtoken").verify(pred, "key", (err succ) => {...})`. */ - private class VerifyStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - VerifyStep() { this = DataFlow::moduleMember("jsonwebtoken", "verify").getACall() } - + private class VerifyStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = this.getArgument(0) and - succ = this.getABoundCallbackParameter(2, 1) + exists(DataFlow::CallNode call | + call = DataFlow::moduleMember("jsonwebtoken", "verify").getACall() + | + pred = call.getArgument(0) and + succ = call.getABoundCallbackParameter(2, 1) + ) } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/LodashUnderscore.qll b/javascript/ql/src/semmle/javascript/frameworks/LodashUnderscore.qll index 97ee73933ad..11cad629562 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/LodashUnderscore.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/LodashUnderscore.qll @@ -441,10 +441,8 @@ module LodashUnderscore { * A model for taint-steps involving (non-function) underscore methods. */ private class UnderscoreTaintStep extends TaintTracking::AdditionalTaintStep { - UnderscoreTaintStep() { underscoreTaintStep(this, _) } - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - underscoreTaintStep(pred, succ) and pred = this + underscoreTaintStep(pred, succ) } } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/Markdown.qll b/javascript/ql/src/semmle/javascript/frameworks/Markdown.qll index bff1c364d00..0ff6cf4dcaa 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Markdown.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Markdown.qll @@ -7,45 +7,45 @@ import javascript /** * A taint step for the `marked` library, that converts markdown to HTML. */ -private class MarkedStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - MarkedStep() { - this = DataFlow::globalVarRef("marked").getACall() - or - this = DataFlow::moduleImport("marked").getACall() - } - +private class MarkedStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - succ = this and - pred = this.getArgument(0) + exists(DataFlow::CallNode call | + call = DataFlow::globalVarRef("marked").getACall() + or + call = DataFlow::moduleImport("marked").getACall() + | + succ = call and + pred = call.getArgument(0) + ) } } /** * A taint step for the `markdown-table` library. */ -private class MarkdownTableStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - MarkdownTableStep() { this = DataFlow::moduleImport("markdown-table").getACall() } - +private class MarkdownTableStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - succ = this and - pred = this.getArgument(0) + exists(DataFlow::CallNode call | call = DataFlow::moduleImport("markdown-table").getACall() | + succ = call and + pred = call.getArgument(0) + ) } } /** * A taint step for the `showdown` library. */ -private class ShowDownStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - ShowDownStep() { - this = - [DataFlow::globalVarRef("showdown"), DataFlow::moduleImport("showdown")] - .getAConstructorInvocation("Converter") - .getAMemberCall(["makeHtml", "makeMd"]) - } - +private class ShowDownStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - succ = this and - pred = this.getArgument(0) + exists(DataFlow::CallNode call | + call = + [DataFlow::globalVarRef("showdown"), DataFlow::moduleImport("showdown")] + .getAConstructorInvocation("Converter") + .getAMemberCall(["makeHtml", "makeMd"]) + | + succ = call and + pred = call.getArgument(0) + ) } } @@ -93,16 +93,16 @@ private module Unified { /** * A taint step for the `unified` library. */ - class UnifiedStep extends TaintTracking::AdditionalTaintStep, UnifiedChain { - UnifiedStep() { - // sanitizer. Mostly looking for `rehype-sanitize`, but also other plugins with `sanitize` in their name. - not this.getAUsedPlugin().getALocalSource() = - DataFlow::moduleImport(any(string s | s.matches("%sanitize%"))) - } - + class UnifiedStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getInput() and - succ = getOutput() + exists(UnifiedChain chain | + // sanitizer. Mostly looking for `rehype-sanitize`, but also other plugins with `sanitize` in their name. + not chain.getAUsedPlugin().getALocalSource() = + DataFlow::moduleImport(any(string s | s.matches("%sanitize%"))) + | + pred = chain.getInput() and + succ = chain.getOutput() + ) } } } @@ -110,11 +110,11 @@ private module Unified { /** * A taint step for the `snarkdown` library. */ -private class SnarkdownStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - SnarkdownStep() { this = DataFlow::moduleImport("snarkdown").getACall() } - +private class SnarkdownStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - this = succ and - pred = this.getArgument(0) + exists(DataFlow::CallNode call | call = DataFlow::moduleImport("snarkdown").getACall() | + call = succ and + pred = call.getArgument(0) + ) } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll b/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll index a472b33aa8a..f4ef4a60dd0 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll @@ -281,74 +281,64 @@ module NodeJSLib { /** * A call to a path-module method that preserves taint. */ - private class PathFlowTarget extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - DataFlow::Node tainted; - - PathFlowTarget() { - exists(string methodName | this = NodeJSLib::Path::moduleMember(methodName).getACall() | + private class PathFlowTarget extends TaintTracking::AdditionalTaintStep { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + exists(DataFlow::CallNode call, string methodName | + call = NodeJSLib::Path::moduleMember(methodName).getACall() and succ = call + | // getters - methodName = "basename" and tainted = getArgument(0) + methodName = "basename" and pred = call.getArgument(0) or - methodName = "dirname" and tainted = getArgument(0) + methodName = "dirname" and pred = call.getArgument(0) or - methodName = "extname" and tainted = getArgument(0) + methodName = "extname" and pred = call.getArgument(0) or // transformers - methodName = "join" and tainted = getAnArgument() + methodName = "join" and pred = call.getAnArgument() or - methodName = "normalize" and tainted = getArgument(0) + methodName = "normalize" and pred = call.getArgument(0) or - methodName = "relative" and tainted = getArgument([0 .. 1]) + methodName = "relative" and pred = call.getArgument([0 .. 1]) or - methodName = "resolve" and tainted = getAnArgument() + methodName = "resolve" and pred = call.getAnArgument() or - methodName = "toNamespacedPath" and tainted = getArgument(0) + methodName = "toNamespacedPath" and pred = call.getArgument(0) ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = tainted and succ = this - } } /** * A call to a fs-module method that preserves taint. */ private class FsFlowTarget extends TaintTracking::AdditionalTaintStep { - DataFlow::Node tainted; - - FsFlowTarget() { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { exists(DataFlow::CallNode call, string methodName | call = FS::moduleMember(methodName).getACall() | methodName = "realpathSync" and - tainted = call.getArgument(0) and - this = call + pred = call.getArgument(0) and + succ = call or methodName = "realpath" and - tainted = call.getArgument(0) and - this = call.getCallback(1).getParameter(1) + pred = call.getArgument(0) and + succ = call.getCallback(1).getParameter(1) ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = tainted and succ = this - } } /** * A model of taint propagation through `new Buffer` and `Buffer.from`. */ - private class BufferTaintStep extends TaintTracking::AdditionalTaintStep, DataFlow::InvokeNode { - BufferTaintStep() { - this = DataFlow::globalVarRef("Buffer").getAnInstantiation() - or - this = DataFlow::globalVarRef("Buffer").getAMemberInvocation("from") - } - + private class BufferTaintStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getArgument(0) and - succ = this + exists(DataFlow::InvokeNode invoke | + invoke = DataFlow::globalVarRef("Buffer").getAnInstantiation() + or + invoke = DataFlow::globalVarRef("Buffer").getAMemberInvocation("from") + | + pred = invoke.getArgument(0) and + succ = invoke + ) } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/PropertyProjection.qll b/javascript/ql/src/semmle/javascript/frameworks/PropertyProjection.qll index 8f031c81c33..bb8234a4b4c 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/PropertyProjection.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/PropertyProjection.qll @@ -132,13 +132,8 @@ private class SimplePropertyProjection extends PropertyProjection::Range { * 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() + pred = succ.(PropertyProjection).getObject() } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/RxJS.qll b/javascript/ql/src/semmle/javascript/frameworks/RxJS.qll index 5fc662ef3d5..5ce2b2a63e5 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/RxJS.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/RxJS.qll @@ -7,12 +7,12 @@ private import javascript /** * A step `x -> y` in `x.subscribe(y => ...)`, modeling flow out of an rxjs Observable. */ -private class RxJsSubscribeStep extends TaintTracking::AdditionalTaintStep, DataFlow::MethodCallNode { - RxJsSubscribeStep() { getMethodName() = "subscribe" } - +private class RxJsSubscribeStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getReceiver() and - succ = getCallback(0).getParameter(0) + exists(DataFlow::MethodCallNode call | call.getMethodName() = "subscribe" | + pred = call.getReceiver() and + succ = call.getCallback(0).getParameter(0) + ) } } @@ -54,24 +54,24 @@ private predicate isIdentityPipe(DataFlow::CallNode pipe) { /** * A step in or out of the map callback in a call of form `x.pipe(map(y => ...))`. */ -private class RxJsPipeMapStep extends TaintTracking::AdditionalTaintStep, DataFlow::MethodCallNode { - RxJsPipeMapStep() { getMethodName() = "pipe" } - +private class RxJsPipeMapStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getReceiver() and - succ = pipeInput(getArgument(0).getALocalSource()) - or - exists(int i | - pred = pipeOutput(getArgument(i).getALocalSource()) and - succ = pipeInput(getArgument(i + 1).getALocalSource()) + exists(DataFlow::MethodCallNode call | call.getMethodName() = "pipe" | + pred = call.getReceiver() and + succ = pipeInput(call.getArgument(0).getALocalSource()) + or + exists(int i | + pred = pipeOutput(call.getArgument(i).getALocalSource()) and + succ = pipeInput(call.getArgument(i + 1).getALocalSource()) + ) + or + pred = pipeOutput(call.getLastArgument().getALocalSource()) and + succ = call + or + // Handle a common case where the last step is `catchError`. + isIdentityPipe(call.getLastArgument().getALocalSource()) and + pred = pipeOutput(call.getArgument(call.getNumArgument() - 2)) and + succ = call ) - or - pred = pipeOutput(getLastArgument().getALocalSource()) and - succ = this - or - // Handle a common case where the last step is `catchError`. - isIdentityPipe(getLastArgument().getALocalSource()) and - pred = pipeOutput(getArgument(getNumArgument() - 2)) and - succ = this } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/Typeahead.qll b/javascript/ql/src/semmle/javascript/frameworks/Typeahead.qll index 83001dd2e09..d7a47f2df4b 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Typeahead.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Typeahead.qll @@ -126,11 +126,13 @@ module Typeahead { /** * A taint step that models that a function in the `source` of typeahead.js is used to determine the input to the suggestion function. */ - private class TypeaheadSourceTaintStep extends TypeaheadSource, TaintTracking::AdditionalTaintStep { + private class TypeaheadSourceTaintStep extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { // Matches `$(...).typeahead({..}, {source: function(q, cb) {..;cb();..}, templates: { suggestion: function() {} } })`. - pred = this.getAFunctionValue().getParameter([1 .. 2]).getACall().getAnArgument() and - succ = this.getASuggestion() + exists(TypeaheadSource source | + pred = source.getAFunctionValue().getParameter([1 .. 2]).getACall().getAnArgument() and + succ = source.getASuggestion() + ) } } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/UriLibraries.qll b/javascript/ql/src/semmle/javascript/frameworks/UriLibraries.qll index a18a43a308b..9d9ed43c509 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/UriLibraries.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/UriLibraries.qll @@ -4,10 +4,22 @@ import javascript +private newtype TUnit = TUnitInjector() + +private class UriLibraryStepGlue extends TaintTracking::AdditionalTaintStep { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + any(UriLibraryStep step).step(pred, succ) + } +} + /** * A taint propagating data flow edge arising from an operation in a URI library. */ -abstract class UriLibraryStep extends DataFlow::ValueNode, TaintTracking::AdditionalTaintStep { } +class UriLibraryStep extends TUnit { + abstract predicate step(DataFlow::Node pred, DataFlow::Node succ); + + string toString() { result = "Additional URI library taint step class" } +} /** * Provides classes for working with [urijs](http://medialize.github.io/URI.js/) code. @@ -57,25 +69,25 @@ module urijs { * A taint step in the urijs library. */ private class Step extends UriLibraryStep { - DataFlow::Node src; - - Step() { - // flow through "constructors" (`new` is optional) - exists(DataFlow::InvokeNode invk | invk = this and invk = invocation() | - src = invk.getAnArgument() + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + exists(DataFlow::ValueNode value | value = succ | + // flow through "constructors" (`new` is optional) + exists(DataFlow::InvokeNode invk | invk = value and invk = invocation() | + pred = invk.getAnArgument() + ) + or + // flow through chained calls + exists(DataFlow::MethodCallNode mc | mc = value and value = chainCall() | + pred = mc.getReceiver() or + pred = mc.getAnArgument() + ) + or + // flow through getter calls + exists(DataFlow::MethodCallNode mc | mc = value and value = getter() | + pred = mc.getReceiver() + ) ) - or - // flow through chained calls - exists(DataFlow::MethodCallNode mc | mc = this and this = chainCall() | - src = mc.getReceiver() or - src = mc.getAnArgument() - ) - or - // flow through getter calls - exists(DataFlow::MethodCallNode mc | mc = this and this = getter() | src = mc.getReceiver()) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this } } } @@ -93,22 +105,20 @@ module uridashjs { /** * A taint step in the urijs library. */ - private class Step extends UriLibraryStep, DataFlow::CallNode { - DataFlow::Node src; - - Step() { - exists(string name | + private class Step extends UriLibraryStep { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + exists(DataFlow::CallNode call, string name | name = "parse" or name = "serialize" or name = "resolve" or name = "normalize" | - this = uridashjsMember(name).getACall() and - src = getAnArgument() + call instanceof DataFlow::ValueNode and + call = succ and + call = uridashjsMember(name).getACall() and + pred = call.getAnArgument() ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this } } } @@ -126,22 +136,20 @@ module punycode { /** * A taint step in the punycode library. */ - private class Step extends UriLibraryStep, DataFlow::CallNode { - DataFlow::Node src; - - Step() { - exists(string name | + private class Step extends UriLibraryStep { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + exists(DataFlow::CallNode call, string name | name = "decode" or name = "encode" or name = "toUnicode" or name = "toASCII" | - this = punycodeMember(name).getACall() and - src = getAnArgument() + call instanceof DataFlow::ValueNode and + call = succ and + call = punycodeMember(name).getACall() and + pred = call.getAnArgument() ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this } } } @@ -162,24 +170,23 @@ module urlParse { /** * A taint step in the url-parse library. */ - private class Step extends UriLibraryStep, DataFlow::CallNode { - DataFlow::Node src; - - Step() { + private class Step extends UriLibraryStep { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { // parse(src) - this = call() and - src = getAnArgument() + succ = call() and + succ instanceof DataFlow::ValueNode and + pred = succ.(DataFlow::CallNode).getAnArgument() or - exists(DataFlow::MethodCallNode mc | this = mc and mc = call().getAMethodCall("set") | + exists(DataFlow::MethodCallNode mc | + mc instanceof DataFlow::ValueNode and succ = mc and mc = call().getAMethodCall("set") + | // src = parse(...); src.set(x, y) - src = mc.getReceiver() + pred = mc.getReceiver() or // parse(x).set(y, src) - src = mc.getArgument(1) + pred = mc.getArgument(1) ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this } } } @@ -197,20 +204,18 @@ module querystringify { /** * A taint step in the querystringify library. */ - private class Step extends UriLibraryStep, DataFlow::CallNode { - DataFlow::Node src; - - Step() { - exists(string name | + private class Step extends UriLibraryStep { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + exists(DataFlow::CallNode call, string name | name = "parse" or name = "stringify" | - this = querystringifyMember(name).getACall() and - src = getAnArgument() + call = querystringifyMember(name).getACall() and + pred = call.getAnArgument() and + succ = call and + call instanceof DataFlow::ValueNode ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this } } } @@ -228,22 +233,20 @@ module querydashstring { /** * A taint step in the query-string library. */ - private class Step extends UriLibraryStep, DataFlow::CallNode { - DataFlow::Node src; - - Step() { - exists(string name | + private class Step extends UriLibraryStep { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + exists(DataFlow::CallNode call, string name | name = "parse" or name = "extract" or name = "parseUrl" or name = "stringify" | - this = querydashstringMember(name).getACall() and - src = getAnArgument() + call = querydashstringMember(name).getACall() and + pred = call.getAnArgument() and + call instanceof DataFlow::ValueNode and + call = succ ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this } } } @@ -259,21 +262,19 @@ module url { /** * A taint step in the url library. */ - private class Step extends UriLibraryStep, DataFlow::CallNode { - DataFlow::Node src; - - Step() { - exists(string name | + private class Step extends UriLibraryStep { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + exists(DataFlow::CallNode call, string name | name = "parse" or name = "format" or name = "resolve" | - this = urlMember(name).getACall() and - src = getAnArgument() + call = urlMember(name).getACall() and + call instanceof DataFlow::ValueNode and + succ = call and + pred = call.getAnArgument() ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this } } } @@ -291,22 +292,20 @@ module querystring { /** * A taint step in the querystring library. */ - private class Step extends UriLibraryStep, DataFlow::CallNode { - DataFlow::Node src; - - Step() { - exists(string name | + private class Step extends UriLibraryStep { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + exists(DataFlow::CallNode call, string name | name = "escape" or name = "unescape" or name = "parse" or name = "stringify" | - this = querystringMember(name).getACall() and - src = getAnArgument() + call = querystringMember(name).getACall() and + call instanceof DataFlow::ValueNode and + call = succ and + pred = call.getAnArgument() ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this } } } @@ -317,56 +316,55 @@ private module ClosureLibraryUri { /** * Taint step from an argument of a `goog.Uri` call to the return value. */ - private class ArgumentStep extends UriLibraryStep, DataFlow::InvokeNode { - int arg; - - ArgumentStep() { - // goog.Uri constructor - this = Closure::moduleImport("goog.Uri").getAnInstantiation() and arg = 0 - or - // static methods on goog.Uri - exists(string name | this = Closure::moduleImport("goog.Uri." + name).getACall() | - name = "parse" and arg = 0 - or - name = "create" and - (arg = 0 or arg = 2 or arg = 4) - or - name = "resolve" and - (arg = 0 or arg = 1) - ) - or - // static methods in goog.uri.utils - arg = 0 and - exists(string name | this = Closure::moduleImport("goog.uri.utils." + name).getACall() | - name = "appendParam" or // preserve taint from the original URI, but not from the appended param - name = "appendParams" or - name = "appendParamsFromMap" or - name = "appendPath" or - name = "getParamValue" or - name = "getParamValues" or - name = "getPath" or - name = "getPathAndAfter" or - name = "getQueryData" or - name = "parseQueryData" or - name = "removeFragment" or - name = "removeParam" or - name = "setParam" or - name = "setParamsFromMap" or - name = "setPath" or - name = "split" - ) - or - // static methods in goog.string - arg = 0 and - exists(string name | this = Closure::moduleImport("goog.string." + name).getACall() | - name = "urlDecode" or - name = "urlEncode" - ) - } - + private class ArgumentStep extends UriLibraryStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getArgument(arg) and - succ = this + exists(DataFlow::InvokeNode invoke, int arg | + // goog.Uri constructor + invoke = Closure::moduleImport("goog.Uri").getAnInstantiation() and arg = 0 + or + // static methods on goog.Uri + exists(string name | invoke = Closure::moduleImport("goog.Uri." + name).getACall() | + name = "parse" and arg = 0 + or + name = "create" and + (arg = 0 or arg = 2 or arg = 4) + or + name = "resolve" and + (arg = 0 or arg = 1) + ) + or + // static methods in goog.uri.utils + arg = 0 and + exists(string name | invoke = Closure::moduleImport("goog.uri.utils." + name).getACall() | + name = "appendParam" or // preserve taint from the original URI, but not from the appended param + name = "appendParams" or + name = "appendParamsFromMap" or + name = "appendPath" or + name = "getParamValue" or + name = "getParamValues" or + name = "getPath" or + name = "getPathAndAfter" or + name = "getQueryData" or + name = "parseQueryData" or + name = "removeFragment" or + name = "removeParam" or + name = "setParam" or + name = "setParamsFromMap" or + name = "setPath" or + name = "split" + ) + or + // static methods in goog.string + arg = 0 and + exists(string name | invoke = Closure::moduleImport("goog.string." + name).getACall() | + name = "urlDecode" or + name = "urlEncode" + ) + | + pred = invoke.getArgument(arg) and + succ = invoke and + succ instanceof DataFlow::ValueNode + ) } } @@ -375,7 +373,7 @@ private module ClosureLibraryUri { * * Setters mutate the URI object and return the same instance. */ - private class SetterCall extends DataFlow::MethodCallNode, UriLibraryStep { + private class SetterCall extends DataFlow::MethodCallNode { DataFlow::NewNode uri; string name; @@ -391,14 +389,20 @@ private module ClosureLibraryUri { ) } - DataFlow::NewNode getUri() { result = uri } + string getName() { result = name } + DataFlow::NewNode getUri() { result = uri } + } + + private class SetterCallStep extends UriLibraryStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = getReceiver() and succ = this - or - (name = "setDomain" or name = "setPath" or name = "setScheme") and - pred = getArgument(0) and - succ = uri + exists(SetterCall setterCall | + pred = setterCall.getReceiver() and succ = setterCall + or + setterCall.getName() = ["setDomain", "setPath", "setScheme"] and + pred = setterCall.getArgument(0) and + succ = setterCall.getUri() + ) } } @@ -409,25 +413,21 @@ private module ClosureLibraryUri { /** * A taint step in the path module. */ - private class Step extends UriLibraryStep, DataFlow::CallNode { - DataFlow::Node src; - - Step() { - exists(DataFlow::SourceNode ref | + private class Step extends UriLibraryStep { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + exists(DataFlow::CallNode call, DataFlow::SourceNode ref | ref = NodeJSLib::Path::moduleMember("parse") or // a ponyfill: https://www.npmjs.com/package/path-parse ref = DataFlow::moduleImport("path-parse") or ref = DataFlow::moduleMember("path-parse", "posix") or ref = DataFlow::moduleMember("path-parse", "win32") | - this = ref.getACall() and - src = getAnArgument() + call = ref.getACall() and + call instanceof DataFlow::ValueNode and + call = succ and + pred = call.getAnArgument() ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = src and succ = this - } } } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/Vue.qll b/javascript/ql/src/semmle/javascript/frameworks/Vue.qll index 3424c24fee1..6577799e994 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Vue.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Vue.qll @@ -476,18 +476,14 @@ module Vue { * A taint propagating data flow edge through a Vue instance property. */ class InstanceHeapStep extends TaintTracking::AdditionalTaintStep { - DataFlow::Node src; - - InstanceHeapStep() { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { exists(Instance i, string name, DataFlow::FunctionNode bound | bound.flowsTo(i.getABoundFunction()) and not bound.getFunction() instanceof ArrowFunctionExpr and - bound.getReceiver().getAPropertyRead(name) = this and - src = i.getAPropertyValue(name) + bound.getReceiver().getAPropertyRead(name) = succ and + pred = i.getAPropertyValue(name) ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this } } /* diff --git a/javascript/ql/src/semmle/javascript/frameworks/XmlParsers.qll b/javascript/ql/src/semmle/javascript/frameworks/XmlParsers.qll index 80c2ff8a608..f2e2a8c50f4 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/XmlParsers.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/XmlParsers.qll @@ -277,13 +277,11 @@ module XML { } private class XMLParserTaintStep extends js::TaintTracking::AdditionalTaintStep { - XML::ParserInvocation parser; - - XMLParserTaintStep() { this.asExpr() = parser } - override predicate step(js::DataFlow::Node pred, js::DataFlow::Node succ) { - pred.asExpr() = parser.getSourceArgument() and - succ = parser.getAResult() + exists(XML::ParserInvocation parser | + pred.asExpr() = parser.getSourceArgument() and + succ = parser.getAResult() + ) } } } diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll index 095a2e072c1..29263c37204 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll @@ -656,7 +656,7 @@ module TaintedPath { or promiseTaintStep(src, dst) and srclabel = dstlabel or - any(TaintTracking::PersistentStorageTaintStep st).step(src, dst) and srclabel = dstlabel + TaintTracking::persistentStorageTaintStep(src, dst) and srclabel = dstlabel or exists(DataFlow::PropRead read | read = dst | src = read.getBase() and diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll b/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll index ea911f95300..4b26ccac14b 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll @@ -359,22 +359,17 @@ module DomBasedXss { * `div` element is part of the template for `inst`. */ class VHtmlSourceWrite extends TaintTracking::AdditionalTaintStep { - VHtmlSink attr; - - VHtmlSourceWrite() { - exists(Vue::Instance instance, string expr | + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + exists(VHtmlSink attr, Vue::Instance instance, string expr | attr.getAttr().getRoot() = instance.getTemplateElement().(Vue::Template::HtmlElement).getElement() and expr = attr.getAttr().getValue() and // only support for simple identifier expressions expr.regexpMatch("(?i)[a-z0-9_]+") and - this = instance.getAPropertyValue(expr) + pred = instance.getAPropertyValue(expr) and + succ = attr ) } - - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = this and succ = attr - } } /** diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query5.ql b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query5.ql index ee55f91d23e..b7955861a63 100644 --- a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query5.ql +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query5.ql @@ -1,11 +1,11 @@ import javascript -class StepThroughResolveSymlinks extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { - StepThroughResolveSymlinks() { this = DataFlow::moduleImport("resolve-symlinks").getACall() } - +class StepThroughResolveSymlinks extends TaintTracking::AdditionalTaintStep { override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - pred = this.getArgument(0) and - succ = this + exists(DataFlow::CallNode call | call = DataFlow::moduleImport("resolve-symlinks").getACall() | + pred = call.getArgument(0) and + succ = call + ) } }