Merge pull request github#8942 from hvitved/ruby/dataflow/hashes

Ruby: Data-flow through hashes

Author: hvitved

ruby/ql/lib/change-notes/2022-05-19-hashes-data-flow.md

@@ -0,0 +1,4 @@
+---
+category: majorAnalysis
+---
+Added data-flow support for [hashes](https://docs.ruby-lang.org/en/3.1/Hash.html).

ruby/ql/lib/codeql/ruby/dataflow/FlowSummary.qll

@@ -57,6 +57,24 @@ module SummaryComponent {
    */
   SummaryComponent elementAny() { result = SC::content(TAnyElementContent()) }
 
+  /**
+   * Gets a summary component that represents an element in a collection at known
+   * integer index `lower` or above.
+   */
+  SummaryComponent elementLowerBound(int lower) {
+    result = SC::content(TElementLowerBoundContent(lower))
+  }
+
+  /** Gets a summary component that represents a value in a pair at an unknown key. */
+  SummaryComponent pairValueUnknown() {
+    result = SC::content(TSingletonContent(TUnknownPairValueContent()))
+  }
+
+  /** Gets a summary component that represents a value in a pair at a known key. */
+  SummaryComponent pairValueKnown(ConstantValue key) {
+    result = SC::content(TSingletonContent(TKnownPairValueContent(key)))
+  }
+
   /** Gets a summary component that represents the return value of a call. */
   SummaryComponent return() { result = SC::return(any(NormalReturnKind rk)) }
 }

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

@@ -6,6 +6,7 @@ private import DataFlowPublic
 private import DataFlowDispatch
 private import SsaImpl as SsaImpl
 private import FlowSummaryImpl as FlowSummaryImpl
+private import FlowSummaryImplSpecific as FlowSummaryImplSpecific
 
 /** Gets the callable in which this node occurs. */
 DataFlowCallable nodeGetEnclosingCallable(NodeImpl n) { result = n.getEnclosingCallable() }
@@ -177,7 +178,7 @@ private class Argument extends CfgNodes::ExprCfgNode {
     exists(int i |
       this = call.getArgument(i) and
       not this.getExpr() instanceof BlockArgument and
-      not exists(this.getExpr().(Pair).getKey().getConstantValue().getSymbol()) and
+      not this.getExpr().(Pair).getKey().getConstantValue().isSymbol(_) and
       arg.isPositional(i)
     )
     or
@@ -334,16 +335,21 @@ private module Cached {
   cached
   newtype TContentSet =
     TSingletonContent(Content c) or
-    TAnyElementContent()
+    TAnyElementContent() or
+    TElementLowerBoundContent(int lower) {
+      FlowSummaryImplSpecific::ParsePositions::isParsedElementLowerBoundPosition(_, lower)
+    }
 
   cached
   newtype TContent =
     TKnownElementContent(ConstantValue cv) {
       not cv.isInt(_) or
       cv.getInt() in [0 .. 10]
     } or
-    TFieldContent(string name) { name = any(InstanceVariable v).getName() } or
-    TUnknownElementContent()
+    TUnknownElementContent() or
+    TKnownPairValueContent(ConstantValue cv) or
+    TUnknownPairValueContent() or
+    TFieldContent(string name) { name = any(InstanceVariable v).getName() }
 
   /**
    * Holds if `e` is an `ExprNode` that may be returned by a call to `c`.
@@ -362,6 +368,8 @@ private module Cached {
 
 class TElementContent = TKnownElementContent or TUnknownElementContent;
 
+class TPairValueContent = TKnownPairValueContent or TUnknownPairValueContent;
+
 import Cached
 
 /** Holds if `n` should be hidden from path explanations. */
@@ -818,6 +826,25 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
     ).getReceiver()
   or
   FlowSummaryImpl::Private::Steps::summaryStoreStep(node1, c, node2)
+  or
+  // Needed for pairs passed into method calls where the key is not a symbol,
+  // that is, where it is not a keyword argument.
+  node2.asExpr() =
+    any(CfgNodes::ExprNodes::PairCfgNode pair |
+      exists(CfgNodes::ExprCfgNode key |
+        key = pair.getKey() and
+        pair.getValue() = node1.asExpr()
+      |
+        exists(ConstantValue cv |
+          cv = key.getConstantValue() and
+          not cv.isSymbol(_) and // handled as a keyword argument
+          c.isSingleton(TKnownPairValueContent(cv))
+        )
+        or
+        not exists(key.getConstantValue()) and
+        c.isSingleton(TUnknownPairValueContent())
+      )
+    )
 }
 
 /**

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll

@@ -242,6 +242,35 @@ module Content {
     not exists(TKnownElementContent(cv)) and
     result = TUnknownElementContent()
   }
+
+  /**
+   * Gets the constant value of `e`, which corresponds to a valid known
+   * element index. Unlike calling simply `e.getConstantValue()`, this
+   * excludes negative array indices.
+   */
+  ConstantValue getKnownElementIndex(Expr e) {
+    result = getElementContent(e.getConstantValue()).(KnownElementContent).getIndex()
+  }
+
+  /** A value in a pair with a known or unknown key. */
+  class PairValueContent extends Content, TPairValueContent { }
+
+  /** A value in a pair with a known key. */
+  class KnownPairValueContent extends PairValueContent, TKnownPairValueContent {
+    private ConstantValue key;
+
+    KnownPairValueContent() { this = TKnownPairValueContent(key) }
+
+    /** Gets the index in the collection. */
+    ConstantValue getIndex() { result = key }
+
+    override string toString() { result = "pair " + key }
+  }
+
+  /** A value in a pair with an unknown key. */
+  class UnknownPairValueContent extends PairValueContent, TUnknownPairValueContent {
+    override string toString() { result = "pair" }
+  }
 }
 
 /**
@@ -257,6 +286,12 @@ class ContentSet extends TContentSet {
   /** Holds if this content set represents all `ElementContent`s. */
   predicate isAnyElement() { this = TAnyElementContent() }
 
