Ruby: Use a newtype instead of DataFlow::FlowState for hardcoded-data

Author: alexrford

ruby/ql/lib/codeql/ruby/security/HardcodedDataInterpretedAsCodeCustomizations.qll

@@ -19,19 +19,40 @@ module HardcodedDataInterpretedAsCode {
    * Flow states used to distinguish value-preserving flow from taint flow.
    */
   module FlowState {
-    /** Flow state used to track value-preserving flow. */
-    DataFlow::FlowState data() { result = "data" }
+    /**
+     * Flow state used to track value-preserving flow.
+     * DEPRECATED: Use `Data()`
+     */
+    deprecated DataFlow::FlowState data() { result = "data" }
 
-    /** Flow state used to tainted data (non-value preserving flow). */
-    DataFlow::FlowState taint() { result = "taint" }
+    /**
+     * Flow state used to tainted data (non-value preserving flow).
+     * DEPRECATED: Use `Taint()`
+     */
+    deprecated DataFlow::FlowState taint() { result = "taint" }
+
+    /**
+     * Flow states used to distinguish value-preserving flow from taint flow.
+     */
+    newtype State =
+      Data() or
+      Taint()
   }
 
   /**
    * A data flow source for hard-coded data.
    */
   abstract class Source extends DataFlow::Node {
-    /** Gets a flow label for which this is a source. */
-    DataFlow::FlowState getLabel() { result = FlowState::data() }
+    /**
+     * Gets a flow label for which this is a source.
+     * DEPRECATED: Use `getALabel()`
+     */
+    deprecated DataFlow::FlowState getLabel() { result = FlowState::data() }
+
+    /**
+     * Gets a flow label for which this is a source.
+     */
+    FlowState::State getALabel() { result = FlowState::Data() }
   }
 
   /**
@@ -41,13 +62,26 @@ module HardcodedDataInterpretedAsCode {
     /** Gets a description of what kind of sink this is. */
     abstract string getKind();
 
-    /** Gets a flow label for which this is a sink. */
-    DataFlow::FlowState getLabel() {
+    /**
+     * Gets a flow label for which this is a sink.
+     * DEPRECATED: Use `getALabel()`
+     */
+    deprecated DataFlow::FlowState getLabel() {
       // We want to ignore value-flow and only consider taint-flow, since the
       // source is just a hex string, and evaluating that directly will just
       // cause a syntax error.
       result = FlowState::taint()
     }
+
+    /**
+     * Gets a flow label for which this is a sink.
+     */
+    FlowState::State getALabel() {
+      // We want to ignore value-flow and only consider taint-flow, since the
+      // source is just a hex string, and evaluating that directly will just
+      // cause a syntax error.
+      result = FlowState::Taint()
+    }
   }
 
   /** A sanitizer for hard-coded data. */

ruby/ql/lib/codeql/ruby/security/HardcodedDataInterpretedAsCodeQuery.qll

@@ -45,33 +45,33 @@ deprecated class Configuration extends DataFlow::Configuration {
   }
 }
 
-/*
- * We implement `DataFlow::ConfigSig` rather than
- * `TaintTracking::ConfigSig`, so that we can set the flow state to
- * `"taint"` on a taint step.
- */
-
 private module Config implements DataFlow::StateConfigSig {
-  class FlowState = DataFlow::FlowState;
+  class FlowState = FlowState::State;
 
-  predicate isSource(DataFlow::Node source, FlowState label) { source.(Source).getLabel() = label }
+  predicate isSource(DataFlow::Node source, FlowState label) { source.(Source).getALabel() = label }
 
-  predicate isSink(DataFlow::Node sink, FlowState label) { sink.(Sink).getLabel() = label }
+  predicate isSink(DataFlow::Node sink, FlowState label) { sink.(Sink).getALabel() = label }
 
   predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
 
   predicate isAdditionalFlowStep(
-    DataFlow::Node nodeFrom, DataFlow::FlowState stateFrom, DataFlow::Node nodeTo,
-    DataFlow::FlowState stateTo
+    DataFlow::Node nodeFrom, FlowState stateFrom, DataFlow::Node nodeTo, FlowState stateTo
   ) {
     defaultAdditionalTaintStep(nodeFrom, nodeTo) and
     // This is a taint step, so the flow state becomes `taint`.
-    stateFrom = [FlowState::data(), FlowState::taint()] and
-    stateTo = FlowState::taint()
+    (
+      stateFrom = FlowState::Taint()
+      or
+      stateFrom = FlowState::Data()
+    ) and
+    stateTo = FlowState::Taint()
   }
 }
 
 /**
  * Taint-tracking for reasoning about hard-coded data being interpreted as code.
+ * We implement `DataFlow::GlobalWithState` rather than
+ * `TaintTracking::GlobalWithState`, so that we can set the flow state to
+ * `Taint()` on a taint step.
  */
 module HardcodedDataInterpretedAsCodeFlow = DataFlow::GlobalWithState<Config>;