JS: Be more conservative about flagging "search" call arguments as regex

Author: asgerf

javascript/ql/lib/semmle/javascript/Regexp.qll

@@ -958,6 +958,27 @@ private predicate isUsedAsNonMatchObject(DataFlow::MethodCallNode call) {
   )
 }
 
+/**
+ * Holds if `call` is a call to `search` whose result is used in a way that suggests it returns a number.
+ */
+pragma[inline]
+private predicate isUsedAsNumber(DataFlow::LocalSourceNode value) {
+  any(Comparison compare)
+      .hasOperands(value.getALocalUse().asExpr(), any(Expr e | e.analyze().getAType() = TTNumber()))
+  or
+  value.flowsToExpr(any(ArithmeticExpr e).getAnOperand())
+  or
+  value.flowsToExpr(any(UnaryExpr e | e.getOperator() = "-").getOperand())
+  or
+  value.flowsToExpr(any(IndexExpr expr).getPropertyNameExpr())
+  or
+  exists(DataFlow::CallNode call |
+    call.getCalleeName() =
+      ["substring", "substr", "slice", "splice", "charAt", "charCodeAt", "codePointAt"] and
+    value.flowsTo(call.getAnArgument())
+  )
+}
+
 /**
  * Holds if `source` may be interpreted as a regular expression.
  */
@@ -985,9 +1006,9 @@ predicate isInterpretedAsRegExp(DataFlow::Node source) {
       methodName = "search" and
       source = mce.getArgument(0) and
       mce.getNumArgument() = 1 and
-      // "search" is a common method name, and so we exclude chained accesses
-      // because `String.prototype.search` returns a number
-      not exists(PropAccess p | p.getBase() = mce.getEnclosingExpr())
+      // "search" is a common method name, and the built-in "search" method is rarely used,
+      // so to reduce FPs we also require that the return value appears to be used as a number.
+      isUsedAsNumber(mce)
     )
     or
     exists(DataFlow::SourceNode schema | schema = JsonSchema::getAPartOfJsonSchema() |