diff --git a/javascript/ql/src/semmle/javascript/dataflow/TypeTracking.qll b/javascript/ql/src/semmle/javascript/dataflow/TypeTracking.qll index 1c06cf564a7..b2decce2abe 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/TypeTracking.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/TypeTracking.qll @@ -9,13 +9,19 @@ import javascript private import internal.FlowSteps +private class PropertyName extends string { + PropertyName() { this = any(DataFlow::PropRef pr).getPropertyName() } +} + /** * A description of a step on an inter-procedural data flow path. */ private newtype TStepSummary = LevelStep() or CallStep() or - ReturnStep() + ReturnStep() or + StoreStep(PropertyName prop) or + LoadStep(PropertyName prop) /** * INTERNAL: Use `TypeTracker` or `TypeBackTracker` instead. @@ -64,6 +70,14 @@ module StepSummary { // Flow through an instance field between members of the same class DataFlow::localFieldStep(predNode, succ) and summary = LevelStep() + or + exists(string prop | + basicStoreStep(predNode, succ, prop) and + summary = StoreStep(prop) + or + loadStep(predNode, succ, prop) and + summary = LoadStep(prop) + ) ) } @@ -73,8 +87,22 @@ module StepSummary { * 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()) + exists(boolean hadCall, boolean hasCall, string oldProp, string newProp | + hadCall = type.hasCall() and + oldProp = type.getProp() + | + not (hadCall = true and summary.hasReturn() = true) and + hasCall = hadCall.booleanOr(summary.hasCall()) and + ( + if summary instanceof StoreStep + then oldProp = "" and summary = StoreStep(newProp) + else + if summary instanceof LoadStep + then summary = LoadStep(oldProp) and newProp = "" + else newProp = oldProp + ) and + result = MkTypeTracker(hasCall, newProp) + ) } /** @@ -83,12 +111,27 @@ module StepSummary { * 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()) + exists(boolean hadReturn, boolean hasReturn, string oldProp, string newProp | + hadReturn = type.hasReturn() and + oldProp = type.getProp() + | + not (hadReturn = true and summary.hasCall() = true) and + hasReturn = hadReturn.booleanOr(summary.hasReturn()) and + ( + if summary instanceof StoreStep + then summary = StoreStep(oldProp) and newProp = "" + else + if summary instanceof LoadStep + then oldProp = "" and summary = LoadStep(newProp) + else newProp = oldProp + ) and + result = MkTypeBackTracker(hasReturn, newProp) + ) } } -private newtype TTypeTracker = MkTypeTracker(Boolean hasCall) +private newtype TTypeTracker = + MkTypeTracker(Boolean hasCall, string prop) { prop = "" or prop instanceof PropertyName } /** * EXPERIMENTAL. @@ -112,7 +155,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 @@ -121,18 +164,24 @@ private newtype TTypeTracker = MkTypeTracker(Boolean hasCall) class TypeTracker extends TTypeTracker { Boolean hasCall; - TypeTracker() { this = MkTypeTracker(hasCall) } + string prop; + + TypeTracker() { this = MkTypeTracker(hasCall, prop) } 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. @@ -140,9 +189,16 @@ class TypeTracker extends TTypeTracker { * Holds if this type has been tracked into a call. */ boolean hasCall() { result = hasCall } + + string getProp() { result = prop } } -private newtype TTypeBackTracker = MkTypeBackTracker(Boolean hasReturn) +module TypeTracker { + TypeTracker end() { result.end() } +} + +private newtype TTypeBackTracker = + MkTypeBackTracker(Boolean hasReturn, string prop) { prop = "" or prop instanceof PropertyName } /** * EXPERIMENTAL. @@ -168,24 +224,30 @@ 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) } 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. @@ -193,4 +255,10 @@ class TypeBackTracker extends TTypeBackTracker { * Holds if this type has been back-tracked into a call through return edge. */ boolean hasReturn() { result = hasReturn } + + string getProp() { result = prop } +} + +module TypeBackTracker { + TypeBackTracker end() { result.end() } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/Connect.qll b/javascript/ql/src/semmle/javascript/frameworks/Connect.qll index f01a995b9ef..31e561d66b0 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Connect.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Connect.qll @@ -118,7 +118,7 @@ module Connect { } override DataFlow::SourceNode getARouteHandler() { - result = getARouteHandler(_) + result = getARouteHandler(DataFlow::TypeBackTracker::end()) } private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) { diff --git a/javascript/ql/src/semmle/javascript/frameworks/Electron.qll b/javascript/ql/src/semmle/javascript/frameworks/Electron.qll index b2e6ff09ced..8ea429110ad 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Electron.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Electron.qll @@ -69,7 +69,7 @@ module Electron { * A data flow node whose value may originate from a browser object instantiation. */ private class BrowserObjectByFlow extends BrowserObject { - BrowserObjectByFlow() { browserObject(_).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 3b1b8e53ef3..8f68d437786 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Express.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Express.qll @@ -111,7 +111,7 @@ module Express { Expr getLastRouteHandlerExpr() { result = max(int i | | getRouteHandlerExpr(i) order by i) } override DataFlow::SourceNode getARouteHandler() { - result = getARouteHandler(_) + result = getARouteHandler(DataFlow::TypeBackTracker::end()) } private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) { diff --git a/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll b/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll index 6bce331e7f3..2ac592c20d6 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll @@ -282,7 +282,7 @@ module HTTP { */ abstract RouteHandler getRouteHandler(); - predicate flowsTo(DataFlow::Node nd) { flowsToSourceNode(_).flowsTo(nd) } + predicate flowsTo(DataFlow::Node nd) { flowsToSourceNode(DataFlow::TypeTracker::end()).flowsTo(nd) } private DataFlow::SourceNode flowsToSourceNode(DataFlow::TypeTracker t) { t.start() and @@ -303,7 +303,7 @@ module HTTP { */ abstract RouteHandler getRouteHandler(); - predicate flowsTo(DataFlow::Node nd) { flowsToSourceNode(_).flowsTo(nd) } + predicate flowsTo(DataFlow::Node nd) { flowsToSourceNode(DataFlow::TypeTracker::end()).flowsTo(nd) } private DataFlow::SourceNode flowsToSourceNode(DataFlow::TypeTracker t) { t.start() and diff --git a/javascript/ql/src/semmle/javascript/frameworks/Hapi.qll b/javascript/ql/src/semmle/javascript/frameworks/Hapi.qll index ab553a12f29..5bca1aa5378 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Hapi.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Hapi.qll @@ -193,7 +193,7 @@ module Hapi { } override DataFlow::SourceNode getARouteHandler() { - result = getARouteHandler(_) + result = getARouteHandler(DataFlow::TypeBackTracker::end()) } private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) { diff --git a/javascript/ql/src/semmle/javascript/frameworks/Koa.qll b/javascript/ql/src/semmle/javascript/frameworks/Koa.qll index 6c3e9f7848c..34ef023c815 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Koa.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Koa.qll @@ -79,7 +79,7 @@ module Koa { RouteHandler getRouteHandler() { result = rh } predicate flowsTo(DataFlow::Node nd) { - flowsToSourceNode(_).flowsTo(nd) + flowsToSourceNode(DataFlow::TypeTracker::end()).flowsTo(nd) } private DataFlow::SourceNode flowsToSourceNode(DataFlow::TypeTracker t) { diff --git a/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll b/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll index a1550f2dddb..f578f2eb5c9 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll @@ -189,7 +189,7 @@ module NodeJSLib { } override DataFlow::SourceNode getARouteHandler() { - result = getARouteHandler(_) + result = getARouteHandler(DataFlow::TypeBackTracker::end()) } private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) { diff --git a/javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll b/javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll index 1d83be0755f..a42f028b48c 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll @@ -61,7 +61,7 @@ module SocketIO { class ServerNode extends DataFlow::SourceNode { ServerObject srv; - ServerNode() { this = server(srv, _) } + ServerNode() { this = server(srv, DataFlow::TypeTracker::end()) } /** Gets the server to which this node refers. */ ServerObject getServer() { result = srv } @@ -130,7 +130,7 @@ module SocketIO { class NamespaceNode extends DataFlow::SourceNode { NamespaceObject ns; - NamespaceNode() { this = namespace(ns, _) } + NamespaceNode() { this = namespace(ns, DataFlow::TypeTracker::end()) } /** Gets the namespace to which this node refers. */ NamespaceObject getNamespace() { result = ns } @@ -194,7 +194,7 @@ module SocketIO { class SocketNode extends DataFlow::SourceNode { NamespaceObject ns; - SocketNode() { this = socket(ns, _) } + SocketNode() { this = socket(ns, DataFlow::TypeTracker::end()) } /** Gets the namespace to which this socket belongs. */ NamespaceObject getNamespace() { result = ns } @@ -417,7 +417,7 @@ module SocketIOClient { class SocketNode extends DataFlow::SourceNode { DataFlow::InvokeNode invk; - SocketNode() { this = socket(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.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/HTTP-heuristics/RouteHandler.expected b/javascript/ql/test/library-tests/frameworks/HTTP-heuristics/RouteHandler.expected index a5a22b0cf84..4f21ca278f4 100644 --- a/javascript/ql/test/library-tests/frameworks/HTTP-heuristics/RouteHandler.expected +++ b/javascript/ql/test/library-tests/frameworks/HTTP-heuristics/RouteHandler.expected @@ -11,6 +11,8 @@ | src/exported-handler.js:1:19:1:55 | functio ... res) {} | | src/exported-middleware-attacher-2.js:2:13:2:32 | function(req, res){} | | src/exported-middleware-attacher.js:2:13:2:32 | function(req, res){} | +| src/handler-in-property.js:5:14:5:33 | function(req, res){} | +| src/handler-in-property.js:12:18:12:37 | function(req, res){} | | src/middleware-attacher-getter.js:4:17:4:36 | function(req, res){} | | src/middleware-attacher-getter.js:19:19:19:38 | function(req, res){} | | src/middleware-attacher.js:3:13:3:32 | function(req, res){} | 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 12dfcd4af93..62cf78ad113 100644 --- a/javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteHandlerCandidate.expected +++ b/javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteHandlerCandidate.expected @@ -1,7 +1,5 @@ | src/bound-handler.js:4:1:4:28 | functio ... res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | src/bound-handler.js:9:12:9:31 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | -| src/handler-in-property.js:5:14:5:33 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | -| src/handler-in-property.js:12:18:12:37 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. | | 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`. |