Merge branch 'master' into rdmarsh/cpp/malloc-alias-locations

Author: rdmarsh2

change-notes/1.24/analysis-javascript.md

@@ -43,6 +43,7 @@
 | Cross-site scripting through exception (`js/xss-through-exception`) | security, external/cwe/cwe-079, external/cwe/cwe-116              | Highlights potential XSS vulnerabilities where an exception is written to the DOM. Results are not shown on LGTM by default. |
 | Regular expression always matches (`js/regex/always-matches`) | correctness, regular-expressions | Highlights regular expression checks that trivially succeed by matching an empty substring. Results are shown on LGTM by default. |
 | Missing await (`js/missing-await`) | correctness | Highlights expressions that operate directly on a promise object in a nonsensical way, instead of awaiting its result. Results are shown on LGTM by default. |
+| Polynomial regular expression used on uncontrolled data (`js/polynomial-redos`) | security, external/cwe/cwe-730, external/cwe/cwe-400 | Highlights expensive regular expressions that may be used on malicious input. Results are shown on LGTM by default. | 
 | Prototype pollution in utility function (`js/prototype-pollution-utility`) | security, external/cwe/cwe-400, external/cwe/cwe-471 | Highlights recursive copying operations that are susceptible to prototype pollution. Results are shown on LGTM by default. |
 | Unsafe jQuery plugin (`js/unsafe-jquery-plugin`) | Highlights potential XSS vulnerabilities in unsafely designed jQuery plugins. Results are shown on LGTM by default. |
 

javascript/ql/src/AngularJS/DependencyMismatch.ql

@@ -24,8 +24,9 @@ where
     exists(string n | p = f.getDependencyParameter(n) |
       p.getName() != n and
       exists(f.getDependencyParameter(p.getName())) and
-      msg = "This parameter is named '" + p.getName() + "', " +
-          "but actually refers to dependency '" + n + "'."
+      msg =
+        "This parameter is named '" + p.getName() + "', " + "but actually refers to dependency '" +
+          n + "'."
     )
   )
 select p, msg

javascript/ql/src/AngularJS/DoubleCompilation.ql

@@ -15,7 +15,8 @@ import javascript
 from AngularJS::ServiceReference compile, SimpleParameter elem, CallExpr c
 where
   compile.getName() = "$compile" and
-  elem = any(AngularJS::CustomDirective d)
+  elem =
+    any(AngularJS::CustomDirective d)
         .getALinkFunction()
         .(AngularJS::LinkFunction)
         .getElementParameter() and

javascript/ql/src/AngularJS/IncompatibleService.ql

@@ -130,7 +130,8 @@ where
   kind = getServiceKind(request, name) and
   exists(request.getAServiceDefinition(name)) and // ignore unknown/undefined services
   not isCompatibleRequestedService(request, kind) and
-  compatibleWithString = concat(string compatibleKind |
+  compatibleWithString =
+    concat(string compatibleKind |
       isCompatibleRequestedService(request, compatibleKind) and
       not isWildcardKind(compatibleKind)
     |

javascript/ql/src/AngularJS/UnusedAngularDependency.ql

@@ -34,8 +34,9 @@ predicate isMissingParameter(AngularJS::InjectableFunction f, string msg, ASTNod
         then dependenciesAreString = "dependency is"
         else dependenciesAreString = "dependencies are"
       ) and
-      msg = "This function has " + paramCount + " " + parametersString + ", but " + injectionCount +
-          " " + dependenciesAreString + " injected into it."
+      msg =
+        "This function has " + paramCount + " " + parametersString + ", but " + injectionCount + " "
+          + dependenciesAreString + " injected into it."
     )
   )
 }

javascript/ql/src/Declarations/Declarations.qll

