Merge branch 'master' of github.com:Semmle/ql into js/more-fs-modules

Author: esbena

change-notes/1.24/analysis-javascript.md

@@ -44,7 +44,7 @@
 | 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. |
+| Prototype pollution in utility function (`js/prototype-pollution-utility`) | security, external/cwe/cwe-400, external/cwe/cwe-471 | Highlights recursive assignment 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. |
 | Unnecessary use of `cat` process (`js/unnecessary-use-of-cat`) | correctness, security, maintainability | Highlights command executions of `cat` where the fs API should be used instead. Results are shown on LGTM by default. |
 

cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

@@ -275,7 +275,7 @@ private predicate isChiForAllAliasedMemory(Instruction instr) {
   or
   isChiForAllAliasedMemory(instr.(ChiInstruction).getTotal())
   or
-  isChiForAllAliasedMemory(instr.(PhiInstruction).getAnInput())
+  isChiForAllAliasedMemory(instr.(PhiInstruction).getAnInputOperand().getAnyDef())
 }
 
 private predicate modelTaintToReturnValue(Function f, int parameterIn) {

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowDispatch.qll

@@ -135,6 +135,12 @@ private module VirtualDispatch {
       exists(FunctionInstruction fi |
         this.flowsFrom(DataFlow::instructionNode(fi), _) and
         result = fi.getFunctionSymbol()
+      ) and
+      (
+        this.getNumberOfArguments() <= result.getEffectiveNumberOfParameters() and
+        this.getNumberOfArguments() >= result.getEffectiveNumberOfParameters()
+        or
+        result.isVarargs()
       )
     }
   }

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll

@@ -1202,6 +1202,11 @@ class CallInstruction extends Instruction {
   final Instruction getPositionalArgument(int index) {
     result = getPositionalArgumentOperand(index).getDef()
   }
+
+  /**
+   * Gets the number of arguments of the call, including the `this` pointer, if any.
+   */
+  final int getNumberOfArguments() { result = count(this.getAnArgumentOperand()) }
 }
 
 /**

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll

@@ -1202,6 +1202,11 @@ class CallInstruction extends Instruction {
   final Instruction getPositionalArgument(int index) {
     result = getPositionalArgumentOperand(index).getDef()
   }
+
+  /**
+   * Gets the number of arguments of the call, including the `this` pointer, if any.
+   */
+  final int getNumberOfArguments() { result = count(this.getAnArgumentOperand()) }
 }
 
 /**

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll

@@ -1202,6 +1202,11 @@ class CallInstruction extends Instruction {
   final Instruction getPositionalArgument(int index) {
     result = getPositionalArgumentOperand(index).getDef()
   }
+
+  /**
+   * Gets the number of arguments of the call, including the `this` pointer, if any.
+   */
+  final int getNumberOfArguments() { result = count(this.getAnArgumentOperand()) }
 }
 
 /**

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll

@@ -1202,6 +1202,11 @@ class CallInstruction extends Instruction {
   final Instruction getPositionalArgument(int index) {
     result = getPositionalArgumentOperand(index).getDef()
   }
+
+  /**
+   * Gets the number of arguments of the call, including the `this` pointer, if any.
+   */
+  final int getNumberOfArguments() { result = count(this.getAnArgumentOperand()) }
 }
 
 /**

csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll

@@ -1202,6 +1202,11 @@ class CallInstruction extends Instruction {
   final Instruction getPositionalArgument(int index) {
     result = getPositionalArgumentOperand(index).getDef()
   }
+
+  /**
+   * Gets the number of arguments of the call, including the `this` pointer, if any.
+   */
+  final int getNumberOfArguments() { result = count(this.getAnArgumentOperand()) }
 }
 
 /**

javascript/ql/src/Declarations/UnreachableMethodOverloads.ql

@@ -100,6 +100,13 @@ predicate signaturesMatch(MethodSignature method, MethodSignature other) {
   method.getName() = other.getName() and
   // same number of parameters.
   method.getBody().getNumParameter() = other.getBody().getNumParameter() and
+  // same this parameter (if exists)
+  (
+    not exists(method.getBody().getThisTypeAnnotation()) and
+    not exists(other.getBody().getThisTypeAnnotation())
+    or
+    method.getBody().getThisTypeAnnotation().getType() = other.getBody().getThisTypeAnnotation().getType()
+  ) 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)

javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.qhelp

@@ -13,7 +13,8 @@
 
   <p>
     One way to cause prototype pollution is through use of an unsafe <em>merge</em> or <em>extend</em> function
-    to recursively copy properties from one object to another.
+    to recursively copy properties from one object to another, or through the use of a <em>deep assignment</em>
+    function to assign to an unverified chain of property names.
     Such a function has the potential to modify any object reachable from the destination object, and
     the built-in <code>Object.prototype</code> is usually reachable through the special properties
     <code>__proto__</code> and <code>constructor.prototype</code>.
@@ -23,13 +24,13 @@
 <recommendation>
   <p>
     The most effective place to guard against this is in the function that performs
-    the recursive copy.
+    the recursive copy or deep assignment.
   </p>
 
   <p>
-    Only merge a property recursively when it is an own property of the <em>destination</em> object.
+    Only merge or assign a property recursively when it is an own property of the <em>destination</em> object.
     Alternatively, blacklist the property names <code>__proto__</code> and <code>constructor</code>
-    from being merged.
+    from being merged or assigned to.
   </p>
 </recommendation>