introduce and use TaintTracking::isTypeofGuard

Author: erik-krogh

javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql

@@ -18,6 +18,7 @@ import javascript
 import DataFlow
 import PathGraph
 import semmle.javascript.DynamicPropertyAccess
+private import semmle.javascript.dataflow.InferredTypes
 
 /**
  * A call of form `x.split(".")` where `x` is a parameter.
@@ -394,34 +395,31 @@ class InstanceOfGuard extends DataFlow::LabeledBarrierGuardNode, DataFlow::Value
  */
 class TypeofGuard extends DataFlow::LabeledBarrierGuardNode, DataFlow::ValueNode {
   override EqualityTest astNode;
-  TypeofExpr typeof;
-  string typeofStr;
+  Expr operand;
+  InferredType type;
 
-  TypeofGuard() {
-    typeof = astNode.getAnOperand() and
-    typeofStr = astNode.getAnOperand().getStringValue()
-  }
+  TypeofGuard() { TaintTracking::isTypeofGuard(astNode, operand, type) }
 
   override predicate blocks(boolean outcome, Expr e, DataFlow::FlowLabel label) {
-    e = typeof.getOperand() and
+    e = operand and
     outcome = astNode.getPolarity() and
     (
-      typeofStr = "object" and
+      type = TTObject() and
       label = "constructor"
       or
-      typeofStr = "function" and
+      type = TTFunction() and
       label = "__proto__"
     )
     or
-    e = typeof.getOperand() and
+    e = operand and
     outcome = astNode.getPolarity().booleanNot() and
     (
       // If something is not an object, sanitize object, as both must end
       // in non-function prototype object.
-      typeofStr = "object" and
+      type = TTObject() and
       label instanceof UnsafePropLabel
       or
-      typeofStr = "function" and
+      type = TTFunction() and
       label = "constructor"
     )
   }

javascript/ql/src/semmle/javascript/DefensiveProgramming.qll

@@ -326,20 +326,15 @@ module DefensiveExpressionTest {
    */
   private class TypeofTest extends EqualityTest {
     Expr operand;
-    TypeofTag tag;
+    InferredType type;
 
-    TypeofTest() {
-      exists(Expr op1, Expr op2 | hasOperands(op1, op2) |
-        operand = op1.(TypeofExpr).getOperand() and
-        op2.mayHaveStringValue(tag)
-      )
-    }
+    TypeofTest() { TaintTracking::isTypeofGuard(this, operand, type) }
 
     boolean getTheTestResult() {
       exists(boolean testResult |
-        testResult = true and operand.analyze().getTheType().getTypeofTag() = tag
+        testResult = true and operand.analyze().getTheType() = type
         or
-        testResult = false and not operand.analyze().getAType().getTypeofTag() = tag
+        testResult = false and not operand.analyze().getAType() = type
       |
         if getPolarity() = true then result = testResult else result = testResult.booleanNot()
       )
@@ -353,7 +348,7 @@ module DefensiveExpressionTest {
     /**
      * Gets the `typeof` tag that is tested.
      */
-    TypeofTag getTag() { result = tag }
+    TypeofTag getTag() { result = type.getTypeofTag() }
   }
 
   /**

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

@@ -899,12 +899,7 @@ module TaintTracking {
     Expr x;
     override EqualityTest astNode;
 
-    TypeOfUndefinedSanitizer() {
-      exists(StringLiteral str, TypeofExpr typeof | astNode.hasOperands(str, typeof) |
-        str.getValue() = "undefined" and
-        typeof.getOperand() = x
-      )
-    }
+    TypeOfUndefinedSanitizer() { isTypeofGuard(astNode, x, TTUndefined()) }
 
     override predicate sanitizes(boolean outcome, Expr e) {
       outcome = astNode.getPolarity() and
@@ -914,6 +909,18 @@ module TaintTracking {
     override predicate appliesTo(Configuration cfg) { any() }
   }
 
+  /**
+   * Holds if `test` is a guard that checks if `operand` is typeof `type`.
+   *
+   * See `TypeOfUndefinedSanitizer` for example usage.
+   */
+  predicate isTypeofGuard(EqualityTest test, Expr operand, InferredType type) {
+    exists(Expr str, TypeofExpr typeof | test.hasOperands(str, typeof) |
+      str.mayHaveStringValue(type.getTypeofTag()) and
+      typeof.getOperand() = operand
+    )
+  }
+
   /** DEPRECATED. This class has been renamed to `MembershipTestSanitizer`. */
   deprecated class StringInclusionSanitizer = MembershipTestSanitizer;
 

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

@@ -14,6 +14,7 @@
  */
 
 import javascript
+private import semmle.javascript.dataflow.InferredTypes
 
 module TaintedObject {
   private import DataFlow
@@ -98,25 +99,24 @@ module TaintedObject {
    */
   private class TypeTestGuard extends SanitizerGuard, ValueNode {
     override EqualityTest astNode;
-    TypeofExpr typeof;
+    Expr operand;
     boolean polarity;
 
     TypeTestGuard() {
-      astNode.getAnOperand() = typeof and
-      (
+      exists(InferredType type | TaintTracking::isTypeofGuard(astNode, operand, type) |
         // typeof x === "object" sanitizes `x` when it evaluates to false
-        astNode.getAnOperand().getStringValue() = "object" and
+        type = TTObject() and
         polarity = astNode.getPolarity().booleanNot()
         or
         // typeof x === "string" sanitizes `x` when it evaluates to true
-        astNode.getAnOperand().getStringValue() != "object" and
+        type != TTObject() and
         polarity = astNode.getPolarity()
       )
     }
 
     override predicate sanitizes(boolean outcome, Expr e, FlowLabel label) {
       polarity = outcome and
-      e = typeof.getOperand() and
+      e = operand and
       label = label()
     }
   }

javascript/ql/src/semmle/javascript/security/dataflow/PrototypePollutingAssignment.qll

@@ -9,6 +9,7 @@
 
 private import javascript
 private import semmle.javascript.DynamicPropertyAccess
+private import semmle.javascript.dataflow.InferredTypes
 
 /**
  * Provides a taint tracking configuration for reasoning about
@@ -164,22 +165,18 @@ module PrototypePollutingAssignment {
   private class TypeofCheck extends TaintTracking::LabeledSanitizerGuardNode, DataFlow::ValueNode {
     override EqualityTest astNode;
     Expr operand;
-    string value;
+    boolean polarity;
 
     TypeofCheck() {
-      exists(TypeofExpr typeof, Expr str |
-        astNode.hasOperands(typeof, str) and
-        typeof.getOperand() = operand and
-        str.getStringValue() = value
+      exists(InferredType type | TaintTracking::isTypeofGuard(astNode, operand, type) |
+        type = TTObject() and polarity = astNode.getPolarity().booleanNot()
+        or
+        type != TTObject() and polarity = astNode.getPolarity()
       )
     }
 
     override predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) {
-      (
-        value = "object" and outcome = astNode.getPolarity().booleanNot()
-        or
-        value != "object" and outcome = astNode.getPolarity()
-      ) and
+      polarity = outcome and
       e = operand and
       label instanceof ObjectPrototype
     }

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

@@ -134,10 +134,7 @@ module UnsafeJQueryPlugin {
             SyntacticConstants::isUndefined(undef)
           )
           or
-          exists(Expr op1, Expr op2 | test.hasOperands(op1, op2) |
-            read.asExpr() = op1.(TypeofExpr).getOperand() and
-            op2.mayHaveStringValue(any(InferredType t | t = TTUndefined()).getTypeofTag())
-          )
+          TaintTracking::isTypeofGuard(test, read.asExpr(), TTUndefined())
         )
         or
         polarity = true and

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

@@ -200,12 +200,8 @@ module UnsafeShellCommandConstruction {
     override EqualityTest astNode;
 
     TypeOfSanitizer() {
-      exists(Expr str, TypeofExpr typeof | astNode.hasOperands(str, typeof) |
-        str.mayHaveStringValue(any(InferredType t |
-            t = TTUndefined() or t = TTNumber() or t = TTBoolean()
-          ).getTypeofTag()) and
-        typeof.getOperand() = x
-      )
+      TaintTracking::isTypeofGuard(astNode, x,
+        any(InferredType t | t = TTUndefined() or t = TTNumber() or t = TTBoolean()))
     }
 
     override predicate sanitizes(boolean outcome, Expr e) {

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

@@ -98,16 +98,13 @@ module UnvalidatedDynamicMethodCall {
    */
   class FunctionCheck extends TaintTracking::LabeledSanitizerGuardNode, DataFlow::ValueNode {
     override EqualityTest astNode;
-    TypeofExpr t;
+    Expr operand;
 
-    FunctionCheck() {
-      astNode.getAnOperand().getStringValue() = "function" and
-      astNode.getAnOperand().getUnderlyingValue() = t
-    }
+    FunctionCheck() { TaintTracking::isTypeofGuard(astNode, operand, TTFunction()) }
 
     override predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) {
       outcome = astNode.getPolarity() and
-      e = t.getOperand().getUnderlyingValue() and
+      e = operand and
       label instanceof MaybeNonFunction
     }
   }

javascript/ql/src/semmle/javascript/security/dataflow/XssThroughDom.qll

@@ -4,6 +4,7 @@
  */
 
 import javascript
+private import semmle.javascript.dataflow.InferredTypes
 
 /**
  * Classes and predicates for the XSS through DOM query.
@@ -103,25 +104,24 @@ module XssThroughDom {
    */
   class TypeTestGuard extends TaintTracking::SanitizerGuardNode, DataFlow::ValueNode {
     override EqualityTest astNode;
-    TypeofExpr typeof;
+    Expr operand;
     boolean polarity;
 
     TypeTestGuard() {
-      astNode.getAnOperand() = typeof and
-      (
+      exists(InferredType type | TaintTracking::isTypeofGuard(astNode, operand, type) |
         // typeof x === "string" sanitizes `x` when it evaluates to false
-        astNode.getAnOperand().getStringValue() = "string" and
+        type = TTString() and
         polarity = astNode.getPolarity().booleanNot()
         or
         // typeof x === "object" sanitizes `x` when it evaluates to true
-        astNode.getAnOperand().getStringValue() != "string" and
+        type != TTString() and
         polarity = astNode.getPolarity()
       )
     }
 
     override predicate sanitizes(boolean outcome, Expr e) {
       polarity = outcome and
-      e = typeof.getOperand()
+      e = operand
     }
   }
 }