@@ -31,7 +31,8 @@ VarRef refInContainer(Variable var, RefKind kind, StmtContainer sc) {
  * declaration of `var` (if `kind` is `Decl()`) in `sc`.
  */
 VarRef firstRefInContainer(Variable var, RefKind kind, StmtContainer sc) {
-  result = min(refInContainer(var, kind, sc) as ref
+  result =
+    min(refInContainer(var, kind, sc) as ref
       order by
         ref.getLocation().getStartLine(), ref.getLocation().getStartColumn()
     )
@@ -52,7 +53,8 @@ VarRef refInTopLevel(Variable var, RefKind kind, TopLevel tl) {
  * declaration of `var` (if `kind` is `Decl()`) in `tl`.
  */
 VarRef firstRefInTopLevel(Variable var, RefKind kind, TopLevel tl) {
-  result = min(refInTopLevel(var, kind, tl) as ref
+  result =
+    min(refInTopLevel(var, kind, tl) as ref
       order by
         ref.getLocation().getStartLine(), ref.getLocation().getStartColumn()
     )

javascript/ql/src/Declarations/MissingVarDecl.ql

@@ -57,7 +57,8 @@ GlobalVarAccess getAccessIn(GlobalVariable v, Function f) {
  * Gets the (lexically) first access to variable `v` in function `f`.
  */
 GlobalVarAccess getFirstAccessIn(GlobalVariable v, Function f) {
-  result = min(getAccessIn(v, f) as gva
+  result =
+    min(getAccessIn(v, f) as gva
       order by
         gva.getLocation().getStartLine(), gva.getLocation().getStartColumn()
     )

javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.ql

@@ -1,7 +1,7 @@
 /**
  * @name Suspicious method name declaration
- * @description A method declaration with a name that is a special keyword in another 
- *              context is suspicious. 
+ * @description A method declaration with a name that is a special keyword in another
+ *              context is suspicious.
  * @kind problem
  * @problem.severity warning
  * @id js/suspicious-method-name-declaration
@@ -19,7 +19,7 @@ import javascript
 predicate isSuspiciousMethodName(string name, ClassOrInterface container) {
   name = "function"
   or
-  // "constructor" is only suspicious outside a class.  
+  // "constructor" is only suspicious outside a class.
   name = "constructor" and not container instanceof ClassDefinition
   or
   // "new" is only suspicious inside a class.
@@ -31,39 +31,41 @@ where
   container.getLocation().getFile().getFileType().isTypeScript() and
   container.getMember(name) = member and
   isSuspiciousMethodName(name, container) and
-  
   // Cases to ignore.
   not (
     // Assume that a "new" method is intentional if the class has an explicit constructor.
     name = "new" and
     container instanceof ClassDefinition and
     exists(ConstructorDeclaration constructor |
       container.getMember("constructor") = constructor and
-      not constructor.isSynthetic() 
-    ) 
+      not constructor.isSynthetic()
+    )
     or
     // Explicitly declared static methods are fine.
     container instanceof ClassDefinition and
     member.isStatic()
     or
-    // Only looking for declared methods. Methods with a body are OK. 
+    // Only looking for declared methods. Methods with a body are OK.
     exists(member.getBody().getBody())
     or
     // The developer was not confused about "function" when there are other methods in the interface.
-    name = "function" and 
+    name = "function" and
     exists(MethodDeclaration other | other = container.getAMethod() |
       other.getName() != "function" and
       not other.(ConstructorDeclaration).isSynthetic()
     )
-  )
-  
-  and
-  
+  ) and
   (
-    name = "constructor" and msg = "The member name 'constructor' does not declare a constructor in interfaces, but it does in classes." 
+    name = "constructor" and
+    msg =
+      "The member name 'constructor' does not declare a constructor in interfaces, but it does in classes."
     or
-    name = "new" and msg = "The member name 'new' does not declare a constructor, but 'constructor' does in class declarations."
+    name = "new" and
+    msg =
+      "The member name 'new' does not declare a constructor, but 'constructor' does in class declarations."
     or
-    name = "function" and msg = "The member name 'function' does not declare a function, it declares a method named 'function'."
+    name = "function" and
+    msg =
+      "The member name 'function' does not declare a function, it declares a method named 'function'."
   )
 select member, msg

javascript/ql/src/Declarations/UnreachableMethodOverloads.ql

@@ -44,43 +44,39 @@ string getKind(MemberDeclaration m) {
  * A call-signature that originates from a MethodSignature in the AST.
  */
 private class MethodCallSig extends CallSignatureType {
-    string name;
-    
-    MethodCallSig() {
-        exists(MethodSignature sig | 
-            this = sig.getBody().getCallSignature() and
-            name = sig.getName()
-        )
-    }
-    
-    /**
-     * Gets the name of any member that has this signature.
-     */
-    string getName() {
-        result = name
-    }
+  string name;
+
+  MethodCallSig() {
+    exists(MethodSignature sig |
+      this = sig.getBody().getCallSignature() and
+      name = sig.getName()
+    )
+  }
+
+  /**
+   * Gets the name of any member that has this signature.
+   */
+  string getName() { result = name }
 }
 
 /**
  * Holds if the two call signatures could be overloads of each other and have the same parameter types.
  */
 predicate matchingCallSignature(MethodCallSig method, MethodCallSig other) {
-  method.getName() = other.getName() and 
-  
+  method.getName() = other.getName() and
   method.getNumOptionalParameter() = other.getNumOptionalParameter() and
   method.getNumParameter() = other.getNumParameter() and
   method.getNumRequiredParameter() = other.getNumRequiredParameter() and
-  // purposely not looking at number of type arguments. 
-  
-  method.getKind() = other.getKind() and 
-  
-  
-  forall(int i | i in [0 .. -1 + method.getNumParameter()] | 
+  // purposely not looking at number of type arguments.
+  method.getKind() = other.getKind() and
+  forall(int i | i in [0 .. -1 + method.getNumParameter()] |
     method.getParameter(i) = other.getParameter(i) // This is sometimes imprecise, so it is still a good idea to compare type annotations.
-  ) and 
-  
-  // shared type parameters are equal. 
-  forall(int i | i in [0 .. -1 + min(int num | num = method.getNumTypeParameter() or num = other.getNumTypeParameter())] |
+  ) and
+  // shared type parameters are equal.
+  forall(int i |
+    i in [0 .. -1 +
+          min(int num | num = method.getNumTypeParameter() or num = other.getNumTypeParameter())]
+  |
     method.getTypeParameterBound(i) = other.getTypeParameterBound(i)
   )
 }
@@ -89,7 +85,7 @@ predicate matchingCallSignature(MethodCallSig method, MethodCallSig other) {
  * Gets which overload index the MethodSignature has among the overloads of the same name.
  */
 int getOverloadIndex(MethodSignature sig) {
-    sig.getDeclaringType().getMethodOverload(sig.getName(), result) = sig
+  sig.getDeclaringType().getMethodOverload(sig.getName(), result) = sig
 }
 
 /**
@@ -100,41 +96,34 @@ predicate signaturesMatch(MethodSignature method, MethodSignature other) {
   method.getDeclaringType() = other.getDeclaringType() and
   // same static modifier.
   getKind(method) = getKind(other) and
-
   // same name.
   method.getName() = other.getName() and
-
   // same number of parameters.
   method.getBody().getNumParameter() = other.getBody().getNumParameter() and
-  
   // The types are compared in matchingCallSignature. This is sanity-check that the textual representation of the type-annotations are somewhat similar.
   forall(int i | i in [0 .. -1 + method.getBody().getNumParameter()] |
-    getParameterTypeAnnotation(method, i) = getParameterTypeAnnotation(other, i)   
+    getParameterTypeAnnotation(method, i) = getParameterTypeAnnotation(other, i)
   ) and
-  
   matchingCallSignature(method.getBody().getCallSignature(), other.getBody().getCallSignature())
 }
 
 from ClassOrInterface decl, string name, MethodSignature previous, MethodSignature unreachable
 where
   previous = decl.getMethod(name) and
   unreachable = getOtherMatchingSignatures(previous) and
-  
   // If the method is part of inheritance between classes/interfaces, then there can sometimes be reasons for having this pattern.
-  not exists(decl.getASuperTypeDeclaration().getMethod(name)) and 
-  not exists(ClassOrInterface sub | 
+  not exists(decl.getASuperTypeDeclaration().getMethod(name)) and
+  not exists(ClassOrInterface sub |
     decl = sub.getASuperTypeDeclaration() and
     exists(sub.getMethod(name))
   ) and
-  
-  
   // If a later method overload has more type parameters, then that overload can be selected by explicitly declaring the type arguments at the callsite.
   // This comparison removes those cases.
   unreachable.getBody().getNumTypeParameter() <= previous.getBody().getNumTypeParameter() and
-  
   // We always select the first of the overloaded methods.
   not exists(MethodSignature later | later = getOtherMatchingSignatures(previous) |
     getOverloadIndex(later) < getOverloadIndex(previous)
   )
 select unreachable,
-  "This overload of " + name + "() is unreachable, the $@ overload will always be selected.", previous, "previous"
+  "This overload of " + name + "() is unreachable, the $@ overload will always be selected.",
+  previous, "previous"

javascript/ql/src/Declarations/UnstableCyclicImport.ql

@@ -74,7 +74,8 @@ class CandidateVarAccess extends VarAccess {
  * We use this to avoid duplicate alerts about the same underlying cyclic import.
  */
 VarAccess getFirstCandidateAccess(ImportDeclaration decl) {
-  result = min(decl.getASpecifier().getLocal().getVariable().getAnAccess().(CandidateVarAccess) as p
+  result =
+    min(decl.getASpecifier().getLocal().getVariable().getAnAccess().(CandidateVarAccess) as p
       order by
         p.getFirstToken().getIndex()
     )
@@ -133,14 +134,15 @@ string pathToModule(Module source, Module destination, int steps) {
     steps > 1 and
     exists(Module next |
       // Only extend the path to one of the potential successors, as we only need one example.
-      next = min(Module mod |
+      next =
+        min(Module mod |
           isImportedAtRuntime(source, mod) and
           numberOfStepsToModule(mod, destination, steps - 1)
         |
           mod order by mod.getName()
         ) and
-      result = repr(getARuntimeImport(source, next)) + " => " +
-          pathToModule(next, destination, steps - 1)
+      result =
+        repr(getARuntimeImport(source, next)) + " => " + pathToModule(next, destination, steps - 1)
     )
   )
 }