use the number guard in existing queries that contained typeof checks

Author: erik-krogh

javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll

@@ -1166,6 +1166,25 @@ module TaintTracking {
     )
   }
 
+  /** Holds if `guard` is a test that checks if `operand` is a number. */
+  predicate isNumberGuard(DataFlow::Node guard, Expr operand, boolean polarity) {
+    exists(DataFlow::CallNode isNaN |
+      isNaN = DataFlow::globalVarRef("isNaN").getACall() and guard = isNaN and polarity = false
+    |
+      operand = isNaN.getArgument(0).asExpr()
+      or
+      exists(DataFlow::CallNode parse |
+        parse = DataFlow::globalVarRef(["parseInt", "parseFloat"]).getACall()
+      |
+        parse = isNaN.getArgument(0) and
+        operand = parse.getArgument(0).asExpr()
+      )
+    )
+    or
+    isTypeofGuard(guard.asExpr(), operand, "number") and
+    polarity = guard.asExpr().(EqualityTest).getPolarity()
+  }
+
   /** DEPRECATED. This class has been renamed to `MembershipTestSanitizer`. */
   deprecated class StringInclusionSanitizer = MembershipTestSanitizer;
 

javascript/ql/lib/semmle/javascript/security/TaintedObject.qll

@@ -110,6 +110,16 @@ module TaintedObject {
     }
   }
 
+  /** A guard that checks whether `x` is a number. */
+  class NumberGuard extends SanitizerGuard instanceof DataFlow::CallNode {
+    Expr x;
+    boolean polarity;
+
+    NumberGuard() { TaintTracking::isNumberGuard(this, x, polarity) }
+
+    override predicate sanitizes(boolean outcome, Expr e) { e = x and outcome = polarity }
+  }
+
   /**
    * A sanitizer guard that validates an input against a JSON schema.
    */

javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentQuery.qll

@@ -122,6 +122,7 @@ class Configuration extends TaintTracking::Configuration {
     guard instanceof InstanceofCheck or
     guard instanceof IsArrayCheck or
     guard instanceof TypeofCheck or
+    guard instanceof NumberGuard or
     guard instanceof EqualityCheck or
     guard instanceof IncludesCheck
   }
@@ -228,6 +229,16 @@ private class TypeofCheck extends TaintTracking::LabeledSanitizerGuardNode, Data
   }
 }
 
+/** A guard that checks whether `x` is a number. */
+class NumberGuard extends TaintTracking::SanitizerGuardNode instanceof DataFlow::CallNode {
+  Expr x;
+  boolean polarity;
+
+  NumberGuard() { TaintTracking::isNumberGuard(this, x, polarity) }
+
+  override predicate sanitizes(boolean outcome, Expr e) { e = x and outcome = polarity }
+}
+
 /** A call to `Array.isArray`, which is false for `Object.prototype`. */
 private class IsArrayCheck extends TaintTracking::LabeledSanitizerGuardNode, DataFlow::CallNode {
   IsArrayCheck() { this = DataFlow::globalVarRef("Array").getAMemberCall("isArray") }

javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll

@@ -153,6 +153,16 @@ module UnsafeJQueryPlugin {
     }
   }
 
+  /** A guard that checks whether `x` is a number. */
+  class NumberGuard extends TaintTracking::SanitizerGuardNode instanceof DataFlow::CallNode {
+    Expr x;
+    boolean polarity;
+
+    NumberGuard() { TaintTracking::isNumberGuard(this, x, polarity) }
+
+    override predicate sanitizes(boolean outcome, Expr e) { e = x and outcome = polarity }
+  }
+
   /**
    * The client-provided options object for a jQuery plugin, considered as a source for unsafe jQuery plugins.
    */

javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeJQueryPluginQuery.qll

@@ -46,7 +46,8 @@ class Configuration extends TaintTracking::Configuration {
   override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode node) {
     super.isSanitizerGuard(node) or
     node instanceof IsElementSanitizer or
-    node instanceof PropertyPresenceSanitizer
+    node instanceof PropertyPresenceSanitizer or
+    node instanceof NumberGuard
   }
 }
 

javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll

@@ -286,27 +286,14 @@ module UnsafeShellCommandConstruction {
     }
   }
 
-  /**
-   * A guard of the form `isNaN(x)`, which sanitizes `x` in its "else" branch.
-   */
-  class NaNGuard extends TaintTracking::SanitizerGuardNode instanceof DataFlow::CallNode {
+  /** A guard that checks whether `x` is a number. */
+  class NumberGuard extends TaintTracking::SanitizerGuardNode instanceof DataFlow::CallNode {
     Expr x;
+    boolean polarity;
 
-    NaNGuard() {
-      this = DataFlow::globalVarRef("isNaN").getACall() and
-      (
-        x = this.getArgument(0).asExpr()
-        or
-        exists(DataFlow::CallNode parse |
-          parse = DataFlow::globalVarRef(["parseInt", "parseFloat"]).getACall()
-        |
-          parse = this.getArgument(0) and
-          x = parse.getArgument(0).asExpr()
-        )
-      )
-    }
+    NumberGuard() { TaintTracking::isNumberGuard(this, x, polarity) }
 
-    override predicate sanitizes(boolean outcome, Expr e) { e = x and outcome = false }
+    override predicate sanitizes(boolean outcome, Expr e) { e = x and outcome = polarity }
   }
 
   private import semmle.javascript.dataflow.internal.AccessPaths

javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionQuery.qll

@@ -25,7 +25,7 @@ class Configuration extends TaintTracking::Configuration {
   override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
     guard instanceof PathExistsSanitizerGuard or
     guard instanceof TaintTracking::AdHocWhitelistCheckSanitizer or
-    guard instanceof NaNGuard or
+    guard instanceof NumberGuard or
     guard instanceof TypeOfSanitizer
   }
 

javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll

@@ -108,4 +108,14 @@ module UnvalidatedDynamicMethodCall {
       label instanceof MaybeNonFunction
     }
   }
+
+  /** A guard that checks whether `x` is a number. */
+  class NumberGuard extends TaintTracking::SanitizerGuardNode instanceof DataFlow::CallNode {
+    Expr x;
+    boolean polarity;
+
+    NumberGuard() { TaintTracking::isNumberGuard(this, x, polarity) }
+
+    override predicate sanitizes(boolean outcome, Expr e) { e = x and outcome = polarity }
+  }
 }

javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll

@@ -40,6 +40,11 @@ class Configuration extends TaintTracking::Configuration {
 
   override predicate isSanitizer(DataFlow::Node nd) { super.isSanitizer(nd) }
 
+  override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
+    guard instanceof NumberGuard or
+    guard instanceof FunctionCheck
+  }
+
   override predicate isAdditionalFlowStep(
     DataFlow::Node src, DataFlow::Node dst, DataFlow::FlowLabel srclabel,
     DataFlow::FlowLabel dstlabel

javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomQuery.qll

@@ -28,6 +28,7 @@ class Configuration extends TaintTracking::Configuration {
   override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
     guard instanceof TypeTestGuard or
     guard instanceof UnsafeJQuery::PropertyPresenceSanitizer or
+    guard instanceof UnsafeJQuery::NumberGuard or
     guard instanceof DomBasedXss::SanitizerGuard
   }