From a1c5724be7bc06f7b59afe2b965028b3cd346d29 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Fri, 11 Feb 2022 17:57:37 +0100 Subject: [PATCH] fix most ql-for-ql warnings in JS --- .../dataflow/BackendIdor/BackendIdor.ql | 4 +- .../InformationDisclosure.ql | 3 +- .../queries/dataflow/StoredXss/StoredXss.ql | 2 +- .../StoredXss/StoredXssTypeTracking.ql | 2 +- .../adaptivethreatmodeling/BaseScoring.qll | 2 +- .../javascript/DefensiveProgramming.qll | 2 +- .../ql/lib/semmle/javascript/Routing.qll | 2 +- .../ql/lib/semmle/javascript/TypeScript.qll | 9 +- .../javascript/dataflow/TaintTracking.qll | 43 ++---- .../javascript/dataflow/TypeInference.qll | 2 +- .../internal/InterModuleTypeInference.qll | 5 +- .../internal/VariableTypeInference.qll | 12 +- .../AngularJS/DependencyInjections.qll | 2 +- .../AngularJS/ServiceDefinitions.qll | 128 ++++++++---------- .../semmle/javascript/frameworks/Cheerio.qll | 2 +- .../semmle/javascript/frameworks/Redux.qll | 4 +- .../semmle/javascript/frameworks/SocketIO.qll | 2 +- .../javascript/security/TaintedObject.qll | 2 +- .../ClientSideUrlRedirectCustomizations.qll | 1 + .../security/dataflow/MissingRateLimiting.qll | 2 +- ...otypePollutingAssignmentCustomizations.qll | 2 +- .../PrototypePollutionCustomizations.qll | 2 +- .../UnsafeCodeConstructionCustomizations.qll | 2 +- ...nsafeDynamicMethodAccessCustomizations.qll | 2 +- .../AngularJS/DeadAngularJSEventListener.ql | 32 ++--- .../ql/src/AngularJS/IncompatibleService.ql | 2 +- javascript/ql/src/Comments/CommentedOut.qll | 7 +- .../SuspiciousMethodNameDeclaration.ql | 2 +- .../experimental/Security/CWE-918/SSRF.qll | 2 + 29 files changed, 117 insertions(+), 167 deletions(-) diff --git a/javascript/ql/examples/queries/dataflow/BackendIdor/BackendIdor.ql b/javascript/ql/examples/queries/dataflow/BackendIdor/BackendIdor.ql index 3f7e555738e..3842475ecf8 100644 --- a/javascript/ql/examples/queries/dataflow/BackendIdor/BackendIdor.ql +++ b/javascript/ql/examples/queries/dataflow/BackendIdor/BackendIdor.ql @@ -13,7 +13,7 @@ import DataFlow import DataFlow::PathGraph /** - * Tracks user-controlled values into a 'userId' property sent to a backend service. + * A taint-tracking configuration that tracks user-controlled values into a 'userId' property sent to a backend service. */ class IdorTaint extends TaintTracking::Configuration { IdorTaint() { this = "IdorTaint" } @@ -34,7 +34,7 @@ class IdorTaint extends TaintTracking::Configuration { } /** - * Sanitize values that have succesfully been compared to another value. + * A sanitizer for values that have succesfully been compared to another value. */ class EqualityGuard extends TaintTracking::SanitizerGuardNode, ValueNode { override EqualityTest astNode; diff --git a/javascript/ql/examples/queries/dataflow/InformationDisclosure/InformationDisclosure.ql b/javascript/ql/examples/queries/dataflow/InformationDisclosure/InformationDisclosure.ql index 07a942967b1..1fe76a178e2 100644 --- a/javascript/ql/examples/queries/dataflow/InformationDisclosure/InformationDisclosure.ql +++ b/javascript/ql/examples/queries/dataflow/InformationDisclosure/InformationDisclosure.ql @@ -13,7 +13,8 @@ import DataFlow import DataFlow::PathGraph /** - * Tracks authentication tokens ("authKey") to a postMessage call with unrestricted target origin. + * A dataflow configuration that tracks authentication tokens ("authKey") + * to a postMessage call with unrestricted target origin. * * For example: * ``` diff --git a/javascript/ql/examples/queries/dataflow/StoredXss/StoredXss.ql b/javascript/ql/examples/queries/dataflow/StoredXss/StoredXss.ql index c18266451f8..c31095d4995 100644 --- a/javascript/ql/examples/queries/dataflow/StoredXss/StoredXss.ql +++ b/javascript/ql/examples/queries/dataflow/StoredXss/StoredXss.ql @@ -12,7 +12,7 @@ import semmle.javascript.security.dataflow.StoredXssQuery import DataFlow::PathGraph /** - * Data returned from a MySQL query, such as the `data` parameter in this example: + * The data returned from a MySQL query, such as the `data` parameter in this example: * ``` * let mysql = require('mysql'); * let connection = mysql.createConnection(); diff --git a/javascript/ql/examples/queries/dataflow/StoredXss/StoredXssTypeTracking.ql b/javascript/ql/examples/queries/dataflow/StoredXss/StoredXssTypeTracking.ql index c68541717f1..5309fac6900 100644 --- a/javascript/ql/examples/queries/dataflow/StoredXss/StoredXssTypeTracking.ql +++ b/javascript/ql/examples/queries/dataflow/StoredXss/StoredXssTypeTracking.ql @@ -28,7 +28,7 @@ DataFlow::SourceNode mysqlConnection(DataFlow::TypeTracker t) { DataFlow::SourceNode mysqlConnection() { result = mysqlConnection(DataFlow::TypeTracker::end()) } /** - * Data returned from a MySQL query. + * The data returned from a MySQL query. * * For example: * ``` diff --git a/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/BaseScoring.qll b/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/BaseScoring.qll index 5ef517e09ec..1af5966997a 100644 --- a/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/BaseScoring.qll +++ b/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/BaseScoring.qll @@ -15,7 +15,7 @@ external predicate availableMlModels( ATMConfig getCfg() { any() } /** - * Scoring information produced by a scoring model. + * A string containing scoring information produced by a scoring model. * * Scoring models include embedding models and endpoint scoring models. */ diff --git a/javascript/ql/lib/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/lib/semmle/javascript/DefensiveProgramming.qll index f00e6420c61..677e931fa25 100644 --- a/javascript/ql/lib/semmle/javascript/DefensiveProgramming.qll +++ b/javascript/ql/lib/semmle/javascript/DefensiveProgramming.qll @@ -188,7 +188,7 @@ module DefensiveExpressionTest { } /** - * Comparison against `undefined`, such as `x === undefined`. + * A comparison against `undefined`, such as `x === undefined`. */ class UndefinedComparison extends NullUndefinedComparison { UndefinedComparison() { op2type = TTUndefined() } diff --git a/javascript/ql/lib/semmle/javascript/Routing.qll b/javascript/ql/lib/semmle/javascript/Routing.qll index 61300f11a42..ad75132f9f9 100644 --- a/javascript/ql/lib/semmle/javascript/Routing.qll +++ b/javascript/ql/lib/semmle/javascript/Routing.qll @@ -652,7 +652,7 @@ module Routing { */ module Router { /** - * Creation of a mutable router object. + * The creation of a mutable router object. */ abstract class Range extends DataFlow::Node { /** Gets a reference to this router. */ diff --git a/javascript/ql/lib/semmle/javascript/TypeScript.qll b/javascript/ql/lib/semmle/javascript/TypeScript.qll index d0e6d351fe1..b32e7b13021 100644 --- a/javascript/ql/lib/semmle/javascript/TypeScript.qll +++ b/javascript/ql/lib/semmle/javascript/TypeScript.qll @@ -1679,7 +1679,7 @@ class EnumScope extends @enum_scope, Scope { } /** - * Scope induced by a declaration of form `declare module "X" {...}`. + * A scope induced by a declaration of form `declare module "X" {...}`. */ class ExternalModuleScope extends @external_module_scope, Scope { override string toString() { result = "external module scope" } @@ -2836,12 +2836,7 @@ class ConstructorCallSignatureType extends CallSignatureType, @constructor_signa private class PromiseTypeName extends TypeName { PromiseTypeName() { // The name must suggest it is a promise. - exists(string name | name = this.getName() | - name.matches("%Promise") or - name.matches("%PromiseLike") or - name.matches("%Thenable") or - name.matches("%Deferred") - ) and + this.getName().matches(["%Promise", "%PromiseLike", "%Thenable", "%Deferred"]) and // The `then` method should take a callback, taking an argument of type `T`. exists(TypeReference self, Type thenMethod | self = this.getType() | self.getNumTypeArgument() = 1 and diff --git a/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll b/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll index 517ef9c2bd5..e199e585f17 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll @@ -635,39 +635,18 @@ module TaintTracking { pred.asExpr() = succ.getAstNode().(MethodCallExpr).getReceiver() and ( // sorted, interesting, properties of String.prototype - name = "anchor" or - name = "big" or - name = "blink" or - name = "bold" or - name = "concat" or - name = "fixed" or - name = "fontcolor" or - name = "fontsize" or - name = "italics" or - name = "link" or - name = "padEnd" or - name = "padStart" or - name = "repeat" or - name = "replace" or - name = "replaceAll" or - name = "slice" or - name = "small" or - name = "split" or - name = "strike" or - name = "sub" or - name = "substr" or - name = "substring" or - name = "sup" or - name = "toLocaleLowerCase" or - name = "toLocaleUpperCase" or - name = "toLowerCase" or - name = "toUpperCase" or - name = "trim" or - name = "trimLeft" or - name = "trimRight" or + name = + [ + "anchor", "big", "blink", "bold", "concat", "fixed", "fontcolor", "fontsize", + "italics", "link", "padEnd", "padStart", "repeat", "replace", "replaceAll", "slice", + "small", "split", "strike", "sub", "substr", "substring", "sup", + "toLocaleLowerCase", "toLocaleUpperCase", "toLowerCase", "toUpperCase", "trim", + "trimLeft", "trimRight" + ] + or // sorted, interesting, properties of Object.prototype - name = "toString" or - name = "valueOf" or + name = ["toString", "valueOf"] + or // sorted, interesting, properties of Array.prototype name = "join" ) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/TypeInference.qll b/javascript/ql/lib/semmle/javascript/dataflow/TypeInference.qll index 801997e026a..35b8f9b4444 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/TypeInference.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/TypeInference.qll @@ -230,7 +230,7 @@ class AnalyzedModule extends TopLevel { } /** - * Flow analysis for functions. + * A function for which analysis results are available. */ class AnalyzedFunction extends DataFlow::AnalyzedValueNode { override Function astNode; diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll index 4986164f859..0704955bfb5 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll @@ -206,7 +206,7 @@ private class AnalyzedDestructuredImport extends AnalyzedPropertyRead { } /** - * Flow analysis for `require` calls, interpreted as an implicit read of + * A call to `require`, interpreted as an implicit read of * the `module.exports` property of the imported module. */ class AnalyzedRequireCall extends AnalyzedPropertyRead, DataFlow::ValueNode { @@ -221,7 +221,8 @@ class AnalyzedRequireCall extends AnalyzedPropertyRead, DataFlow::ValueNode { } /** - * Flow analysis for special TypeScript `require` calls in an import-assignment. + * A special TypeScript `require` call in an import-assignment, + * interpreted as an implicit of the `module.exports` property of the imported module. */ class AnalyzedExternalModuleReference extends AnalyzedPropertyRead, DataFlow::ValueNode { Module required; diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/VariableTypeInference.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/VariableTypeInference.qll index e4a7430a655..37804e4c26f 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/VariableTypeInference.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/VariableTypeInference.qll @@ -75,7 +75,7 @@ private class AnalyzedSsaVariableUseWithNonLocalFlow extends AnalyzedValueNode { } /** - * Flow analysis for `VarDef`s. + * A vardef with helper predicates for flow analysis. */ class AnalyzedVarDef extends VarDef { /** @@ -184,7 +184,7 @@ private class AnalyzedAmdParameter extends AnalyzedVarDef { } /** - * Flow analysis for SSA definitions. + * An SSA definitions that has been analyzed. */ abstract class AnalyzedSsaDefinition extends SsaDefinition { /** @@ -237,7 +237,7 @@ private class AnalyzedPhiNode extends AnalyzedSsaDefinition, SsaPhiNode { } /** - * Flow analysis for refinement nodes. + * An analyzed refinement node. */ class AnalyzedRefinement extends AnalyzedSsaDefinition, SsaRefinementNode { override AbstractValue getAnRhsValue() { @@ -254,7 +254,7 @@ class AnalyzedRefinement extends AnalyzedSsaDefinition, SsaRefinementNode { } /** - * Flow analysis for refinement nodes where the guard is a condition. + * A refinement node where the guard is a condition. * * For such nodes, we want to split any indefinite abstract values flowing into the node * into sets of more precise abstract values to enable them to be refined. @@ -272,7 +272,7 @@ class AnalyzedConditionGuard extends AnalyzedRefinement { } /** - * Flow analysis for condition guards with an outcome of `true`. + * A refinement for a condition guard with an outcome of `true`. * * For example, in `if(x) s; else t;`, this will restrict the possible values of `x` at * the beginning of `s` to those that are truthy. @@ -290,7 +290,7 @@ class AnalyzedPositiveConditionGuard extends AnalyzedRefinement { } /** - * Flow analysis for condition guards with an outcome of `false`. + * A refinement for a condition guard with an outcome of `false`. * * For example, in `if(x) s; else t;`, this will restrict the possible values of `x` at * the beginning of `t` to those that are falsy. diff --git a/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/DependencyInjections.qll b/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/DependencyInjections.qll index 4919490f52d..01be5035325 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/DependencyInjections.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/DependencyInjections.qll @@ -26,7 +26,7 @@ class InjectorInvokeCall extends DataFlow::CallNode, DependencyInjection { } /** - * Base class for expressions that dependency-inject some of their input with AngularJS dependency injection services. + * An expression that dependency-inject some of its input with AngularJS dependency injection services. */ abstract class DependencyInjection extends DataFlow::ValueNode { /** diff --git a/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/ServiceDefinitions.qll b/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/ServiceDefinitions.qll index fe7885b435e..dcce784cd1a 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/ServiceDefinitions.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/ServiceDefinitions.qll @@ -126,67 +126,49 @@ private string getBuiltinKind(string name) { result = "service" and ( // ng - name = "$anchorScroll" or - name = "$animate" or - name = "$animateCss" or - name = "$cacheFactory" or - name = "$controller" or - name = "$document" or - name = "$exceptionHandler" or - name = "$filter" or - name = "$http" or - name = "$httpBackend" or - name = "$httpParamSerializer" or - name = "$httpParamSerializerJQLike" or - name = "$interpolate" or - name = "$interval" or - name = "$jsonpCallbacks" or - name = "$locale" or - name = "$location" or - name = "$log" or - name = "$parse" or - name = "$q" or - name = "$rootElement" or - name = "$rootScope" or - name = "$sce" or - name = "$sceDelegate" or - name = "$templateCache" or - name = "$templateRequest" or - name = "$timeout" or - name = "$window" or - name = "$xhrFactory" or + name = + [ + "$anchorScroll", "$animate", "$animateCss", "$cacheFactory", "$controller", "$document", + "$exceptionHandler", "$filter", "$http", "$httpBackend", "$httpParamSerializer", + "$httpParamSerializerJQLike", "$interpolate", "$interval", "$jsonpCallbacks", "$locale", + "$location", "$log", "$parse", "$q", "$rootElement", "$rootScope", "$sce", "$sceDelegate", + "$templateCache", "$templateRequest", "$timeout", "$window", "$xhrFactory" + ] + or // auto - name = "$injector" or - name = "$provide" or + name = ["$injector", "$provide"] + or // ngAnimate - name = "$animate" or - name = "$animateCss" or + name = ["$animate", "$animateCss"] + or // ngAria - name = "$aria" or + name = "$aria" + or // ngComponentRouter - name = "$rootRouter" or - name = "$routerRootComponent" or + name = ["$rootRouter", "$routerRootComponent"] + or // ngCookies - name = "$cookieStore" or - name = "$cookies" or + name = ["$cookieStore", "$cookies"] + or //ngMock - name = "$animate" or - name = "$componentController" or - name = "$controller" or - name = "$exceptionHandler" or - name = "$httpBackend" or - name = "$interval" or - name = "$log" or - name = "$timeout" or + name = + [ + "$animate", "$componentController", "$controller", "$exceptionHandler", "$httpBackend", + "$interval", "$log", "$timeout" + ] + or //ngMockE2E - name = "$httpBackend" or + name = "$httpBackend" + or // ngResource - name = "$resource" or + name = "$resource" + or // ngRoute - name = "$route" or - name = "$routeParams" or + name = ["$route", "$routeParams"] + or // ngSanitize - name = "$sanitize" or + name = "$sanitize" + or // ngTouch name = "$swipe" ) @@ -194,32 +176,29 @@ private string getBuiltinKind(string name) { result = "provider" and ( // ng - name = "$anchorScrollProvider" or - name = "$animateProvider" or - name = "$compileProvider" or - name = "$controllerProvider" or - name = "$filterProvider" or - name = "$httpProvider" or - name = "$interpolateProvider" or - name = "$locationProvider" or - name = "$logProvider" or - name = "$parseProvider" or - name = "$provider" or - name = "$qProvider" or - name = "$rootScopeProvider" or - name = "$sceDelegateProvider" or - name = "$sceProvider" or - name = "$templateRequestProvider" or + name = + [ + "$anchorScrollProvider", "$animateProvider", "$compileProvider", "$controllerProvider", + "$filterProvider", "$httpProvider", "$interpolateProvider", "$locationProvider", + "$logProvider", "$parseProvider", "$provider", "$qProvider", "$rootScopeProvider", + "$sceDelegateProvider", "$sceProvider", "$templateRequestProvider" + ] + or // ngAria - name = "$ariaProvider" or + name = "$ariaProvider" + or // ngCookies - name = "$cookiesProvider" or + name = "$cookiesProvider" + or // ngmock - name = "$exceptionHandlerProvider" or + name = "$exceptionHandlerProvider" + or // ngResource - name = "$resourceProvider" or + name = "$resourceProvider" + or // ngRoute - name = "$routeProvider" or + name = "$routeProvider" + or // ngSanitize name = "$sanitizeProvider" ) @@ -227,9 +206,8 @@ private string getBuiltinKind(string name) { result = "type" and ( // ng - name = "$cacheFactory" or - name = "$compile" or - name = "$rootScope" or + name = ["$cacheFactory", "$compile", "$rootScope"] + or // ngMock name = "$rootScope" ) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Cheerio.qll b/javascript/ql/lib/semmle/javascript/frameworks/Cheerio.qll index c3d02749a21..ddd446098e4 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Cheerio.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Cheerio.qll @@ -25,7 +25,7 @@ module Cheerio { module CheerioObjectCreation { /** - * Creation of a `cheerio` object. + * The creation of a `cheerio` object. */ abstract class Range extends DataFlow::SourceNode { } diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Redux.qll b/javascript/ql/lib/semmle/javascript/frameworks/Redux.qll index 375fcefac2e..1495cd489f7 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Redux.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Redux.qll @@ -73,7 +73,7 @@ module Redux { /** Companion module to the `StoreCreation` class. */ module StoreCreation { /** - * Creation of a redux store. Additional `StoreCreation` instances can be generated by subclassing this class. + * The creation of a redux store. Additional `StoreCreation` instances can be generated by subclassing this class. */ abstract class Range extends DataFlow::SourceNode { /** Gets the data flow node holding the root reducer for this store. */ @@ -135,7 +135,7 @@ module Redux { } /** - * Creation of a reducer function that delegates to one or more other reducer functions. + * The creation of a reducer function that delegates to one or more other reducer functions. * * Delegating reducers can delegate specific parts of the state object (`getStateHandlerArg`), * actions of a specific type (`getActionHandlerArg`), or everything (`getAPlainHandlerArg`). diff --git a/javascript/ql/lib/semmle/javascript/frameworks/SocketIO.qll b/javascript/ql/lib/semmle/javascript/frameworks/SocketIO.qll index 205644f883f..ff55e86e219 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/SocketIO.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/SocketIO.qll @@ -59,7 +59,7 @@ module SocketIO { private API::Node server() { result = node or - exists(API::Node pred | pred = server() | + exists(API::Node pred | pred = this.server() | // invocation of a chainable method exists(API::CallNode mcn, string m | m = "adapter" or diff --git a/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll b/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll index 664d3559c1f..6c5c0f21057 100644 --- a/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll +++ b/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll @@ -79,7 +79,7 @@ module TaintedObject { } /** - * Sanitizer guard that blocks deep object taint. + * A sanitizer guard that blocks deep object taint. */ abstract class SanitizerGuard extends TaintTracking::LabeledSanitizerGuardNode { } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll index 1452d60e70a..76b64b3e944 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll @@ -120,6 +120,7 @@ module ClientSideUrlRedirect { } /** + * The first argument to a call to `openExternal` seen as a sink for unvalidated URL redirection. * Improper use of openExternal can be leveraged to compromise the user's host. * When openExternal is used with untrusted content, it can be leveraged to execute arbitrary commands. */ diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll index 94e70f34a19..fc30c91018a 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll @@ -128,7 +128,7 @@ deprecated class RateLimiter extends Express::RouteHandlerExpr { } /** - * Creation of a middleware function that acts as a rate limiter. + * The creation of a middleware function that acts as a rate limiter. */ abstract class RateLimitingMiddleware extends DataFlow::SourceNode { /** Gets a data flow node referring to this middleware. */ diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll index f7868c290aa..f7902caa9fe 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll @@ -38,7 +38,7 @@ module PrototypePollutingAssignment { */ abstract class Sanitizer extends DataFlow::Node { } - /** Flow label representing the `Object.prototype` value. */ + /** A flow label representing the `Object.prototype` value. */ abstract class ObjectPrototype extends DataFlow::FlowLabel { ObjectPrototype() { this = "Object.prototype" } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionCustomizations.qll index a9b3c3e3282..8a4726030a1 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionCustomizations.qll @@ -11,7 +11,7 @@ import semmle.javascript.dependencies.SemVer module PrototypePollution { /** - * Label for wrappers around tainted objects, that is, objects that are + * A label for wrappers around tainted objects, that is, objects that are * not completely user-controlled, but contain a user-controlled object. * * For example, `options` below is a tainted wrapper, but is not itself diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeCodeConstructionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeCodeConstructionCustomizations.qll index 47e56486135..9534dfda70e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeCodeConstructionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeCodeConstructionCustomizations.qll @@ -45,7 +45,7 @@ module UnsafeCodeConstruction { private DataFlow::Node isExecutedAsCode(DataFlow::TypeBackTracker t, CodeInjection::Sink codeSink) { t.start() and result = codeSink or - exists(DataFlow::TypeBackTracker t2 | t2 = t.smallstep(result, isExecutedAsCode(t, codeSink))) + exists(DataFlow::TypeBackTracker t2 | t2 = t.smallstep(result, isExecutedAsCode(t2, codeSink))) } /** diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll index 86c4a8417f8..9fed42d5bdd 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll @@ -44,7 +44,7 @@ module UnsafeDynamicMethodAccess { UnsafeFunction unsafeFunction() { any() } /** - * Flow label describing values that may refer to an unsafe + * A flow label describing values that may refer to an unsafe * function as a result of an attacker-controlled property name. */ abstract class UnsafeFunction extends DataFlow::FlowLabel { diff --git a/javascript/ql/src/AngularJS/DeadAngularJSEventListener.ql b/javascript/ql/src/AngularJS/DeadAngularJSEventListener.ql index 2fb5881723f..36a90784275 100644 --- a/javascript/ql/src/AngularJS/DeadAngularJSEventListener.ql +++ b/javascript/ql/src/AngularJS/DeadAngularJSEventListener.ql @@ -16,28 +16,26 @@ import javascript */ predicate isABuiltinEventName(string name) { // $rootScope.Scope - name = "$destroy" or + name = "$destroy" + or // $location - name = "$locationChangeStart" or - name = "$locationChangeSuccess" or + name = ["$locationChangeStart", "$locationChangeSuccess"] + or // ngView - name = "$viewContentLoaded" or + name = "$viewContentLoaded" + or // angular-ui/ui-router - name = "$stateChangeStart" or - name = "$stateNotFound" or - name = "$stateChangeSuccess" or - name = "$stateChangeError" or - name = "$viewContentLoading " or - name = "$viewContentLoaded " or + name = + [ + "$stateChangeStart", "$stateNotFound", "$stateChangeSuccess", "$stateChangeError", + "$viewContentLoading ", "$viewContentLoaded " + ] + or // $route - name = "$routeChangeStart" or - name = "$routeChangeSuccess" or - name = "$routeChangeError" or - name = "$routeUpdate" or + name = ["$routeChangeStart", "$routeChangeSuccess", "$routeChangeError", "$routeUpdate"] + or // ngInclude - name = "$includeContentRequested" or - name = "$includeContentLoaded" or - name = "$includeContentError" + name = ["$includeContentRequested", "$includeContentLoaded", "$includeContentError"] } /** diff --git a/javascript/ql/src/AngularJS/IncompatibleService.ql b/javascript/ql/src/AngularJS/IncompatibleService.ql index 4fb290cd56d..c22eccc2dd4 100644 --- a/javascript/ql/src/AngularJS/IncompatibleService.ql +++ b/javascript/ql/src/AngularJS/IncompatibleService.ql @@ -64,7 +64,7 @@ predicate isWildcardKind(string kind) { * (see https://docs.angularjs.org/guide/di) */ predicate isCompatibleRequestedService(InjectableFunctionServiceRequest request, string kind) { - isWildcardKind(kind) + isWildcardKind(kind) and exists(request) or ( isServiceDirectiveOrFilterFunction(request) or diff --git a/javascript/ql/src/Comments/CommentedOut.qll b/javascript/ql/src/Comments/CommentedOut.qll index a0d00d70642..ed8c8d09f37 100644 --- a/javascript/ql/src/Comments/CommentedOut.qll +++ b/javascript/ql/src/Comments/CommentedOut.qll @@ -33,12 +33,7 @@ private string getALineOfCommentedOutCode(Comment c) { * disregarded when looking for commented-out code. */ private predicate containsCodeExample(Comment c) { - exists(string text | text = c.getText() | - text.matches("%
%
%") or - text.matches("%%%") or - text.matches("%@example%") or - text.matches("%```%") - ) + c.getText().matches(["%
%
%", "%%%", "%@example%", "%```%"]) } /** diff --git a/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.ql b/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.ql index efa824ae7b0..c185e5c4d04 100644 --- a/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.ql +++ b/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.ql @@ -17,7 +17,7 @@ import javascript * Holds if the method name on the given container is likely to be a mistake. */ predicate isSuspiciousMethodName(string name, ClassOrInterface container) { - name = "function" + name = "function" and exists(container) // suspicious in any container or // "constructor" is only suspicious outside a class. name = "constructor" and not container instanceof ClassDefinition diff --git a/javascript/ql/src/experimental/Security/CWE-918/SSRF.qll b/javascript/ql/src/experimental/Security/CWE-918/SSRF.qll index cc582534e23..c43418d453a 100644 --- a/javascript/ql/src/experimental/Security/CWE-918/SSRF.qll +++ b/javascript/ql/src/experimental/Security/CWE-918/SSRF.qll @@ -41,6 +41,8 @@ class Configuration extends TaintTracking::Configuration { } /** + * A sanitizer for ternary operators. + * * This sanitizers models the next example: * let valid = req.params.id ? Number.isInteger(req.params.id) : false * if (valid) { sink(req.params.id) }