JS: Migrate UnsafeDynamicMethodAccess

Author: asgerf

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

@@ -10,26 +10,61 @@ import semmle.javascript.frameworks.Express
 import PropertyInjectionShared
 
 module UnsafeDynamicMethodAccess {
-  private import DataFlow::FlowLabel
+  private newtype TFlowState =
+    TTaint() or
+    TUnsafeFunction()
+
+  /** A flow state to associate with a tracked value. */
+  class FlowState extends TFlowState {
+    /** Gets a string representation fo this flow state */
+    string toString() {
+      this = TTaint() and result = "taint"
+      or
+      this = TUnsafeFunction() and result = "unsafe-function"
+    }
+
+    deprecated DataFlow::FlowLabel toFlowLabel() {
+      this = TTaint() and result.isTaint()
+      or
+      this = TUnsafeFunction() and result instanceof UnsafeFunction
+    }
+  }
+
+  /** Predicates for working with flow states. */
+  module FlowState {
+    deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label }
+
+    /** A tainted value. */
+    FlowState taint() { result = TTaint() }
+
+    /** A reference to an unsafe function, such as `eval`, obtained by reading from a tainted property name. */
+    FlowState unsafeFunction() { result = TUnsafeFunction() }
+  }
 
   /**
    * A data flow source for unsafe dynamic method access.
    */
   abstract class Source extends DataFlow::Node {
     /**
-     * Gets the flow label relevant for this source.
+     * Gets a flow state relevant for this source.
      */
-    DataFlow::FlowLabel getFlowLabel() { result = taint() }
+    FlowState getAFlowState() { result = FlowState::taint() }
+
+    /** DEPRECATED. Use `getAFlowState()` instead. */
+    deprecated DataFlow::FlowLabel getFlowLabel() { result = this.getAFlowState().toFlowLabel() }
   }
 
   /**
    * A data flow sink for unsafe dynamic method access.
    */
   abstract class Sink extends DataFlow::Node {
     /**
-     * Gets the flow label relevant for this sink
+     * Gets a flow state relevant for this sink.
      */
-    abstract DataFlow::FlowLabel getFlowLabel();
+    FlowState getAFlowState() { result = FlowState::taint() }
+
+    /** DEPRECATED. Use `getAFlowState()` instead. */
+    deprecated DataFlow::FlowLabel getFlowLabel() { result = this.getAFlowState().toFlowLabel() }
   }
 
   /**
@@ -38,16 +73,20 @@ module UnsafeDynamicMethodAccess {
   abstract class Sanitizer extends DataFlow::Node { }
 
   /**
+   * DEPRECATED. Use `FlowState::unsafeFunction()` instead.
+   *
    * Gets the flow label describing values that may refer to an unsafe
    * function as a result of an attacker-controlled property name.
    */
-  UnsafeFunction unsafeFunction() { any() }
+  deprecated UnsafeFunction unsafeFunction() { any() }
 
   /**
+   * DEPRECATED. Use `FlowState::unsafeFunction()` instead.
+   *
    * A flow label describing values that may refer to an unsafe
    * function as a result of an attacker-controlled property name.
    */
