Merge pull request #18547 from erik-krogh/suffixCheck

JS: Fix FPs with js/incorrect-suffix-check
github · Jan 22, 2025 · 4bd4937 · 4bd4937
2 parents 546a497 + 04bbd59
commit 4bd4937
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 0 deletions.
diff --git a/javascript/ql/src/Security/CWE-020/IncorrectSuffixCheck.ql b/javascript/ql/src/Security/CWE-020/IncorrectSuffixCheck.ql
@@ -44,12 +44,25 @@ class IndexOfCall extends DataFlow::MethodCallNode {
    * Gets an `indexOf` call with the same receiver, argument, and method name, including this call itself.
    */
   IndexOfCall getAnEquivalentIndexOfCall() {
+    result = this
+    or
     exists(DataFlow::Node recv, string m |
       this.receiverAndMethodName(recv, m) and result.receiverAndMethodName(recv, m)
     |
+      // both directly reference the same value
       result.getArgument(0).getALocalSource() = this.getArgument(0).getALocalSource()
       or
+      // both use the same string literal
       result.getArgument(0).getStringValue() = this.getArgument(0).getStringValue()
+      or
+      // both use the same concatenation of a string and a value
+      exists(Expr origin, StringLiteral str, AddExpr otherAdd |
+        this.getArgument(0).asExpr().(AddExpr).hasOperands(origin, str) and
+        otherAdd = result.getArgument(0).asExpr()
+      |
+        otherAdd.getAnOperand().(StringLiteral).getStringValue() = str.getStringValue() and
+        otherAdd.getAnOperand().flow().getALocalSource() = origin.flow().getALocalSource()
+      )
     )
   }
 

diff --git a/javascript/ql/src/change-notes/2025-01-22-indexof-suffix-check.md b/javascript/ql/src/change-notes/2025-01-22-indexof-suffix-check.md
@@ -0,0 +1,4 @@
+---
+category: majorAnalysis
+---
+* The `js/incorrect-suffix-check` query now recognises some good patterns of the form `origin.indexOf("." + allowedOrigin)` that were previously falsely flagged.
diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck/tst.js b/javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck/tst.js
@@ -97,3 +97,15 @@ function lastIndexNeqMinusOne(x) {
 function lastIndexEqMinusOne(x) {
   return x.lastIndexOf("example.com") === -1 || x.lastIndexOf("example.com") === x.length - "example.com".length; // OK
 }
+
+function sameCheck(allowedOrigin) {
+    const trustedAuthority = "example.com";
+
+    const ind = trustedAuthority.indexOf("." + allowedOrigin);
+    return ind > 0 && ind === trustedAuthority.length - allowedOrigin.length - 1; // OK
+}
+
+function sameConcatenation(allowedOrigin) {
+    const trustedAuthority = "example.com";
+    return trustedAuthority.indexOf("." + allowedOrigin) > 0 && trustedAuthority.indexOf("." + allowedOrigin) === trustedAuthority.length - allowedOrigin.length - 1; // OK
+}