+  /**
+   * Holds if this content set represents all `KnownElementContent`s where
+   * the index is an integer greater than or equal to `lower`.
+   */
+  predicate isElementLowerBound(int lower) { this = TElementLowerBoundContent(lower) }
+
   /** Gets a textual representation of this content set. */
   string toString() {
     exists(Content c |
@@ -265,7 +300,12 @@ class ContentSet extends TContentSet {
     )
     or
     this.isAnyElement() and
-    result = "any array element"
+    result = "any element"
+    or
+    exists(int lower |
+      this.isElementLowerBound(lower) and
+      result = lower + ".."
+    )
   }
 
   /** Gets a content that may be stored into when storing into this set. */
@@ -274,6 +314,9 @@ class ContentSet extends TContentSet {
     or
     this.isAnyElement() and
     result = TUnknownElementContent()
+    or
+    this.isElementLowerBound(_) and
+    result = TUnknownElementContent()
   }
 
   /** Gets a content that may be read from when reading from this set. */
@@ -282,6 +325,12 @@ class ContentSet extends TContentSet {
     or
     this.isAnyElement() and
     result instanceof Content::ElementContent
+    or
+    exists(int lower, int i |
+      this.isElementLowerBound(lower) and
+      result.(Content::KnownElementContent).getIndex().isInt(i) and
+      i >= lower
+    )
   }
 }
 

ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImplSpecific.qll

@@ -67,6 +67,11 @@ private SummaryComponent interpretElementArg(string arg) {
   arg = "any" and
   result = FlowSummary::SummaryComponent::elementAny()
   or
+  exists(int lower |
+    ParsePositions::isParsedElementLowerBoundPosition(arg, lower) and
+    result = FlowSummary::SummaryComponent::elementLowerBound(lower)
+  )
+  or
   exists(ConstantValue cv | result = FlowSummary::SummaryComponent::elementKnown(cv) |
     cv.isInt(AccessPath::parseInt(arg))
     or
@@ -103,6 +108,16 @@ SummaryComponent interpretComponentSpecific(AccessPathToken c) {
       interpretElementArg(c.getAnArgument("WithoutElement")) and
     result = FlowSummary::SummaryComponent::withoutContent(cs)
   )
+  or
+  exists(string arg | arg = c.getAnArgument("PairValue") |
+    arg = "?" and
+    result = FlowSummary::SummaryComponent::pairValueUnknown()
+    or
+    exists(ConstantValue cv |
+      result = FlowSummary::SummaryComponent::pairValueKnown(cv) and
+      cv.serialize() = arg
+    )
+  )
 }
 
 /** Gets the textual representation of a summary component in the format used for flow summaries. */
@@ -217,6 +232,13 @@ module ParsePositions {
     )
   }
 
+  private predicate isElementBody(string body) {
+    exists(AccessPathToken tok |
+      tok.getName() = "Element" and
+      body = tok.getAnArgument()
+    )
+  }
+
   predicate isParsedParameterPosition(string c, int i) {
     isParamBody(c) and
     i = AccessPath::parseInt(c)
@@ -241,6 +263,11 @@ module ParsePositions {
     isArgBody(c) and
     c = paramName + ":"
   }
+
+  predicate isParsedElementLowerBoundPosition(string c, int lower) {
+    isElementBody(c) and
+    lower = AccessPath::parseLowerBound(c)
+  }
 }
 
 /** Gets the argument position obtained by parsing `X` in `Parameter[X]`. */

ruby/ql/lib/codeql/ruby/frameworks/Core.qll

@@ -10,6 +10,7 @@ import core.Object::Object
 import core.Kernel::Kernel
 import core.Module
 import core.Array
+import core.Hash
 import core.String
 import core.Regexp
 import core.IO