Merge pull request #1894 from asger-semmle/fp-incorrect-suffix-check

Approved by xiemaisi

Author: semmle-qlci

change-notes/1.23/analysis-javascript.md

@@ -20,6 +20,7 @@
 | Client-side cross-site scripting (`js/xss`) | More results | More potential vulnerabilities involving functions that manipulate DOM attributes are now recognized. |
 | Code injection (`js/code-injection`) | More results | More potential vulnerabilities involving functions that manipulate DOM event handler attributes are now recognized. |
 | Prototype pollution (`js/prototype-pollution`) | More results | The query now highlights vulnerable uses of jQuery and Angular, and the results are shown on LGTM by default. |
+| Incorrect suffix check (`js/incorrect-suffix-check`) | Fewer false-positive results | The query recognizes valid checks in more cases. |
 
 ## Changes to QL libraries
 

javascript/ql/src/Security/CWE-020/IncorrectSuffixCheck.ql

@@ -35,7 +35,11 @@ class IndexOfCall extends DataFlow::MethodCallNode {
    */
   IndexOfCall getAnEquivalentIndexOfCall() {
     result.getReceiver().getALocalSource() = this.getReceiver().getALocalSource() and
-    result.getArgument(0).getALocalSource() = this.getArgument(0).getALocalSource() and
+    (
+      result.getArgument(0).getALocalSource() = this.getArgument(0).getALocalSource()
+      or
+      result.getArgument(0).getStringValue() = this.getArgument(0).getStringValue()
+    ) and
     result.getMethodName() = this.getMethodName()
   }
 

javascript/ql/test/query-tests/Security/CWE-020/tst.js

@@ -89,3 +89,11 @@ function withIndexOfCheckLowerEq(x, y) {
   let index = x.indexOf(y);
   return !(index <= -1) && index === x.length - y.length - 1; // OK
 }
+
+function lastIndexNeqMinusOne(x) {
+  return x.lastIndexOf("example.com") !== -1 && x.lastIndexOf("example.com") === x.length - "example.com".length; // OK
+}
+
+function lastIndexEqMinusOne(x) {
+  return x.lastIndexOf("example.com") === -1 || x.lastIndexOf("example.com") === x.length - "example.com".length; // OK
+}