-  abstract class UnsafeFunction extends DataFlow::FlowLabel {
+  abstract deprecated class UnsafeFunction extends DataFlow::FlowLabel {
     UnsafeFunction() { this = "UnsafeFunction" }
   }
 
@@ -67,6 +106,6 @@ module UnsafeDynamicMethodAccess {
   class CalleeAsSink extends Sink {
     CalleeAsSink() { this = any(DataFlow::InvokeNode node).getCalleeNode() }
 
-    override DataFlow::FlowLabel getFlowLabel() { result = unsafeFunction() }
+    override FlowState getAFlowState() { result = FlowState::unsafeFunction() }
   }
 }

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

@@ -11,25 +11,24 @@ import javascript
 import PropertyInjectionShared
 private import DataFlow::FlowLabel
 import UnsafeDynamicMethodAccessCustomizations::UnsafeDynamicMethodAccess
+private import UnsafeDynamicMethodAccessCustomizations::UnsafeDynamicMethodAccess as UnsafeDynamicMethodAccess
 
 // Materialize flow labels
-private class ConcreteUnsafeFunction extends UnsafeFunction {
+deprecated private class ConcreteUnsafeFunction extends UnsafeFunction {
   ConcreteUnsafeFunction() { this = this }
 }
 
 /**
  * A taint-tracking configuration for reasoning about unsafe dynamic method access.
  */
 module UnsafeDynamicMethodAccessConfig implements DataFlow::StateConfigSig {
-  class FlowState = DataFlow::FlowLabel;
+  class FlowState = UnsafeDynamicMethodAccess::FlowState;
 
-  predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
-    source.(Source).getFlowLabel() = label
+  predicate isSource(DataFlow::Node source, FlowState label) {
+    source.(Source).getAFlowState() = label
   }
 
-  predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
-    sink.(Sink).getFlowLabel() = label
-  }
+  predicate isSink(DataFlow::Node sink, FlowState label) { sink.(Sink).getAFlowState() = label }
 
   predicate isBarrier(DataFlow::Node node) {
     node instanceof Sanitizer
@@ -38,44 +37,42 @@ module UnsafeDynamicMethodAccessConfig implements DataFlow::StateConfigSig {
     not StringConcatenation::isCoercion(node)
   }
 
-  predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) {
+  predicate isBarrier(DataFlow::Node node, FlowState label) {
     TaintTracking::defaultSanitizer(node) and
-    label.isTaint()
+    label = FlowState::taint()
   }
 
   /** An additional flow step for use in both this configuration and the legacy configuration. */
   additional predicate additionalFlowStep(
-    DataFlow::Node src, DataFlow::FlowLabel srclabel, DataFlow::Node dst,
-    DataFlow::FlowLabel dstlabel
+    DataFlow::Node src, FlowState srclabel, DataFlow::Node dst, FlowState dstlabel
   ) {
     // Reading a property of the global object or of a function
     exists(DataFlow::PropRead read |
       PropertyInjection::hasUnsafeMethods(read.getBase().getALocalSource()) and
       src = read.getPropertyNameExpr().flow() and
       dst = read and
-      srclabel.isTaint() and
-      dstlabel = unsafeFunction()
+      srclabel = FlowState::taint() and
+      dstlabel = FlowState::unsafeFunction()
     )
     or
     // Reading a chain of properties from any object with a prototype can lead to Function
     exists(PropertyProjection proj |
       not PropertyInjection::isPrototypeLessObject(proj.getObject().getALocalSource()) and
       src = proj.getASelector() and
       dst = proj and
-      srclabel.isTaint() and
-      dstlabel = unsafeFunction()
+      srclabel = FlowState::taint() and
+      dstlabel = FlowState::unsafeFunction()
     )
   }
 
   predicate isAdditionalFlowStep(
-    DataFlow::Node src, DataFlow::FlowLabel srclabel, DataFlow::Node dst,
-    DataFlow::FlowLabel dstlabel
+    DataFlow::Node src, FlowState srclabel, DataFlow::Node dst, FlowState dstlabel
   ) {
     additionalFlowStep(src, srclabel, dst, dstlabel)
     or
     // We're not using a taint-tracking config because taint steps would then apply to all flow states.
     // So we use a plain data flow config and manually add the default taint steps.
-    srclabel.isTaint() and
+    srclabel = FlowState::taint() and
     TaintTracking::defaultTaintStep(src, dst) and
     srclabel = dstlabel
   }
@@ -93,11 +90,11 @@ deprecated class Configuration extends TaintTracking::Configuration {
   Configuration() { this = "UnsafeDynamicMethodAccess" }
 
   override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
-    UnsafeDynamicMethodAccessConfig::isSource(source, label)
+    UnsafeDynamicMethodAccessConfig::isSource(source, FlowState::fromFlowLabel(label))
   }
 
   override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
-    UnsafeDynamicMethodAccessConfig::isSink(sink, label)
+    UnsafeDynamicMethodAccessConfig::isSink(sink, FlowState::fromFlowLabel(label))
   }
 
   override predicate isSanitizer(DataFlow::Node node) {
@@ -117,6 +114,7 @@ deprecated class Configuration extends TaintTracking::Configuration {
     DataFlow::Node src, DataFlow::Node dst, DataFlow::FlowLabel srclabel,
     DataFlow::FlowLabel dstlabel
   ) {
-    UnsafeDynamicMethodAccessConfig::additionalFlowStep(src, srclabel, dst, dstlabel)
+    UnsafeDynamicMethodAccessConfig::additionalFlowStep(src, FlowState::fromFlowLabel(srclabel),
+      dst, FlowState::fromFlowLabel(dstlabel))
   }
 }