diff --git a/javascript/ql/src/semmle/javascript/AMD.qll b/javascript/ql/src/semmle/javascript/AMD.qll index 0323154e1dc..70b4970960e 100644 --- a/javascript/ql/src/semmle/javascript/AMD.qll +++ b/javascript/ql/src/semmle/javascript/AMD.qll @@ -170,20 +170,19 @@ class AMDModuleDefinition extends CallExpr { } } -/** A path expression appearing in the list of dependencies of an AMD module. */ -private class AMDDependencyPath extends PathExprInModule, ConstantString { - AMDDependencyPath() { - exists(AMDModuleDefinition amd | this.getParentExpr*() = amd.getDependencies().getAnElement()) +/** An AMD dependency, considered as a path expression. */ +private class AmdDependencyPath extends PathExprCandidate { + AmdDependencyPath() { + exists(AMDModuleDefinition amd | + this = amd.getDependencies().getAnElement() or + this = amd.getARequireCall().getAnArgument() + ) } - - override string getValue() { result = this.(ConstantString).getStringValue() } } -/** A path expression appearing in a `require` call in an AMD module. */ -private class AMDRequirePath extends PathExprInModule, ConstantString { - AMDRequirePath() { - exists(AMDModuleDefinition amd | this.getParentExpr*() = amd.getARequireCall().getAnArgument()) - } +/** A constant path element appearing in an AMD dependency expression. */ +private class ConstantAmdDependencyPathElement extends PathExprInModule, ConstantString { + ConstantAmdDependencyPathElement() { this = any(AmdDependencyPath amd).getAPart() } override string getValue() { result = this.(ConstantString).getStringValue() } } diff --git a/javascript/ql/src/semmle/javascript/Closure.qll b/javascript/ql/src/semmle/javascript/Closure.qll index 937e70dd226..991a3568930 100644 --- a/javascript/ql/src/semmle/javascript/Closure.qll +++ b/javascript/ql/src/semmle/javascript/Closure.qll @@ -64,7 +64,7 @@ module Closure { * a top-level expression statement. */ private predicate isTopLevelExpr(DataFlow::Node node) { - node.getTopLevel().getAChildStmt().(ExprStmt).getExpr().flow() = node + any(TopLevel tl).getAChildStmt().(ExprStmt).getExpr().flow() = node } /** diff --git a/javascript/ql/src/semmle/javascript/NodeJS.qll b/javascript/ql/src/semmle/javascript/NodeJS.qll index c6da1a96c90..9d9bea4f096 100644 --- a/javascript/ql/src/semmle/javascript/NodeJS.qll +++ b/javascript/ql/src/semmle/javascript/NodeJS.qll @@ -239,22 +239,22 @@ class Require extends CallExpr, Import { } } -/** A literal path expression appearing in a `require` import. */ -private class LiteralRequiredPath extends PathExprInModule, ConstantString { - LiteralRequiredPath() { exists(Require req | this.getParentExpr*() = req.getArgument(0)) } - - override string getValue() { result = this.getStringValue() } -} - -/** A literal path expression appearing in a call to `require.resolve`. */ -private class LiteralRequireResolvePath extends PathExprInModule, ConstantString { - LiteralRequireResolvePath() { +/** An argument to `require` or `require.resolve`, considered as a path expression. */ +private class RequirePath extends PathExprCandidate { + RequirePath() { + this = any(Require req).getArgument(0) + or exists(RequireVariable req, MethodCallExpr reqres | reqres.getReceiver() = req.getAnAccess() and reqres.getMethodName() = "resolve" and - this.getParentExpr*() = reqres.getArgument(0) + this = reqres.getArgument(0) ) } +} + +/** A constant path element appearing in a call to `require` or `require.resolve`. */ +private class ConstantRequirePathElement extends PathExprInModule, ConstantString { + ConstantRequirePathElement() { this = any(RequirePath rp).getAPart() } override string getValue() { result = this.getStringValue() } } diff --git a/javascript/ql/src/semmle/javascript/Paths.qll b/javascript/ql/src/semmle/javascript/Paths.qll index 516f235dd06..9dceefcfdda 100644 --- a/javascript/ql/src/semmle/javascript/Paths.qll +++ b/javascript/ql/src/semmle/javascript/Paths.qll @@ -236,3 +236,20 @@ private class ConcatPath extends PathExpr { result = this.(AddExpr).getAnOperand().(PathExpr).getSearchRoot(priority) } } + +/** + * An expression that appears in a syntactic position where it may represent a path. + * + * Examples include arguments to the CommonJS `require` function or AMD dependency arguments. + */ +abstract class PathExprCandidate extends Expr { + /** + * Gets an expression that is nested inside this expression. + * + * Equivalent to `getAChildExpr*()`, but useful to enforce a better join order (in spite of + * what the optimizer thinks, there are generally far fewer `PathExprCandidate`s than + * `ConstantString`s). + */ + pragma[nomagic] + Expr getAPart() { result = this or result = getAPart().getAChildExpr() } +} diff --git a/javascript/ql/src/semmle/javascript/dataflow/Sources.qll b/javascript/ql/src/semmle/javascript/dataflow/Sources.qll index 436855732d2..d51c544217d 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/Sources.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/Sources.qll @@ -162,10 +162,11 @@ class SourceNode extends DataFlow::Node { * * See `TypeTracker` for more details about how to use this. */ + pragma[inline] DataFlow::SourceNode track(TypeTracker t2, TypeTracker t) { exists(StepSummary summary | StepSummary::step(this, result, summary) and - t = StepSummary::append(t2, summary) + t = t2.append(summary) ) } @@ -176,10 +177,11 @@ class SourceNode extends DataFlow::Node { * * See `TypeBackTracker` for more details about how to use this. */ + pragma[inline] DataFlow::SourceNode backtrack(TypeBackTracker t2, TypeBackTracker t) { exists(StepSummary summary | StepSummary::step(result, this, summary) and - t = StepSummary::prepend(summary, t2) + t = t2.prepend(summary) ) } } diff --git a/javascript/ql/src/semmle/javascript/dataflow/TypeTracking.qll b/javascript/ql/src/semmle/javascript/dataflow/TypeTracking.qll index f1b2dce3eeb..2164426f6af 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/TypeTracking.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/TypeTracking.qll @@ -9,133 +9,87 @@ import javascript private import internal.FlowSteps -/** - * A pair of booleans, indicating whether a path goes through a return and/or a call. - * - * Identical to `TPathSummary` except without flow labels. - */ -private newtype TStepSummary = MkStepSummary(boolean hasReturn, boolean hasCall) { - (hasReturn = true or hasReturn = false) and - (hasCall = true or hasCall = false) +private class PropertyName extends string { + PropertyName() { this = any(DataFlow::PropRef pr).getPropertyName() } } +private class OptionalPropertyName extends string { + OptionalPropertyName() { this instanceof PropertyName or this = "" } +} + +/** + * A description of a step on an inter-procedural data flow path. + */ +private newtype TStepSummary = + LevelStep() or + CallStep() or + ReturnStep() or + StoreStep(PropertyName prop) or + LoadStep(PropertyName prop) + /** * INTERNAL: Use `TypeTracker` or `TypeBackTracker` instead. * - * Summary of the steps needed to track a value to a given dataflow node. + * A description of a step on an inter-procedural data flow path. */ class StepSummary extends TStepSummary { - Boolean hasReturn; - - Boolean hasCall; - - StepSummary() { this = MkStepSummary(hasReturn, hasCall) } - - /** Indicates whether the path represented by this summary contains any return steps. */ - boolean hasReturn() { result = hasReturn } - - /** Indicates whether the path represented by this summary contains any call steps. */ - boolean hasCall() { result = hasCall } - - /** - * Gets the summary for the path obtained by appending `that` to `this`. - * - * Note that a path containing a `return` step cannot be appended to a path containing - * a `call` step in order to maintain well-formedness. - */ - StepSummary append(StepSummary that) { - exists(Boolean hasReturn2, Boolean hasCall2 | - that = MkStepSummary(hasReturn2, hasCall2) - | - result = MkStepSummary(hasReturn.booleanOr(hasReturn2), hasCall.booleanOr(hasCall2)) and - // avoid constructing invalid paths - not (hasCall = true and hasReturn2 = true) - ) - } - - /** - * Gets the summary for the path obtained by appending `this` to `that`. - */ - StepSummary prepend(StepSummary that) { result = that.append(this) } - - /** Gets a textual representation of this path summary. */ + /** Gets a textual representation of this step summary. */ string toString() { - exists(string withReturn, string withCall | - (if hasReturn = true then withReturn = "with" else withReturn = "without") and - (if hasCall = true then withCall = "with" else withCall = "without") - | - result = "path " + withReturn + " return steps and " + withCall + " call steps" + this instanceof LevelStep and result = "level" + or + this instanceof CallStep and result = "call" + or + this instanceof ReturnStep and result = "return" + or + exists(string prop | this = StoreStep(prop) | + result = "store " + prop + ) + or + exists(string prop | this = LoadStep(prop) | + result = "load" + prop ) } } module StepSummary { - /** - * Gets a summary describing a path without any calls or returns. - */ - StepSummary level() { result = MkStepSummary(false, false) } - - /** - * Gets a summary describing a path with one or more calls, but no returns. - */ - StepSummary call() { result = MkStepSummary(false, true) } - - /** - * Gets a summary describing a path with one or more returns, but no calls. - */ - StepSummary return() { result = MkStepSummary(true, false) } - /** * INTERNAL: Use `SourceNode.track()` or `SourceNode.backtrack()` instead. */ predicate step(DataFlow::SourceNode pred, DataFlow::SourceNode succ, StepSummary summary) { - exists (DataFlow::Node predNode | pred.flowsTo(predNode) | + exists(DataFlow::Node predNode | pred.flowsTo(predNode) | // Flow through properties of objects propertyFlowStep(predNode, succ) and - summary = level() + summary = LevelStep() or // Flow through global variables globalFlowStep(predNode, succ) and - summary = level() + summary = LevelStep() or // Flow into function callStep(predNode, succ) and - summary = call() + summary = CallStep() or // Flow out of function returnStep(predNode, succ) and - summary = return() + summary = ReturnStep() or // Flow through an instance field between members of the same class DataFlow::localFieldStep(predNode, succ) and - summary = level() + summary = LevelStep() + or + exists(string prop | + basicStoreStep(predNode, succ, prop) and + summary = StoreStep(prop) + or + loadStep(predNode, succ, prop) and + summary = LoadStep(prop) + ) ) } - - /** - * INTERNAL. Do not use. - * - * Appends a step summary onto a type-tracking summary. - */ - TypeTracker append(TypeTracker type, StepSummary summary) { - not (type.hasCall() = true and summary.hasReturn() = true) and - result.hasCall() = type.hasCall().booleanOr(summary.hasCall()) - } - - /** - * INTERNAL. Do not use. - * - * Prepends a step summary before a backwards type-tracking summary. - */ - TypeBackTracker prepend(StepSummary summary, TypeBackTracker type) { - not (type.hasReturn() = true and summary.hasCall() = true) and - result.hasReturn() = type.hasReturn().booleanOr(summary.hasReturn()) - } } -private newtype TTypeTracker = MkTypeTracker(boolean hasCall) { - hasCall = true or hasCall = false -} +private newtype TTypeTracker = + MkTypeTracker(Boolean hasCall, OptionalPropertyName prop) /** * EXPERIMENTAL. @@ -159,7 +113,7 @@ private newtype TTypeTracker = MkTypeTracker(boolean hasCall) { * ) * } * - * DataFlow::SourceNode myType() { result = myType(_) } + * DataFlow::SourceNode myType() { result = myType(DataFlow::TypeTracker::end()) } * ``` * * To track values backwards, which can be useful for tracking @@ -168,35 +122,56 @@ private newtype TTypeTracker = MkTypeTracker(boolean hasCall) { class TypeTracker extends TTypeTracker { Boolean hasCall; - TypeTracker() { this = MkTypeTracker(hasCall) } + string prop; + + TypeTracker() { this = MkTypeTracker(hasCall, prop) } + + TypeTracker append(StepSummary step) { + step = LevelStep() and result = this + or + step = CallStep() and result = MkTypeTracker(true, prop) + or + step = ReturnStep() and hasCall = false and result = this + or + step = LoadStep(prop) and result = MkTypeTracker(hasCall, "") + or + exists(string p | + step = StoreStep(p) and prop = "" and result = MkTypeTracker(hasCall, p) + ) + } string toString() { - hasCall = true and result = "type tracker with call steps" - or - hasCall = false and result = "type tracker without call steps" + exists(string withCall, string withProp | + (if hasCall = true then withCall = "with" else withCall = "without") and + (if prop != "" then withProp = " with property " + prop else withProp = "") and + result = "type tracker " + withCall + " call steps" + withProp + ) } /** * Holds if this is the starting point of type tracking. */ - predicate start() { - hasCall = false - } + predicate start() { hasCall = false and prop = "" } + + predicate end() { prop = "" } /** * INTERNAL. DO NOT USE. * * Holds if this type has been tracked into a call. */ - boolean hasCall() { - result = hasCall - } + boolean hasCall() { result = hasCall } + + string getProp() { result = prop } } -private newtype TTypeBackTracker = MkTypeBackTracker(boolean hasReturn) { - hasReturn = true or hasReturn = false +module TypeTracker { + TypeTracker end() { result.end() } } +private newtype TTypeBackTracker = + MkTypeBackTracker(Boolean hasReturn, OptionalPropertyName prop) + /** * EXPERIMENTAL. * @@ -221,33 +196,55 @@ private newtype TTypeBackTracker = MkTypeBackTracker(boolean hasReturn) { * ) * } * - * DataFlow::SourceNode myCallback() { result = myCallback(_) } + * DataFlow::SourceNode myCallback() { result = myCallback(DataFlow::TypeBackTracker::end()) } * ``` */ class TypeBackTracker extends TTypeBackTracker { Boolean hasReturn; - TypeBackTracker() { this = MkTypeBackTracker(hasReturn) } + string prop; + + TypeBackTracker() { this = MkTypeBackTracker(hasReturn, prop) } + + TypeBackTracker prepend(StepSummary step) { + step = LevelStep() and result = this + or + step = CallStep() and hasReturn = false and result = this + or + step = ReturnStep() and result = MkTypeBackTracker(true, prop) + or + exists(string p | + step = LoadStep(p) and prop = "" and result = MkTypeBackTracker(hasReturn, p) + ) + or + step = StoreStep(prop) and result = MkTypeBackTracker(hasReturn, "") + } string toString() { - hasReturn = true and result = "type back-tracker with return steps" - or - hasReturn = false and result = "type back-tracker without return steps" + exists(string withReturn, string withProp | + (if hasReturn = true then withReturn = "with" else withReturn = "without") and + (if prop != "" then withProp = " with property " + prop else withProp = "") and + result = "type back-tracker " + withReturn + " return steps" + withProp + ) } /** * Holds if this is the starting point of type tracking. */ - predicate start() { - hasReturn = false - } + predicate start() { hasReturn = false and prop = "" } + + predicate end() { prop = "" } /** * INTERNAL. DO NOT USE. * * Holds if this type has been back-tracked into a call through return edge. */ - boolean hasReturn() { - result = hasReturn - } + boolean hasReturn() { result = hasReturn } + + string getProp() { result = prop } +} + +module TypeBackTracker { + TypeBackTracker end() { result.end() } } diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll index 2559d6fb656..65c08b24ea4 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll @@ -17,36 +17,6 @@ predicate shouldTrackProperties(AbstractValue obj) { obj instanceof AbstractModuleObject } -/** - * Holds if `invk` may invoke `f`. - */ -predicate calls(DataFlow::InvokeNode invk, Function f) { f = invk.getACallee(0) } - -/** - * Holds if `invk` may invoke `f` indirectly through the given `callback` argument. - * - * This only holds for explicitly modeled partial calls. - */ -private predicate partiallyCalls( - DataFlow::AdditionalPartialInvokeNode invk, DataFlow::AnalyzedNode callback, Function f -) { - invk.isPartialArgument(callback, _, _) and - exists(AbstractFunction callee | callee = callback.getAValue() | - if callback.getAValue().isIndefinite("global") - then f = callee.getFunction() and f.getFile() = invk.getFile() - else f = callee.getFunction() - ) -} - -/** - * Holds if `f` captures the variable defined by `def` in `cap`. - */ -cached -predicate captures(Function f, SsaExplicitDefinition def, SsaVariableCapture cap) { - def.getSourceVariable() = cap.getSourceVariable() and - f = cap.getContainer() -} - /** * Holds if `source` corresponds to an expression returned by `f`, and * `sink` equals `source`. @@ -84,206 +54,255 @@ predicate localFlowStep( } /** - * Holds if `arg` is passed as an argument into parameter `parm` - * through invocation `invk` of function `f`. + * Implements a set of data flow predicates that are used by multiple predicates and + * hence should only be computed once. */ -predicate argumentPassing( - DataFlow::InvokeNode invk, DataFlow::ValueNode arg, Function f, DataFlow::ParameterNode parm -) { - calls(invk, f) and - exists(int i | - f.getParameter(i) = parm.getParameter() and - not parm.isRestParameter() and - arg = invk.getArgument(i) - ) - or - exists(DataFlow::Node callback, int i | - invk.(DataFlow::AdditionalPartialInvokeNode).isPartialArgument(callback, arg, i) and - partiallyCalls(invk, callback, f) and - parm.getParameter() = f.getParameter(i) and - not parm.isRestParameter() - ) -} +cached +private module CachedSteps { + /** + * Holds if `f` captures the variable defined by `def` in `cap`. + */ + cached + predicate captures(Function f, SsaExplicitDefinition def, SsaVariableCapture cap) { + def.getSourceVariable() = cap.getSourceVariable() and + f = cap.getContainer() + } -/** - * Holds if there is a flow step from `pred` to `succ` through parameter passing - * to a function call. - */ -predicate callStep(DataFlow::Node pred, DataFlow::Node succ) { argumentPassing(_, pred, _, succ) } + /** + * Holds if `invk` may invoke `f`. + */ + cached + predicate calls(DataFlow::InvokeNode invk, Function f) { f = invk.getACallee(0) } -/** - * Holds if there is a flow step from `pred` to `succ` through returning - * from a function call or the receiver flowing out of a constructor call. - */ -predicate returnStep(DataFlow::Node pred, DataFlow::Node succ) { - exists(Function f | calls(succ, f) | - returnExpr(f, pred, _) + /** + * Holds if `invk` may invoke `f` indirectly through the given `callback` argument. + * + * This only holds for explicitly modeled partial calls. + */ + private predicate partiallyCalls( + DataFlow::AdditionalPartialInvokeNode invk, DataFlow::AnalyzedNode callback, Function f + ) { + invk.isPartialArgument(callback, _, _) and + exists(AbstractFunction callee | callee = callback.getAValue() | + if callback.getAValue().isIndefinite("global") + then f = callee.getFunction() and f.getFile() = invk.getFile() + else f = callee.getFunction() + ) + } + + /** + * Holds if `arg` is passed as an argument into parameter `parm` + * through invocation `invk` of function `f`. + */ + cached + predicate argumentPassing( + DataFlow::InvokeNode invk, DataFlow::ValueNode arg, Function f, DataFlow::ParameterNode parm + ) { + calls(invk, f) and + exists(int i | + f.getParameter(i) = parm.getParameter() and + not parm.isRestParameter() and + arg = invk.getArgument(i) + ) or - succ instanceof DataFlow::NewNode and - DataFlow::thisNode(pred, f) - ) -} + exists(DataFlow::Node callback, int i | + invk.(DataFlow::AdditionalPartialInvokeNode).isPartialArgument(callback, arg, i) and + partiallyCalls(invk, callback, f) and + parm.getParameter() = f.getParameter(i) and + not parm.isRestParameter() + ) + } -/** - * Holds if there is an assignment to property `prop` of an object represented by `obj` - * with right hand side `rhs` somewhere, and properties of `obj` should be tracked. - */ -pragma[noinline] -private predicate trackedPropertyWrite(AbstractValue obj, string prop, DataFlow::Node rhs) { - exists(AnalyzedPropertyWrite pw | - pw.writes(obj, prop, rhs) and - shouldTrackProperties(obj) and - // avoid introducing spurious global flow - not pw.baseIsIncomplete("global") - ) -} + /** + * Holds if there is a flow step from `pred` to `succ` through parameter passing + * to a function call. + */ + cached + predicate callStep(DataFlow::Node pred, DataFlow::Node succ) { argumentPassing(_, pred, _, succ) } -/** - * Holds if there is a flow step from `pred` to `succ` through an object property. - */ -predicate propertyFlowStep(DataFlow::Node pred, DataFlow::Node succ) { - exists(AbstractValue obj, string prop | - trackedPropertyWrite(obj, prop, pred) and - succ.(AnalyzedPropertyRead).reads(obj, prop) - ) -} + /** + * Holds if there is a flow step from `pred` to `succ` through returning + * from a function call or the receiver flowing out of a constructor call. + */ + cached + predicate returnStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(Function f | calls(succ, f) | + returnExpr(f, pred, _) + or + succ instanceof DataFlow::NewNode and + DataFlow::thisNode(pred, f) + ) + } -/** - * Gets a node whose value is assigned to `gv` in `f`. - */ -pragma[noinline] -private DataFlow::ValueNode getADefIn(GlobalVariable gv, File f) { - exists(VarDef def | - def.getFile() = f and - def.getTarget() = gv.getAReference() and - result = DataFlow::valueNode(def.getSource()) - ) -} + /** + * Holds if there is an assignment to property `prop` of an object represented by `obj` + * with right hand side `rhs` somewhere, and properties of `obj` should be tracked. + */ + pragma[noinline] + private predicate trackedPropertyWrite(AbstractValue obj, string prop, DataFlow::Node rhs) { + exists(AnalyzedPropertyWrite pw | + pw.writes(obj, prop, rhs) and + shouldTrackProperties(obj) and + // avoid introducing spurious global flow + not pw.baseIsIncomplete("global") + ) + } -/** - * Gets a use of `gv` in `f`. - */ -pragma[noinline] -DataFlow::ValueNode getAUseIn(GlobalVariable gv, File f) { - result.getFile() = f and - result = DataFlow::valueNode(gv.getAnAccess()) -} + /** + * Holds if there is a flow step from `pred` to `succ` through an object property. + */ + cached + predicate propertyFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(AbstractValue obj, string prop | + trackedPropertyWrite(obj, prop, pred) and + succ.(AnalyzedPropertyRead).reads(obj, prop) + ) + } -/** - * Holds if there is a flow step from `pred` to `succ` through a global - * variable. Both `pred` and `succ` must be in the same file. - */ -predicate globalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { - exists(GlobalVariable gv, File f | - pred = getADefIn(gv, f) and - succ = getAUseIn(gv, f) - ) -} + /** + * Gets a node whose value is assigned to `gv` in `f`. + */ + pragma[noinline] + private DataFlow::ValueNode getADefIn(GlobalVariable gv, File f) { + exists(VarDef def | + def.getFile() = f and + def.getTarget() = gv.getAReference() and + result = DataFlow::valueNode(def.getSource()) + ) + } -/** - * Holds if there is a write to property `prop` of global variable `gv` - * in file `f`, where the right-hand side of the write is `rhs`. - */ -pragma[noinline] -predicate globalPropertyWrite(GlobalVariable gv, File f, string prop, DataFlow::Node rhs) { - exists(DataFlow::PropWrite pw | pw.writes(getAUseIn(gv, f), prop, rhs)) -} + /** + * Gets a use of `gv` in `f`. + */ + pragma[noinline] + private DataFlow::ValueNode getAUseIn(GlobalVariable gv, File f) { + result.getFile() = f and + result = DataFlow::valueNode(gv.getAnAccess()) + } -/** - * Holds if there is a read from property `prop` of `base`, which is - * an access to global variable `base` in file `f`. - */ -pragma[noinline] -predicate globalPropertyRead(GlobalVariable gv, File f, string prop, DataFlow::Node base) { - exists(DataFlow::PropRead pr | - base = getAUseIn(gv, f) and - pr.accesses(base, prop) - ) -} + /** + * Holds if there is a flow step from `pred` to `succ` through a global + * variable. Both `pred` and `succ` must be in the same file. + */ + cached + predicate globalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(GlobalVariable gv, File f | + pred = getADefIn(gv, f) and + succ = getAUseIn(gv, f) + ) + } -/** - * Holds if there is a store step from `pred` to `succ` under property `prop`, - * that is, `succ` is the local source of the base of a write of property - * `prop` with right-hand side `pred`. - * - * For example, for this code snippet: - * - * ``` - * var a = new A(); - * a.p = e; - * ``` - * - * there is a store step from `e` to `new A()` under property `prop`. - * - * As a special case, if the base of the property write is a global variable, - * then there is a store step from the right-hand side of the write to any - * read of the same property from the same global variable in the same file. - */ -predicate basicStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) { - succ.(DataFlow::SourceNode).hasPropertyWrite(prop, pred) - or - exists(GlobalVariable gv, File f | - globalPropertyWrite(gv, f, prop, pred) and - globalPropertyRead(gv, f, prop, succ) - ) -} + /** + * Holds if there is a write to property `prop` of global variable `gv` + * in file `f`, where the right-hand side of the write is `rhs`. + */ + pragma[noinline] + private predicate globalPropertyWrite(GlobalVariable gv, File f, string prop, DataFlow::Node rhs) { + exists(DataFlow::PropWrite pw | pw.writes(getAUseIn(gv, f), prop, rhs)) + } -/** - * Holds if there is a load step from `pred` to `succ` under property `prop`, - * that is, `succ` is a read of property `prop` from `pred`. - */ -predicate loadStep(DataFlow::Node pred, DataFlow::PropRead succ, string prop) { - succ.accesses(pred, prop) -} + /** + * Holds if there is a read from property `prop` of `base`, which is + * an access to global variable `base` in file `f`. + */ + pragma[noinline] + private predicate globalPropertyRead(GlobalVariable gv, File f, string prop, DataFlow::Node base) { + exists(DataFlow::PropRead pr | + base = getAUseIn(gv, f) and + pr.accesses(base, prop) + ) + } -/** - * Holds if there is a higher-order call with argument `arg`, and `cb` is the local - * source of an argument that flows into the callee position of that call: - * - * ``` - * function f(x, g) { - * g( - * x // arg - * ); - * } - * - * function cb() { // cb - * } - * - * f(arg, cb); - * - * This is an over-approximation of a possible data flow step through a callback - * invocation. - */ -predicate callback(DataFlow::Node arg, DataFlow::SourceNode cb) { - exists(DataFlow::InvokeNode invk, DataFlow::ParameterNode cbParm, DataFlow::Node cbArg | - arg = invk.getAnArgument() and - cbParm.flowsTo(invk.getCalleeNode()) and - callStep(cbArg, cbParm) and - cb.flowsTo(cbArg) - ) - or - exists(DataFlow::ParameterNode cbParm, DataFlow::Node cbArg | - callback(arg, cbParm) and - callStep(cbArg, cbParm) and - cb.flowsTo(cbArg) - ) -} + /** + * Holds if there is a store step from `pred` to `succ` under property `prop`, + * that is, `succ` is the local source of the base of a write of property + * `prop` with right-hand side `pred`. + * + * For example, for this code snippet: + * + * ``` + * var a = new A(); + * a.p = e; + * ``` + * + * there is a store step from `e` to `new A()` under property `prop`. + * + * As a special case, if the base of the property write is a global variable, + * then there is a store step from the right-hand side of the write to any + * read of the same property from the same global variable in the same file. + */ + cached + predicate basicStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) { + succ.(DataFlow::SourceNode).hasPropertyWrite(prop, pred) + or + exists(GlobalVariable gv, File f | + globalPropertyWrite(gv, f, prop, pred) and + globalPropertyRead(gv, f, prop, succ) + ) + } -/** - * Holds if `f` may return `base`, which has a write of property `prop` with right-hand side `rhs`. - */ -predicate returnedPropWrite(Function f, DataFlow::SourceNode base, string prop, DataFlow::Node rhs) { - base.hasPropertyWrite(prop, rhs) and - base.flowsToExpr(f.getAReturnedExpr()) -} + /** + * Holds if there is a load step from `pred` to `succ` under property `prop`, + * that is, `succ` is a read of property `prop` from `pred`. + */ + cached + predicate loadStep(DataFlow::Node pred, DataFlow::PropRead succ, string prop) { + succ.accesses(pred, prop) + } -/** - * Holds if `f` may assign `rhs` to `this.prop`. - */ -predicate receiverPropWrite(Function f, string prop, DataFlow::Node rhs) { - DataFlow::thisNode(f).hasPropertyWrite(prop, rhs) + /** + * Holds if there is a higher-order call with argument `arg`, and `cb` is the local + * source of an argument that flows into the callee position of that call: + * + * ``` + * function f(x, g) { + * g( + * x // arg + * ); + * } + * + * function cb() { // cb + * } + * + * f(arg, cb); + * + * This is an over-approximation of a possible data flow step through a callback + * invocation. + */ + cached + predicate callback(DataFlow::Node arg, DataFlow::SourceNode cb) { + exists(DataFlow::InvokeNode invk, DataFlow::ParameterNode cbParm, DataFlow::Node cbArg | + arg = invk.getAnArgument() and + cbParm.flowsTo(invk.getCalleeNode()) and + callStep(cbArg, cbParm) and + cb.flowsTo(cbArg) + ) + or + exists(DataFlow::ParameterNode cbParm, DataFlow::Node cbArg | + callback(arg, cbParm) and + callStep(cbArg, cbParm) and + cb.flowsTo(cbArg) + ) + } + + /** + * Holds if `f` may return `base`, which has a write of property `prop` with right-hand side `rhs`. + */ + cached + predicate returnedPropWrite(Function f, DataFlow::SourceNode base, string prop, DataFlow::Node rhs) { + base.hasPropertyWrite(prop, rhs) and + base.flowsToExpr(f.getAReturnedExpr()) + } + + /** + * Holds if `f` may assign `rhs` to `this.prop`. + */ + cached + predicate receiverPropWrite(Function f, string prop, DataFlow::Node rhs) { + DataFlow::thisNode(f).hasPropertyWrite(prop, rhs) + } } +import CachedSteps /** * A utility class that is equivalent to `boolean` but does not require type joining. diff --git a/javascript/ql/src/semmle/javascript/frameworks/Connect.qll b/javascript/ql/src/semmle/javascript/frameworks/Connect.qll index 3dcac8ab62b..dd5da051650 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Connect.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Connect.qll @@ -118,8 +118,16 @@ module Connect { } override DataFlow::SourceNode getARouteHandler() { - result.(DataFlow::SourceNode).flowsTo(getARouteHandlerExpr().flow()) or - result.(DataFlow::TrackedNode).flowsTo(getARouteHandlerExpr().flow()) + result = getARouteHandler(DataFlow::TypeBackTracker::end()) + } + + private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) { + t.start() and + result = getARouteHandlerExpr().flow().getALocalSource() + or + exists(DataFlow::TypeBackTracker t2 | + result = getARouteHandler(t2).backtrack(t2, t) + ) } override Expr getServer() { result = server } @@ -169,24 +177,13 @@ module Connect { } /** - * Tracking for `RouteHandlerCandidate`. - */ - private class TrackedRouteHandlerCandidate extends DataFlow::TrackedNode { - TrackedRouteHandlerCandidate() { this instanceof RouteHandlerCandidate } - } - - /** - * A function that looks like a Connect route handler and flows to a route setup. + * A function that flows to a route setup. */ private class TrackedRouteHandlerCandidateWithSetup extends RouteHandler, - HTTP::Servers::StandardRouteHandler, DataFlow::ValueNode { - override Function astNode; + HTTP::Servers::StandardRouteHandler, DataFlow::FunctionNode { TrackedRouteHandlerCandidateWithSetup() { - exists(TrackedRouteHandlerCandidate tracked | - tracked.flowsTo(any(RouteSetup s).getARouteHandlerExpr().flow()) and - this = tracked - ) + this = any(RouteSetup s).getARouteHandler() } override SimpleParameter getRouteHandlerParameter(string kind) { diff --git a/javascript/ql/src/semmle/javascript/frameworks/Electron.qll b/javascript/ql/src/semmle/javascript/frameworks/Electron.qll index 744648469fe..d050338f2d8 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Electron.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Electron.qll @@ -16,7 +16,7 @@ module Electron { /** * An instantiation of `BrowserWindow` or `BrowserView`. */ - abstract private class NewBrowserObject extends BrowserObject, DataFlow::SourceNode { + abstract private class NewBrowserObject extends BrowserObject { DataFlow::NewNode self; NewBrowserObject() { this = self } @@ -56,11 +56,20 @@ module Electron { } } + private DataFlow::SourceNode browserObject(DataFlow::TypeTracker t) { + t.start() and + result instanceof NewBrowserObject + or + exists(DataFlow::TypeTracker t2 | + result = browserObject(t2).track(t2, t) + ) + } + /** * A data flow node whose value may originate from a browser object instantiation. */ private class BrowserObjectByFlow extends BrowserObject { - BrowserObjectByFlow() { any(NewBrowserObject nbo).flowsTo(this) } + BrowserObjectByFlow() { browserObject(DataFlow::TypeTracker::end()).flowsTo(this) } } /** diff --git a/javascript/ql/src/semmle/javascript/frameworks/Express.qll b/javascript/ql/src/semmle/javascript/frameworks/Express.qll index b6f97f4dde4..9bf2399e2ed 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Express.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Express.qll @@ -111,8 +111,16 @@ module Express { Expr getLastRouteHandlerExpr() { result = max(int i | | getRouteHandlerExpr(i) order by i) } override DataFlow::SourceNode getARouteHandler() { - result.(DataFlow::SourceNode).flowsTo(getARouteHandlerExpr().flow()) or - result.(DataFlow::TrackedNode).flowsTo(getARouteHandlerExpr().flow()) + result = getARouteHandler(DataFlow::TypeBackTracker::end()) + } + + private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) { + t.start() and + result = getARouteHandlerExpr().flow().getALocalSource() + or + exists(DataFlow::TypeBackTracker t2 | + result = getARouteHandler(t2).backtrack(t2, t) + ) } override Expr getServer() { result.(Application).getARouteHandler() = getARouteHandler() } @@ -766,24 +774,13 @@ module Express { } /** - * Tracking for `RouteHandlerCandidate`. - */ - private class TrackedRouteHandlerCandidate extends DataFlow::TrackedNode { - TrackedRouteHandlerCandidate() { this instanceof RouteHandlerCandidate } - } - - /** - * A function that looks like an Express route handler and flows to a route setup. + * A function that flows to a route setup. */ private class TrackedRouteHandlerCandidateWithSetup extends RouteHandler, - HTTP::Servers::StandardRouteHandler, DataFlow::ValueNode { - override Function astNode; + HTTP::Servers::StandardRouteHandler, DataFlow::FunctionNode { TrackedRouteHandlerCandidateWithSetup() { - exists(TrackedRouteHandlerCandidate tracked | - tracked.flowsTo(any(RouteSetup s).getARouteHandlerExpr().flow()) and - this = tracked - ) + this = any(RouteSetup s).getARouteHandler() } override SimpleParameter getRouteHandlerParameter(string kind) { diff --git a/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll b/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll index a9c7813f2da..17a41c8ba3d 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll @@ -276,11 +276,20 @@ module HTTP { * a request object enters the flow graph, such as the request * parameter of a route handler. */ - abstract class RequestSource extends DataFlow::TrackedNode { + abstract class RequestSource extends DataFlow::Node { /** * Gets the route handler that handles this request. */ abstract RouteHandler getRouteHandler(); + + predicate flowsTo(DataFlow::Node nd) { ref(DataFlow::TypeTracker::end()).flowsTo(nd) } + + private DataFlow::SourceNode ref(DataFlow::TypeTracker t) { + t.start() and + result = this + or + exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t)) + } } /** @@ -288,11 +297,20 @@ module HTTP { * a response object enters the flow graph, such as the response * parameter of a route handler. */ - abstract class ResponseSource extends DataFlow::TrackedNode { + abstract class ResponseSource extends DataFlow::Node { /** * Gets the route handler that provides this response. */ abstract RouteHandler getRouteHandler(); + + predicate flowsTo(DataFlow::Node nd) { ref(DataFlow::TypeTracker::end()).flowsTo(nd) } + + private DataFlow::SourceNode ref(DataFlow::TypeTracker t) { + t.start() and + result = this + or + exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t)) + } } /** diff --git a/javascript/ql/src/semmle/javascript/frameworks/Hapi.qll b/javascript/ql/src/semmle/javascript/frameworks/Hapi.qll index bf1ccc8b250..2ace343d6ff 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Hapi.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Hapi.qll @@ -193,8 +193,16 @@ module Hapi { } override DataFlow::SourceNode getARouteHandler() { - result.(DataFlow::SourceNode).flowsTo(handler.flow()) or - result.(DataFlow::TrackedNode).flowsTo(handler.flow()) + result = getARouteHandler(DataFlow::TypeBackTracker::end()) + } + + private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) { + t.start() and + result = handler.flow().getALocalSource() + or + exists(DataFlow::TypeBackTracker t2 | + result = getARouteHandler(t2).backtrack(t2, t) + ) } Expr getRouteHandlerExpr() { result = handler } @@ -223,25 +231,13 @@ module Hapi { } } - /** - * Tracking for `RouteHandlerCandidate`. - */ - private class TrackedRouteHandlerCandidate extends DataFlow::TrackedNode { - TrackedRouteHandlerCandidate() { this instanceof RouteHandlerCandidate } - } - /** * A function that looks like a Hapi route handler and flows to a route setup. */ private class TrackedRouteHandlerCandidateWithSetup extends RouteHandler, - HTTP::Servers::StandardRouteHandler, DataFlow::ValueNode { - override Function astNode; - + HTTP::Servers::StandardRouteHandler, DataFlow::FunctionNode { TrackedRouteHandlerCandidateWithSetup() { - exists(TrackedRouteHandlerCandidate tracked | - tracked.flowsTo(any(RouteSetup s).getRouteHandlerExpr().flow()) and - this = tracked - ) + this = any(RouteSetup s).getARouteHandler() } } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/Koa.qll b/javascript/ql/src/semmle/javascript/frameworks/Koa.qll index 98f7856d1d0..20d80059ff7 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Koa.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Koa.qll @@ -64,7 +64,7 @@ module Koa { * A Koa context source, that is, the context parameter of a * route handler, or a `this` access in a route handler. */ - private class ContextSource extends DataFlow::TrackedNode { + private class ContextSource extends DataFlow::Node { RouteHandler rh; ContextSource() { @@ -77,6 +77,19 @@ module Koa { * Gets the route handler that handles this request. */ RouteHandler getRouteHandler() { result = rh } + + predicate flowsTo(DataFlow::Node nd) { + ref(DataFlow::TypeTracker::end()).flowsTo(nd) + } + + private DataFlow::SourceNode ref(DataFlow::TypeTracker t) { + t.start() and + result = this + or + exists(DataFlow::TypeTracker t2 | + result = ref(t2).track(t2, t) + ) + } } /** diff --git a/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll b/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll index 5959eb261bf..01528080f46 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll @@ -189,8 +189,16 @@ module NodeJSLib { } override DataFlow::SourceNode getARouteHandler() { - result.(DataFlow::SourceNode).flowsTo(handler.flow()) or - result.(DataFlow::TrackedNode).flowsTo(handler.flow()) + result = getARouteHandler(DataFlow::TypeBackTracker::end()) + } + + private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) { + t.start() and + result = handler.flow().getALocalSource() + or + exists(DataFlow::TypeBackTracker t2 | + result = getARouteHandler(t2).backtrack(t2, t) + ) } override Expr getServer() { result = server } @@ -613,24 +621,12 @@ module NodeJSLib { } /** - * Tracking for `RouteHandlerCandidate`. - */ - private class TrackedRouteHandlerCandidate extends DataFlow::TrackedNode { - TrackedRouteHandlerCandidate() { this instanceof RouteHandlerCandidate } - } - - /** - * A function that looks like a Node.js route handler and flows to a route setup. + * A function that flows to a route setup. */ private class TrackedRouteHandlerCandidateWithSetup extends RouteHandler, - HTTP::Servers::StandardRouteHandler, DataFlow::ValueNode { - override Function astNode; - + HTTP::Servers::StandardRouteHandler, DataFlow::FunctionNode { TrackedRouteHandlerCandidateWithSetup() { - exists(TrackedRouteHandlerCandidate tracked | - tracked.flowsTo(any(RouteSetup s).getRouteHandlerExpr().flow()) and - this = tracked - ) + this = any(RouteSetup s).getARouteHandler() } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll b/javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll index 8cddd068dc6..c7264ab0b1d 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll @@ -24,16 +24,19 @@ module SocketIO { result = DataFlow::moduleImport("socket.io").getAMemberCall("listen") } - /** A data flow node that may produce (that is, create or return) a socket.io server. */ - class ServerNode extends DataFlow::SourceNode { - ServerObject srv; - - ServerNode() { - this = newServer() and - srv = MkServer(this) + /** + * Gets a data flow node that may refer to the socket.io server created at `srv`. + */ + private DataFlow::SourceNode server(ServerObject srv, DataFlow::TypeTracker t) { + result = newServer() and + srv = MkServer(result) and + t.start() + or + exists(DataFlow::TypeTracker t2, DataFlow::SourceNode pred | pred = server(srv, t2) | + result = pred.track(t2, t) or // invocation of a chainable method - exists(ServerNode base, DataFlow::MethodCallNode mcn, string m | + exists(DataFlow::MethodCallNode mcn, string m | m = "adapter" or m = "attach" or m = "bind" or @@ -44,96 +47,116 @@ module SocketIO { m = "serveClient" or m = "set" | - mcn = base.getAMethodCall(m) and + mcn = pred.getAMethodCall(m) and // exclude getter versions exists(mcn.getAnArgument()) and - this = mcn and - srv = base.getServer() + result = mcn and + t = t2 ) - } + ) + } + + /** A data flow node that may produce (that is, create or return) a socket.io server. */ + class ServerNode extends DataFlow::SourceNode { + ServerObject srv; + + ServerNode() { this = server(srv, DataFlow::TypeTracker::end()) } /** Gets the server to which this node refers. */ ServerObject getServer() { result = srv } } + /** + * Gets the name of a chainable method on socket.io namespace objects, which servers forward + * to their default namespace. + */ + private string namespaceChainableMethod() { + result = "binary" or + result = "clients" or + result = "compress" or + result = "emit" or + result = "in" or + result = "send" or + result = "to" or + result = "use" or + result = "write" or + result = EventEmitter::chainableMethod() + } + + /** + * Gets a data flow node that may refer to the socket.io namespace created at `ns`. + */ + private DataFlow::SourceNode namespace(NamespaceObject ns, DataFlow::TypeTracker t) { + t.start() and + exists(ServerNode srv | + // namespace lookup on `srv` + result = srv.getAPropertyRead("sockets") and + ns = srv.getServer().getDefaultNamespace() + or + exists(DataFlow::MethodCallNode mcn, string path | + mcn = srv.getAMethodCall("of") and + mcn.getArgument(0).mayHaveStringValue(path) and + result = mcn and + ns = MkNamespace(srv.getServer(), path) + ) + or + // invocation of a method that `srv` forwards to its default namespace + result = srv.getAMethodCall(namespaceChainableMethod()) and + ns = srv.getServer().getDefaultNamespace() + ) + or + exists(DataFlow::SourceNode pred, DataFlow::TypeTracker t2 | pred = namespace(ns, t2) | + result = pred.track(t2, t) + or + // invocation of a chainable method + result = pred.getAMethodCall(namespaceChainableMethod()) and + t = t2 + or + // invocation of chainable getter method + exists(string m | + m = "json" or + m = "local" or + m = "volatile" + | + result = pred.getAPropertyRead(m) and + t = t2 + ) + ) + } + /** A data flow node that may produce a namespace object. */ class NamespaceNode extends DataFlow::SourceNode { NamespaceObject ns; - NamespaceNode() { - // namespace lookup - exists(ServerNode srv | - this = srv.getAPropertyRead("sockets") and - ns = srv.getServer().getDefaultNamespace() - or - exists(DataFlow::MethodCallNode mcn, string path | - mcn = srv.getAMethodCall("of") and - mcn.getArgument(0).mayHaveStringValue(path) and - this = mcn and - ns = MkNamespace(srv.getServer(), path) - ) - ) - or - // invocation of a chainable method - exists(string m | - m = "binary" or - m = "clients" or - m = "compress" or - m = "emit" or - m = "in" or - m = "send" or - m = "to" or - m = "use" or - m = "write" or - m = EventEmitter::chainableMethod() - | - exists(NamespaceNode base | - this = base.getAMethodCall(m) and - ns = base.getNamespace() - ) - or - // server objects forward these methods to their default namespace - exists(ServerNode srv | - this = srv.getAMethodCall(m) and - ns = srv.getServer().getDefaultNamespace() - ) - ) - or - // invocation of chainable getter method - exists(NamespaceNode base, string m | - m = "json" or - m = "local" or - m = "volatile" - | - this = base.getAPropertyRead(m) and - ns = base.getNamespace() - ) - } + NamespaceNode() { this = namespace(ns, DataFlow::TypeTracker::end()) } /** Gets the namespace to which this node refers. */ NamespaceObject getNamespace() { result = ns } } - /** A data flow node that may produce a socket object. */ - class SocketNode extends DataFlow::SourceNode { - NamespaceObject ns; - - SocketNode() { - // callback accepting a socket - exists(DataFlow::SourceNode base, string connect, DataFlow::MethodCallNode on | - ( - ns = base.(ServerNode).getServer().getDefaultNamespace() or - ns = base.(NamespaceNode).getNamespace() - ) and - (connect = "connect" or connect = "connection") - | - on = base.getAMethodCall(EventEmitter::on()) and - on.getArgument(0).mayHaveStringValue(connect) and - this = on.getCallback(1).getParameter(0) - ) + /** + * Gets a data flow node that may refer to a socket.io socket belonging to namespace `ns`. + */ + private DataFlow::SourceNode socket(NamespaceObject ns, DataFlow::TypeTracker t) { + // callback accepting a socket + t.start() and + exists(DataFlow::SourceNode base, string connect, DataFlow::MethodCallNode on | + ( + ns = base.(ServerNode).getServer().getDefaultNamespace() or + ns = base.(NamespaceNode).getNamespace() + ) and + (connect = "connect" or connect = "connection") + | + on = base.getAMethodCall(EventEmitter::on()) and + on.getArgument(0).mayHaveStringValue(connect) and + result = on.getCallback(1).getParameter(0) + ) + or + exists(DataFlow::SourceNode pred, DataFlow::TypeTracker t2 | pred = socket(ns, t2) | + result = pred.track(t2, t) or // invocation of a chainable method - exists(SocketNode base, string m | + exists(string m | m = "binary" or m = "compress" or m = "disconnect" or @@ -147,21 +170,28 @@ module SocketIO { m = "write" or m = EventEmitter::chainableMethod() | - this = base.getAMethodCall(m) and - ns = base.getNamespace() + result = pred.getAMethodCall(m) and + t = t2 ) or // invocation of a chainable getter method - exists(SocketNode base, string m | + exists(string m | m = "broadcast" or m = "json" or m = "local" or m = "volatile" | - this = base.getAPropertyRead(m) and - ns = base.getNamespace() + result = pred.getAPropertyRead(m) and + t = t2 ) - } + ) + } + + /** A data flow node that may produce a socket object. */ + class SocketNode extends DataFlow::SourceNode { + NamespaceObject ns; + + SocketNode() { this = socket(ns, DataFlow::TypeTracker::end()) } /** Gets the namespace to which this socket belongs. */ NamespaceObject getNamespace() { result = ns } @@ -361,21 +391,29 @@ module SocketIO { * (npm package `socket.io-client`). */ module SocketIOClient { + /** + * Gets a data flow node that may refer to the socket.io socket created at `invk`. + */ + private DataFlow::SourceNode socket(DataFlow::InvokeNode invk, DataFlow::TypeTracker t) { + t.start() and + exists(DataFlow::SourceNode io | + io = DataFlow::globalVarRef("io") or + io = DataFlow::globalVarRef("io").getAPropertyRead("connect") or + io = DataFlow::moduleImport("socket.io-client") or + io = DataFlow::moduleMember("socket.io-client", "connect") + | + invk = io.getAnInvocation() and + result = invk + ) + or + exists(DataFlow::TypeTracker t2 | result = socket(invk, t2).track(t2, t)) + } + /** A data flow node that may produce a socket object. */ class SocketNode extends DataFlow::SourceNode { DataFlow::InvokeNode invk; - SocketNode() { - exists(DataFlow::SourceNode io | - io = DataFlow::globalVarRef("io") or - io = DataFlow::globalVarRef("io").getAPropertyRead("connect") or - io = DataFlow::moduleImport("socket.io-client") or - io = DataFlow::moduleMember("socket.io-client", "connect") - | - invk = io.getAnInvocation() and - this = invk - ) - } + SocketNode() { this = socket(invk, DataFlow::TypeTracker::end()) } /** Gets the path of the namespace this socket belongs to, if it can be determined. */ string getNamespacePath() { diff --git a/javascript/ql/test/library-tests/TypeTracking/ClassStyle.ql b/javascript/ql/test/library-tests/TypeTracking/ClassStyle.ql index f965a40102f..6bf92c53424 100644 --- a/javascript/ql/test/library-tests/TypeTracking/ClassStyle.ql +++ b/javascript/ql/test/library-tests/TypeTracking/ClassStyle.ql @@ -6,86 +6,68 @@ string chainableMethod() { } class ApiObject extends DataFlow::NewNode { - ApiObject() { - this = DataFlow::moduleImport("@test/myapi").getAnInstantiation() - } + ApiObject() { this = DataFlow::moduleImport("@test/myapi").getAnInstantiation() } DataFlow::SourceNode ref(DataFlow::TypeTracker t) { t.start() and result = this or t.start() and - result = ref(_).getAMethodCall(chainableMethod()) + result = ref(DataFlow::TypeTracker::end()).getAMethodCall(chainableMethod()) or - exists(DataFlow::TypeTracker t2 | - result = ref(t2).track(t2, t) - ) - } - - DataFlow::SourceNode ref() { - result = ref(_) + exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t)) } + + DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) } } class Connection extends DataFlow::SourceNode { ApiObject api; - Connection() { - this = api.ref().getAMethodCall("createConnection") - } + Connection() { this = api.ref().getAMethodCall("createConnection") } DataFlow::SourceNode ref(DataFlow::TypeTracker t) { t.start() and result = this or - exists(DataFlow::TypeTracker t2 | - result = ref(t2).track(t2, t) - ) - } - - DataFlow::SourceNode ref() { - result = ref(_) + exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t)) } + DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) } + DataFlow::SourceNode getACallbackNode(DataFlow::TypeBackTracker t) { t.start() and result = ref().getAMethodCall("getData").getArgument(0).getALocalSource() or - exists(DataFlow::TypeBackTracker t2 | - result = getACallbackNode(t2).backtrack(t2, t) - ) + exists(DataFlow::TypeBackTracker t2 | result = getACallbackNode(t2).backtrack(t2, t)) } DataFlow::FunctionNode getACallback() { - result = getACallbackNode(_).getAFunctionValue() + result = getACallbackNode(DataFlow::TypeBackTracker::end()).getAFunctionValue() } } class DataValue extends DataFlow::SourceNode { Connection connection; - DataValue() { - this = connection.getACallback().getParameter(0) - } + DataValue() { this = connection.getACallback().getParameter(0) } DataFlow::SourceNode ref(DataFlow::TypeTracker t) { t.start() and result = this or - exists(DataFlow::TypeTracker t2 | - result = ref(t2).track(t2, t) - ) - } - - DataFlow::SourceNode ref() { - result = ref(_) + exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t)) } + + DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) } } query DataFlow::SourceNode test_ApiObject() { result = any(ApiObject obj).ref() } query DataFlow::SourceNode test_Connection() { result = any(Connection c).ref() } -query DataFlow::SourceNode test_DataCallback() { result = any(Connection c).getACallbackNode(_) } +query DataFlow::SourceNode test_DataCallback() { + result = any(Connection c).getACallbackNode(DataFlow::TypeBackTracker::end()) +} query DataFlow::SourceNode test_DataValue() { result = any(DataValue v).ref() } diff --git a/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.expected b/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.expected index 0c0fea7a928..8507edce86f 100644 --- a/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.expected +++ b/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.expected @@ -5,6 +5,7 @@ apiObject connection | type tracker with call steps | tst.js:6:15:6:18 | conn | | type tracker with call steps | tst.js:10:5:10:19 | this.connection | +| type tracker with call steps with property connection | tst.js:6:14:6:13 | this | | type tracker without call steps | tst.js:15:10:15:49 | api.cha ... ction() | | type tracker without call steps | tst.js:18:7:18:21 | getConnection() | | type tracker without call steps | tst.js:30:9:30:23 | getConnection() | diff --git a/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.ql b/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.ql index 678dc12de0a..9f265ecc47c 100644 --- a/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.ql +++ b/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.ql @@ -10,52 +10,38 @@ DataFlow::SourceNode apiObject(DataFlow::TypeTracker t) { result = DataFlow::moduleImport("@test/myapi").getAnInstantiation() or t.start() and - result = apiObject(_).getAMethodCall(chainableMethod()) + result = apiObject(DataFlow::TypeTracker::end()).getAMethodCall(chainableMethod()) or - exists(DataFlow::TypeTracker t2 | - result = apiObject(t2).track(t2, t) - ) + exists(DataFlow::TypeTracker t2 | result = apiObject(t2).track(t2, t)) } -query DataFlow::SourceNode apiObject() { - result = apiObject(_) -} +query DataFlow::SourceNode apiObject() { result = apiObject(DataFlow::TypeTracker::end()) } query DataFlow::SourceNode connection(DataFlow::TypeTracker t) { t.start() and result = apiObject().getAMethodCall("createConnection") or - exists(DataFlow::TypeTracker t2 | - result = connection(t2).track(t2, t) - ) + exists(DataFlow::TypeTracker t2 | result = connection(t2).track(t2, t)) } -DataFlow::SourceNode connection() { - result = connection(_) -} +DataFlow::SourceNode connection() { result = connection(DataFlow::TypeTracker::end()) } DataFlow::SourceNode dataCallback(DataFlow::TypeBackTracker t) { t.start() and result = connection().getAMethodCall("getData").getArgument(0).getALocalSource() or - exists(DataFlow::TypeBackTracker t2 | - result = dataCallback(t2).backtrack(t2, t) - ) + exists(DataFlow::TypeBackTracker t2 | result = dataCallback(t2).backtrack(t2, t)) } query DataFlow::SourceNode dataCallback() { - result = dataCallback(_) + result = dataCallback(DataFlow::TypeBackTracker::end()) } DataFlow::SourceNode dataValue(DataFlow::TypeTracker t) { t.start() and result = dataCallback().getAFunctionValue().getParameter(0) or - exists(DataFlow::TypeTracker t2 | - result = dataValue(t2).track(t2, t) - ) + exists(DataFlow::TypeTracker t2 | result = dataValue(t2).track(t2, t)) } -query DataFlow::SourceNode dataValue() { - result = dataValue(_) -} +query DataFlow::SourceNode dataValue() { result = dataValue(DataFlow::TypeTracker::end()) } diff --git a/javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.expected b/javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.expected index 76865b0c1db..fca1b6a91a3 100644 --- a/javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.expected +++ b/javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.expected @@ -4,6 +4,8 @@ | electron.js:3:10:3:48 | new Bro ... s: {}}) | | electron.js:4:5:4:46 | bv | | electron.js:4:10:4:46 | new Bro ... s: {}}) | +| electron.js:35:14:35:14 | x | +| electron.js:36:12:36:12 | x | | electron.js:39:5:39:6 | bw | | electron.js:40:5:40:6 | bv | | electron.ts:3:12:3:13 | bw | diff --git a/javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteHandlerCandidate.expected b/javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteHandlerCandidate.expected index 445223631c0..62cf78ad113 100644 --- a/javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteHandlerCandidate.expected +++ b/javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteHandlerCandidate.expected @@ -3,6 +3,9 @@ | src/hapi.js:1:1:1:30 | functio ... t, h){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | src/iterated-handlers.js:4:2:4:22 | functio ... res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | src/middleware-attacher-getter.js:29:32:29:51 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/nodejs.js:5:22:5:41 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/nodejs.js:11:23:11:42 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/nodejs.js:12:25:12:44 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | src/route-objects.js:7:19:7:38 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | src/route-objects.js:8:12:10:5 | (req, res) {\\n\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | src/route-objects.js:20:16:22:9 | (req, r ... } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | @@ -10,8 +13,20 @@ | src/route-objects.js:40:12:42:5 | (req, res) {\\n\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | src/route-objects.js:50:12:52:5 | (req, res) {\\n\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | src/route-objects.js:56:12:58:5 | functio ... ;\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/tst.js:6:32:6:52 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/tst.js:8:32:8:61 | functio ... nse) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/tst.js:30:36:30:56 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/tst.js:33:18:33:38 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/tst.js:34:18:34:38 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/tst.js:37:5:37:25 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/tst.js:38:5:38:25 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/tst.js:43:18:43:38 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | src/tst.js:46:1:46:23 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | src/tst.js:52:1:54:1 | functio ... req()\\n} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | src/tst.js:61:1:63:1 | functio ... turn;\\n} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | src/tst.js:70:5:72:5 | functio ... \\n\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/tst.js:79:5:81:5 | functio ... \\n\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/tst.js:84:5:86:5 | functio ... \\n\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | src/tst.js:109:16:111:9 | functio ... } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/tst.js:124:16:126:9 | functio ... } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | +| src/tst.js:132:16:134:9 | functio ... } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | diff --git a/javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteSetupCandidate.expected b/javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteSetupCandidate.expected index a349499ef27..e69de29bb2d 100644 --- a/javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteSetupCandidate.expected +++ b/javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteSetupCandidate.expected @@ -1,17 +0,0 @@ -| src/nodejs.js:5:1:5:42 | unknown ... res){}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/nodejs.js:11:1:11:43 | unknown ... res){}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/nodejs.js:12:1:12:45 | unknown ... res){}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:6:1:6:53 | someOth ... es) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:8:1:8:62 | someOth ... se) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:30:1:30:57 | someOth ... es) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:32:1:34:39 | someOth ... es) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:36:1:39:2 | someOth ... ) {}\\n]) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:41:1:43:39 | someOth ... es) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:87:5:87:57 | unknown ... cSetup) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:96:5:96:36 | unknown ... h', rh) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:98:5:98:38 | unknown ... , [rh]) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:104:5:104:45 | unknown ... wn, rh) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:137:5:137:57 | unknown ... cSetup) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:149:5:149:36 | unknown ... h', rh) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:151:5:151:38 | unknown ... , [rh]) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. | -| src/tst.js:157:5:157:45 | unknown ... wn, rh) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |