Merge pull request #508 from esben-semmle/js/indirect-global-call-with-default-arguments

Approved by xiemaisi

Author: semmle-qlci

change-notes/1.19/analysis-javascript.md

@@ -43,6 +43,7 @@
 | Duplicate switch case | Lower severity | The severity of this rule has been revised to "warning". |
 | Information exposure through a stack trace | More results | This rule now also flags cases where the entire exception object (including the stack trace) may be exposed. |
 | Missing CSRF middleware | Fewer false-positive results | This rule now recognizes additional CSRF protection middlewares. |
+| Missing 'this' qualifier | Fewer false-positive results | This rule now recognizes additional intentional calls to global functions. | 
 | Missing variable declaration | Lower severity | The severity of this rule has been revised to "warning". |
 | Regular expression injection | Fewer false-positive results | This rule now identifies calls to `String.prototype.search` with more precision. |
 | Remote property injection | Fewer results | The precision of this rule has been revised to "medium". Results are no longer shown on LGTM by default. |

javascript/ql/src/Declarations/MissingThisQualifier.ql

@@ -50,5 +50,14 @@ where maybeMissingThis(call, intendedTarget, gv)
           decl.isNamespaceExport() and
           call.getContainer().getEnclosingContainer*() instanceof NamespaceDeclaration
         )
+        or
+        // call to global function with additional arguments
+        exists (Function self |
+          intendedTarget.getBody() = self and
+          call.getEnclosingFunction() = self and
+          call.flow().(DataFlow::CallNode).getNumArgument() > self.getNumParameter() and
+          not self.hasRestParameter() and
+          not self.usesArgumentsObject()
+        )
       )
 select call, "This call refers to a global function, and not the local method $@.", intendedTarget, intendedTarget.getName()

javascript/ql/test/query-tests/Declarations/MissingThisQualifier/MissingThisQualifier.expected

@@ -1,4 +1,5 @@
 | abstract-missing.ts:3:5:3:24 | setAudioProperties() | This call refers to a global function, and not the local method $@. | abstract-missing.ts:6:3:6:32 | abstrac ... ties(); | setAudioProperties |
+| indirection.js:7:9:7:20 | m("default") | This call refers to a global function, and not the local method $@. | indirection.js:2:5:4:5 | m() {\\n  ... K\\n    } | m |
 | missing1.js:3:5:3:24 | setAudioProperties() | This call refers to a global function, and not the local method $@. | missing1.js:6:3:7:3 | setAudi ... (){\\n  } | setAudioProperties |
 | missing2.js:3:5:3:24 | setAudioProperties() | This call refers to a global function, and not the local method $@. | missing2.js:7:3:8:3 | static  ... (){\\n  } | setAudioProperties |
 | namespaces-uses.ts:3:5:3:20 | globalFunction() | This call refers to a global function, and not the local method $@. | namespaces-uses.ts:2:3:4:3 | globalF ...  OK\\n  } | globalFunction |

javascript/ql/test/query-tests/Declarations/MissingThisQualifier/indirection.js

@@ -0,0 +1,9 @@
+class X {
+    m() {
+        m("default"); // OK
+    }
+
+    resty(...x) {
+        m("default"); // NOT OK
+    }
+}