hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-04 21:25:44 +02:00
Code Issues Packages Projects Releases Wiki Activity

add explicit this to member call

Browse Source
This commit is contained in:
Erik Krogh Kristensen
2022-01-20 22:28:44 +01:00
parent ddfc3bc00f
commit f500bccbe4
17 changed files with 222 additions and 198 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
javascript/ql/lib/semmle/javascript/ApiGraphs.qll
View File

@@ -998,7 +998,7 @@ module API {
* Gets an API node where a RHS of the node is the `i`th argument to this call.
*/
pragma[noinline]
private Node getAParameterCandidate(int i) { result.getARhs() = getArgument(i) }
private Node getAParameterCandidate(int i) { result.getARhs() = this.getArgument(i) }
/** Gets the API node for a parameter of this invocation. */
Node getAParameter() { result = this.getParameter(_) }

96
javascript/ql/lib/semmle/javascript/Routing.qll
View File

@@ -137,12 +137,12 @@ module Routing {
* this subtree, and subsequently passed on to the successor.
*/
predicate mayResumeDispatch() {
getLastChild().mayResumeDispatch()
this.getLastChild().mayResumeDispatch()
or
exists(this.(RouteHandler).getAContinuationInvocation())
or
// Leaf nodes that aren't functions are assumed to invoke their continuation
not exists(getLastChild()) and
not exists(this.getLastChild()) and
not this instanceof RouteHandler
or
this instanceof MkRouter
@@ -150,7 +150,7 @@ module Routing {
/** Gets the parent of this node, provided that this node may invoke its continuation. */
private Node getContinuationParent() {
result = getParent() and
result = this.getParent() and
result.mayResumeDispatch()
}
@@ -170,7 +170,7 @@ module Routing {
*/
private predicate isFork() {
exists(Node child |
child = getAChild() and
child = this.getAChild() and
child.mayResumeDispatch() and
exists(child.getNextSibling())
)
@@ -184,11 +184,11 @@ module Routing {
* that is, a node that has siblings (i.e. multiple children).
*/
private string getPathFromFork(Node fork) {
isFork() and
this.isFork() and
this = fork and
result = ""
or
exists(Node parent | parent = getParent() |
exists(Node parent | parent = this.getParent() |
not exists(parent.getRelativePath()) and
result = parent.getPathFromFork(fork)
or
@@ -205,27 +205,27 @@ module Routing {
* that is, a node that has siblings (i.e. multiple children).
*/
private string getHttpMethodFromFork(Node fork) {
isFork() and
this.isFork() and
this = fork and
(
result = getOwnHttpMethod()
result = this.getOwnHttpMethod()
or
not exists(getOwnHttpMethod()) and
not exists(this.getOwnHttpMethod()) and
result = "*"
)
or
result = getParent().getHttpMethodFromFork(fork) and
result = this.getParent().getHttpMethodFromFork(fork) and
(
// Only the ancestor restricts the HTTP method
not exists(getOwnHttpMethod())
not exists(this.getOwnHttpMethod())
or
// Intersect permitted HTTP methods
result = getOwnHttpMethod()
result = this.getOwnHttpMethod()
)
or
// The ancestor allows any HTTP method, but this node restricts it
getParent().getHttpMethodFromFork(fork) = "*" and
result = getOwnHttpMethod()
this.getParent().getHttpMethodFromFork(fork) = "*" and
result = this.getOwnHttpMethod()
}
/**
@@ -256,13 +256,15 @@ module Routing {
* Holds if `node` has processed the incoming request strictly prior to this node.
*/
pragma[inline]
predicate isGuardedByNode(Node node) { isGuardedByNodeInternal(pragma[only_bind_out](node)) }
predicate isGuardedByNode(Node node) {
this.isGuardedByNodeInternal(pragma[only_bind_out](node))
}
/**
* Holds if the middleware corresponding to `node` has processed the incoming request strictly prior to this node.
*/
pragma[inline]
predicate isGuardedBy(DataFlow::Node node) { isGuardedByNode(getNode(node)) }
predicate isGuardedBy(DataFlow::Node node) { this.isGuardedByNode(getNode(node)) }
/**
* Gets an HTTP method name which this node will accept, or nothing if the node accepts all HTTP methods, not
@@ -271,16 +273,16 @@ module Routing {
HTTP::RequestMethodName getOwnHttpMethod() { none() } // Overridden in subclass
private Node getAUseSiteInRouteSetup() {
if getParent() instanceof RouteSetup
if this.getParent() instanceof RouteSetup
then result = this
else result = getParent().getAUseSiteInRouteSetup()
else result = this.getParent().getAUseSiteInRouteSetup()
}
/** Gets a place where this route node is installed as a route handler. */
Node getRouteInstallation() {
result = getAUseSiteInRouteSetup()
result = this.getAUseSiteInRouteSetup()
or
not exists(getAUseSiteInRouteSetup()) and
not exists(this.getAUseSiteInRouteSetup()) and
result = this
}
@@ -352,7 +354,7 @@ module Routing {
Node getChild(int n) { none() }
/** Gets the number of children of this route node. */
final int getNumChild() { result = count(int n | exists(getChild(n))) }
final int getNumChild() { result = count(int n | exists(this.getChild(n))) }
/**
* Gets a path prefix to be matched against the path of incoming requests.
@@ -416,7 +418,7 @@ module Routing {
* Gets a data flow node that flows to this use-site in one step.
*/
DataFlow::Node getSource() {
result = getALocalSource()
result = this.getALocalSource()
or
StepSummary::smallstep(result, this, routeStepSummary())
or
@@ -425,7 +427,7 @@ module Routing {
RouteHandlerTrackingStep::step(result, this)
or
exists(string prop |
StepSummary::smallstep(result, getSourceProp(prop).getALocalUse(), StoreStep(prop))
StepSummary::smallstep(result, this.getSourceProp(prop).getALocalUse(), StoreStep(prop))
)
or
this = getAnEnumeratedArrayElement(result)
@@ -435,32 +437,32 @@ module Routing {
private DataFlow::SourceNode getSourceProp(string prop) {
StepSummary::step(result, this, LoadStep(prop))
or
StepSummary::step(result, getSourceProp(prop), routeStepSummary())
StepSummary::step(result, this.getSourceProp(prop), routeStepSummary())
or
StepSummary::step(result, getSourceProp(prop), CopyStep(prop))
StepSummary::step(result, this.getSourceProp(prop), CopyStep(prop))
or
exists(string oldProp |
StepSummary::step(result, getSourceProp(oldProp), LoadStoreStep(prop, oldProp))
StepSummary::step(result, this.getSourceProp(oldProp), LoadStoreStep(prop, oldProp))
)
}
private DataFlow::Node getStrictSource() {
result = getSource() and
result = this.getSource() and
result != this
}
final override Routing::Node getChild(int n) {
n = 0 and
result = MkValueNode(getStrictSource())
result = MkValueNode(this.getStrictSource())
or
// If we cannot find the source of the use-site, but we know it's somehow a reference to a router,
// treat the router as the source. This is needed to handle chaining calls on the router, as the
// specific framework model knows about chaining steps, but the general `getSource()` predicate doesn't.
n = 0 and
not exists(getStrictSource()) and
not exists(this.getStrictSource()) and
exists(Router::Range router |
this = router.getAReference().getALocalUse() and
result = MkRouter(router, getContainer())
result = MkRouter(router, this.getContainer())
)
}
}
@@ -483,7 +485,7 @@ module Routing {
*/
abstract DataFlow::Node getArgumentNode(int n);
final override Node getChild(int n) { result = MkValueNode(getArgumentNode(n)) }
final override Node getChild(int n) { result = MkValueNode(this.getArgumentNode(n)) }
}
/** An argument to a `WithArguments` instance, seen as a use site. */
@@ -497,7 +499,7 @@ module Routing {
private class ImpliedArrayRoute extends ValueNode::WithArguments, DataFlow::ArrayCreationNode {
ImpliedArrayRoute() { this instanceof ValueNode::UseSite }
override DataFlow::Node getArgumentNode(int n) { result = getElement(n) }
override DataFlow::Node getArgumentNode(int n) { result = this.getElement(n) }
}
}
@@ -521,10 +523,10 @@ module Routing {
* A node in the routing tree which has no parent.
*/
class RootNode extends Node {
RootNode() { not exists(getParent()) }
RootNode() { not exists(this.getParent()) }
/** Gets a node that is part of this subtree. */
final Node getADescendant() { result = getAChild*() }
final Node getADescendant() { result = this.getAChild*() }
}
/**
@@ -546,7 +548,7 @@ module Routing {
*
* This is an alias for `getParent`, but may be preferred for readability.
*/
final Node getRouter() { result = getParent() }
final Node getRouter() { result = this.getParent() }
}
/**
@@ -568,7 +570,7 @@ module Routing {
Node getChild(int n) { none() }
/** Gets the number of children of this route node. */
final int getNumChild() { result = count(int n | exists(getChild(n))) }
final int getNumChild() { result = count(int n | exists(this.getChild(n))) }
/**
* Gets a path prefix to be matched against the path of incoming requests.
@@ -630,13 +632,13 @@ module Routing {
* This class can be extended to contribute new kinds of route handlers.
*/
abstract class MethodCall extends RouteSetup::Range, DataFlow::MethodCallNode {
override Node getChild(int n) { result = MkValueNode(getChildNode(n)) }
override Node getChild(int n) { result = MkValueNode(this.getChildNode(n)) }
/** Gets the `n`th child of this route setup. */
DataFlow::Node getChildNode(int n) { result = getArgument(n) }
DataFlow::Node getChildNode(int n) { result = this.getArgument(n) }
override predicate isInstalledAt(Router::Range router, ControlFlowNode cfgNode) {
this = router.getAReference().getAMethodCall() and cfgNode = getEnclosingExpr()
this = router.getAReference().getAMethodCall() and cfgNode = this.getEnclosingExpr()
}
}
@@ -679,14 +681,14 @@ module Routing {
result.isInstalledAt(router, any(ControlFlowNode cfg | cfg.getContainer() = container))
}
override Node getAChild() { result = MkRouteSetup(getARouteSetup()) }
override Node getAChild() { result = MkRouteSetup(this.getARouteSetup()) }
override Node getLastChild() {
result = getMostRecentRouteSetupAt(router, container.getExit())
}
override Node getFirstChild() {
result = getAChild() and not exists(result.getPreviousSibling())
result = this.getAChild() and not exists(result.getPreviousSibling())
}
}
}
@@ -753,14 +755,14 @@ module Routing {
ImpliedRouteSetup() {
FlowSteps::calls(this, target) and
routerIsLiveInContainer(router, target) and
routerIsLiveInContainer(router, getContainer())
routerIsLiveInContainer(router, this.getContainer())
}
override Routing::Node getChild(int n) { result = MkRouter(router, target) and n = 0 }
override predicate isInstalledAt(Router::Range r, ControlFlowNode cfgNode) {
r = router and
cfgNode = getEnclosingExpr()
cfgNode = this.getEnclosingExpr()
}
}
@@ -799,11 +801,11 @@ module Routing {
* if the default behavior is inadequate for that framework.
*/
DataFlow::CallNode getAContinuationInvocation() {
result = getAParameter().ref().getAnInvocation() and
result = this.getAParameter().ref().getAnInvocation() and
result.getNumArgument() = 0
or
result.(DataFlow::MethodCallNode).getMethodName() = "then" and
result.getArgument(0) = getAParameter().ref().getALocalUse()
result.getArgument(0) = this.getAParameter().ref().getALocalUse()
}
}
@@ -827,11 +829,11 @@ module Routing {
t.start() and
result = this
or
exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t))
exists(DataFlow::TypeTracker t2 | result = this.ref(t2).track(t2, t))
}
/** Gets a data flow node referring to this route handler parameter. */
DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) }
DataFlow::SourceNode ref() { result = this.ref(DataFlow::TypeTracker::end()) }
/**
* Gets the corresponding route handler, that is, the function on which this is a parameter.

10
javascript/ql/lib/semmle/javascript/dependencies/FrameworkLibraries.qll
View File

@@ -75,7 +75,9 @@ abstract class FrameworkLibraryInstance extends TopLevel {
* via the `src` attribute of a `<script>` element.
*/
abstract class FrameworkLibraryReference extends HTML::Attribute {
FrameworkLibraryReference() { getName() = "src" and getElement() instanceof HTML::ScriptElement }
FrameworkLibraryReference() {
this.getName() = "src" and this.getElement() instanceof HTML::ScriptElement
}
/**
* Holds if this is a reference to version `v` of framework library `fl`.
@@ -160,7 +162,7 @@ abstract class FrameworkLibraryWithGenericURL extends FrameworkLibraryWithURLReg
string getAnAlias() { none() }
override string getAURLRegex() {
exists(string id | id = getId() or id = getAnAlias() |
exists(string id | id = this.getId() or id = this.getAnAlias() |
result = ".*(?:^|/)" + id + "-(" + semverRegex() + ")" + variantRegex() + "\\.js" or
result =
".*/(?:\\w+@)?(" + semverRegex() + ")/(?:(?:dist|js|" + id + ")/)?" + id + variantRegex() +
@@ -976,7 +978,9 @@ private class TwitterTextClassicInstance extends FrameworkLibraryInstance {
private class FrameworkLibraryReferenceToInstance extends FrameworkLibraryReference {
FrameworkLibraryInstance fli;
FrameworkLibraryReferenceToInstance() { fli = getElement().(HTML::ScriptElement).resolveSource() }
FrameworkLibraryReferenceToInstance() {
fli = this.getElement().(HTML::ScriptElement).resolveSource()
}
override predicate info(FrameworkLibrary fl, string v) { fli.info(fl, v) }
}

12
javascript/ql/lib/semmle/javascript/frameworks/Express.qll
View File

@@ -84,14 +84,14 @@ module Express {
}
private class RoutingTreeSetup extends Routing::RouteSetup::MethodCall {
RoutingTreeSetup() { asExpr() instanceof RouteSetup }
RoutingTreeSetup() { this.asExpr() instanceof RouteSetup }
override string getRelativePath() {
not getMethodName() = "param" and // do not treat parameter name as a path
result = getArgument(0).getStringValue()
not this.getMethodName() = "param" and // do not treat parameter name as a path
result = this.getArgument(0).getStringValue()
}
override HTTP::RequestMethodName getHttpMethod() { result.toLowerCase() = getMethodName() }
override HTTP::RequestMethodName getHttpMethod() { result.toLowerCase() = this.getMethodName() }
}
/**
@@ -117,7 +117,7 @@ module Express {
override predicate isInstalledAt(Routing::Router::Range router, ControlFlowNode cfgNode) {
router.getAReference().getALocalUse() = limitCall.getArgument(0) and
cfgNode = asExpr()
cfgNode = this.asExpr()
}
}
@@ -845,7 +845,7 @@ module Express {
}
/** Gets a data flow node referring to this router. */
DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) }
DataFlow::SourceNode ref() { result = this.ref(DataFlow::TypeTracker::end()) }
/**
* Holds if `sink` may refer to this router.

41
javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll
View File

@@ -161,7 +161,7 @@ module Fastify {
if methodName = "route"
then
result = this.flow().(DataFlow::MethodCallNode).getOptionArgument(0, getNthHandlerName(_))
else result = getLastArgument().flow()
else result = this.getLastArgument().flow()
}
}
@@ -185,22 +185,23 @@ module Fastify {
private class FullRoutingTreeSetup extends Routing::RouteSetup::MethodCall {
FullRoutingTreeSetup() {
asExpr() instanceof RouteSetup and
getMethodName() = "route"
this.asExpr() instanceof RouteSetup and
this.getMethodName() = "route"
}
override string getRelativePath() { result = getOptionArgument(0, "url").getStringValue() }
override string getRelativePath() { result = this.getOptionArgument(0, "url").getStringValue() }
override HTTP::RequestMethodName getHttpMethod() {
result = getOptionArgument(0, "method").getStringValue().toUpperCase()
result = this.getOptionArgument(0, "method").getStringValue().toUpperCase()
}
private DataFlow::Node getRawChild(int n) {
result = getOptionArgument(0, getNthHandlerName(n))
result = this.getOptionArgument(0, getNthHandlerName(n))
}
override DataFlow::Node getChildNode(int n) {
result = rank[n + 1](DataFlow::Node child, int k | child = getRawChild(k) | child order by k)
result =
rank[n + 1](DataFlow::Node child, int k | child = this.getRawChild(k) | child order by k)
}
}
@@ -209,34 +210,38 @@ module Fastify {
private DataFlow::SourceNode pluginBody(DataFlow::TypeBackTracker t) {
t.start() and
result = getArgument(0).getALocalSource()
result = this.getArgument(0).getALocalSource()
or
// step through calls to require('fastify-plugin')
result = pluginBody(t).(FastifyPluginCall).getArgument(0).getALocalSource()
result = this.pluginBody(t).(FastifyPluginCall).getArgument(0).getALocalSource()
or
exists(DataFlow::TypeBackTracker t2 | result = pluginBody(t2).backtrack(t2, t))
exists(DataFlow::TypeBackTracker t2 | result = this.pluginBody(t2).backtrack(t2, t))
}
/** Gets a functino flowing into the first argument. */
DataFlow::FunctionNode pluginBody() { result = pluginBody(DataFlow::TypeBackTracker::end()) }
override HTTP::RequestMethodName getHttpMethod() {
result = getOptionArgument(1, "method").getStringValue().toUpperCase()
DataFlow::FunctionNode pluginBody() {
result = this.pluginBody(DataFlow::TypeBackTracker::end())
}
override string getRelativePath() { result = getOptionArgument(1, "prefix").getStringValue() }
override HTTP::RequestMethodName getHttpMethod() {
result = this.getOptionArgument(1, "method").getStringValue().toUpperCase()
}
override string getRelativePath() {
result = this.getOptionArgument(1, "prefix").getStringValue()
}
override DataFlow::Node getChildNode(int n) {
n = 0 and
(
// If we can see the plugin body, use its server parameter as the child to ensure
// plugins or routes installed in the plugin are ordered
result = pluginBody().getParameter(0)
result = this.pluginBody().getParameter(0)
or
// If we can't see the plugin body, just use the plugin expression so we can
// check if something is guarded by that plugin.
not exists(pluginBody()) and
result = getArgument(0)
not exists(this.pluginBody()) and
result = this.getArgument(0)
)
}
}

2
javascript/ql/lib/semmle/javascript/frameworks/HTTP.qll
View File

@@ -101,7 +101,7 @@ module HTTP {
* Holds if this kind of HTTP request should not generally be considered free of side effects,
* such as for `POST` or `PUT` requests.
*/
predicate isUnsafe() { not isSafe() }
predicate isUnsafe() { not this.isSafe() }
}
/**

4
javascript/ql/lib/semmle/javascript/frameworks/Templating.qll
View File

@@ -161,12 +161,12 @@ module Templating {
/** Gets a data flow node corresponding to a use of the given template variable within this top-level. */
DataFlow::SourceNode getAnAccessPathUse(string accessPath) {
result = getAVariableUse(accessPath)
result = this.getAVariableUse(accessPath)
or
exists(string varName, string suffix |
accessPath = varName + "." + suffix and
suffix != "" and
result = AccessPath::getAReferenceTo(getAVariableUse(varName), suffix)
result = AccessPath::getAReferenceTo(this.getAVariableUse(varName), suffix)
)
}
}

2
javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll
View File

@@ -47,7 +47,7 @@ abstract class ExpensiveRouteHandler extends DataFlow::Node {
*/
deprecated class RateLimitedRouteHandlerExpr extends Express::RouteHandlerExpr {
RateLimitedRouteHandlerExpr() {
Routing::getNode(flow()).isGuardedBy(any(RateLimitingMiddleware m))
Routing::getNode(this.flow()).isGuardedBy(any(RateLimitingMiddleware m))
}
}