Ruby: Use a newtype instead of DataFlow::FlowState for insecure-download

Author: alexrford

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

@@ -21,8 +21,11 @@ module InsecureDownload {
   abstract class Source extends DataFlow::Node {
     /**
      * Gets a flow-label for this source.
+     * DEPRECATED: Use `getAFlowLabel()`
      */
-    abstract DataFlow::FlowState getALabel();
+    abstract deprecated DataFlow::FlowState getALabel();
+
+    abstract Label::State getAFlowLabel();
   }
 
   /**
@@ -36,8 +39,11 @@ module InsecureDownload {
 
     /**
      * Gets a flow-label where this sink is vulnerable.
+     * DEPRECATED: Use `getAFlowLabel()`
      */
-    abstract DataFlow::FlowState getALabel();
+    abstract deprecated DataFlow::FlowState getALabel();
+
+    abstract Label::State getAFlowLabel();
   }
 
   /**
@@ -51,24 +57,35 @@ module InsecureDownload {
   module Label {
     /**
      * A flow-label for a URL that is downloaded over an insecure connection.
+     * DEPRECATED: Use `InsecureState()`
      */
-    class Insecure extends DataFlow::FlowState {
+    deprecated class Insecure extends DataFlow::FlowState {
       Insecure() { this = "insecure" }
     }
 
     /**
      * A flow-label for a URL that is sensitive.
+     * DEPRECATED: Use `SensitiveState()`
      */
-    class Sensitive extends DataFlow::FlowState {
+    deprecated class Sensitive extends DataFlow::FlowState {
       Sensitive() { this = "sensitive" }
     }
 
     /**
      * A flow-label for file URLs that are both sensitive and downloaded over an insecure connection.
+     * DEPRECATED: Use `SensitiveInsecureState()`
      */
-    class SensitiveInsecure extends DataFlow::FlowState {
+    deprecated class SensitiveInsecure extends DataFlow::FlowState {
       SensitiveInsecure() { this = "sensitiveInsecure" }
     }
+
+    /**
+     * Flow-labels for reasoning about download of sensitive file through insecure connection.
+     */
+    newtype State =
+      InsecureState() or
+      SensitiveState() or
+      SensitiveInsecureState()
   }
 
   /**
@@ -88,12 +105,19 @@ module InsecureDownload {
    * seen as a source for downloads of sensitive files through an insecure connection.
    */
   class InsecureFileUrl extends Source, InsecureUrl {
-    override DataFlow::FlowState getALabel() {
+    deprecated override DataFlow::FlowState getALabel() {
       result instanceof Label::Insecure
       or
       hasUnsafeExtension(str) and
       result instanceof Label::SensitiveInsecure
     }
+
+    override Label::State getAFlowLabel() {
+      result = Label::InsecureState()
+      or
+      hasUnsafeExtension(str) and
+      result = Label::SensitiveInsecureState()
+    }
   }
 
   /**
@@ -103,7 +127,9 @@ module InsecureDownload {
   class SensitiveFileName extends Source {
     SensitiveFileName() { hasUnsafeExtension(this.asExpr().getConstantValue().getString()) }
 
-    override DataFlow::FlowState getALabel() { result instanceof Label::Sensitive }
+    deprecated override DataFlow::FlowState getALabel() { result instanceof Label::Sensitive }
+
+    override Label::State getAFlowLabel() { result = Label::SensitiveState() }
   }
 
   /**
@@ -145,11 +171,17 @@ module InsecureDownload {
 
     override DataFlow::Node getDownloadCall() { result = req }
 
-    override DataFlow::FlowState getALabel() {
+    deprecated override DataFlow::FlowState getALabel() {
       result instanceof Label::SensitiveInsecure
       or
       any(req.getAUrlPart()) instanceof InsecureUrl and result instanceof Label::Sensitive
     }
+
+    override Label::State getAFlowLabel() {
+      result = Label::SensitiveInsecureState()
+      or
+      any(req.getAUrlPart()) instanceof InsecureUrl and result = Label::SensitiveState()
+    }
   }
 
   /**
@@ -191,7 +223,9 @@ module InsecureDownload {
       )
     }
 
-    override DataFlow::FlowState getALabel() { result instanceof Label::Insecure }
+    deprecated override DataFlow::FlowState getALabel() { result instanceof Label::Insecure }
+
+    override Label::State getAFlowLabel() { result = Label::InsecureState() }
 
     override DataFlow::Node getDownloadCall() { result = request }
   }

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

@@ -33,15 +33,13 @@ deprecated class Configuration extends DataFlow::Configuration {
 }
 
 private module InsecureDownloadConfig implements DataFlow::StateConfigSig {
-  class FlowState = string;
+  class FlowState = Label::State;
 
-  predicate isSource(DataFlow::Node source, DataFlow::FlowState label) {
-    source.(Source).getALabel() = label
+  predicate isSource(DataFlow::Node source, FlowState label) {
+    source.(Source).getAFlowLabel() = label
   }
 
-  predicate isSink(DataFlow::Node sink, DataFlow::FlowState label) {
-    sink.(Sink).getALabel() = label
-  }
+  predicate isSink(DataFlow::Node sink, FlowState label) { sink.(Sink).getAFlowLabel() = label }
 
   predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
 }