Merge pull request #7914 from hvitved/ruby/generalize-element-content

Ruby: Generalize `ArrayElementContent` to `ElementContent`

Author: hvitved

ruby/ql/lib/codeql/ruby/ast/Constant.qll

@@ -105,6 +105,38 @@ class ConstantValue extends TConstantValue {
 
   /** Holds if this is the `nil` value. */
   predicate isNil() { this = TNil() }
+
+  /** Gets a (unique) serialized version of this value. */
+  final string serialize() {
+    result = this.getInt().toString()
+    or
+    exists(string res | res = this.getFloat().toString() |
+      if exists(res.indexOf(".")) then result = res else result = res + ".0"
+    )
+    or
+    exists(int numerator, int denominator |
+      this.isRational(numerator, denominator) and
+      result = numerator + "/" + denominator
+    )
+    or
+    exists(float real, float imaginary |
+      this.isComplex(real, imaginary) and
+      result = real + "+" + imaginary + "i"
+    )
+    or
+    result = "\"" + this.getString().replaceAll("\"", "\\\"") + "\""
+    or
+    result = ":" + this.getSymbol()
+    or
+    exists(string s, string flags |
+      this.isRegExpWithFlags(s, flags) and
+      result = "/" + s + "/" + flags
+    )
+    or
+    result = this.getBoolean().toString()
+    or
+    this.isNil() and result = "nil"
+  }
 }
 
 /** Provides different sub classes of `ConstantValue`. */

ruby/ql/lib/codeql/ruby/ast/internal/Constant.qll

@@ -404,7 +404,7 @@ cached
 private module Cached {
   cached
   newtype TConstantValue =
-    TInt(int i) { isInt(_, i) or isIntExpr(_, i) } or
+    TInt(int i) { isInt(_, i) or isIntExpr(_, i) or i in [0 .. 100] } or
     TFloat(float f) { isFloat(_, f) or isFloatExpr(_, f) } or
     TRational(int numerator, int denominator) {
       isRational(_, numerator, denominator) or

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

@@ -31,33 +31,27 @@ module SummaryComponent {
   /** Gets a summary component that represents a block argument. */
   SummaryComponent block() { result = argument(any(ParameterPosition pos | pos.isBlock())) }
 
-  /** Gets a summary component that represents an element in an array at an unknown index. */
-  SummaryComponent arrayElementUnknown() {
-    result = SC::content(TSingletonContent(TUnknownArrayElementContent()))
+  /** Gets a summary component that represents an element in a collection at an unknown index. */
+  SummaryComponent elementUnknown() {
+    result = SC::content(TSingletonContent(TUnknownElementContent()))
   }
 
-  /**
-   * Gets a summary component that represents an element in an array at a known index.
-   *
-   * Has no result for negative indices. Wrap-around interpretation of negative indices should be
-   * handled by the caller, if modeling a function that has such behavior.
-   */
-  bindingset[i]
-  SummaryComponent arrayElementKnown(int i) {
-    result = SC::content(TSingletonContent(TKnownArrayElementContent(i)))
-    or
-    // `i` may be out of range
-    i >= 0 and
-    not exists(TKnownArrayElementContent(i)) and
-    result = arrayElementUnknown()
+  /** Gets a summary component that represents an element in a collection at a known index. */
+  SummaryComponent elementKnown(ConstantValue cv) {
+    result = SC::content(TSingletonContent(DataFlow::Content::getElementContent(cv)))
   }
 
   /**
-   * Gets a summary component that represents an element in an array at either an unknown
-   * index or known index. This predicate should never be used in the output specification
-   * of a flow summary; use `arrayElementUnknown()` instead.
+   * Gets a summary component that represents an element in a collection at either an unknown
+   * index or known index. This has the same semantics as
+   *
+   * ```ql
+   * elementKnown() or elementUnknown(_)
+   * ```
+   *
+   * but is more efficient, because it is represented by a single value.
    */
-  SummaryComponent arrayElementAny() { result = SC::content(TAnyArrayElementContent()) }
+  SummaryComponent elementAny() { result = SC::content(TAnyElementContent()) }
 
   /** 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

@@ -319,12 +319,15 @@ private module Cached {
   cached
   newtype TContentSet =
     TSingletonContent(Content c) or
-    TAnyArrayElementContent()
+    TAnyElementContent()
 
   cached
   newtype TContent =
-    TKnownArrayElementContent(int i) { i in [0 .. 10] } or
-    TUnknownArrayElementContent()
+    TKnownElementContent(ConstantValue cv) {
+      not cv.isInt(_) or
+      cv.getInt() in [0 .. 10]
+    } or
+    TUnknownElementContent()
 
   /**
    * Holds if `e` is an `ExprNode` that may be returned by a call to `c`.
@@ -341,7 +344,7 @@ private module Cached {
   }
 }
 
-class TArrayElementContent = TKnownArrayElementContent or TUnknownArrayElementContent;
+class TElementContent = TKnownElementContent or TUnknownElementContent;
 
 import Cached
 
@@ -862,7 +865,7 @@ int accessPathLimit() { result = 5 }
  * Holds if access paths with `c` at their head always should be tracked at high
  * precision. This disables adaptive access path precision for such access paths.
  */
-predicate forceHighPrecision(Content c) { c instanceof Content::ArrayElementContent }
+predicate forceHighPrecision(Content c) { c instanceof Content::ElementContent }
 
 /** The unit type. */
 private newtype TUnit = TMkUnit()

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

@@ -203,24 +203,32 @@ class Content extends TContent {
 
 /** Provides different sub classes of `Content`. */
 module Content {
-  /** An element in an array. */
-  class ArrayElementContent extends Content, TArrayElementContent { }
+  /** An element in a collection, for example an element in an array or in a hash. */
+  class ElementContent extends Content, TElementContent { }
 
-  /** An element in an array at a known index. */
-  class KnownArrayElementContent extends ArrayElementContent, TKnownArrayElementContent {
-    private int i;
+  /** An element in a collection at a known index. */
+  class KnownElementContent extends ElementContent, TKnownElementContent {
+    private ConstantValue cv;
 
-    KnownArrayElementContent() { this = TKnownArrayElementContent(i) }
+    KnownElementContent() { this = TKnownElementContent(cv) }
 
-    /** Gets the index in the array. */
-    int getIndex() { result = i }
+    /** Gets the index in the collection. */
+    ConstantValue getIndex() { result = cv }
 
-    override string toString() { result = "array element " + i }
+    override string toString() { result = "element " + cv }
   }
 
-  /** An element in an array at an unknown index. */
-  class UnknownArrayElementContent extends ArrayElementContent, TUnknownArrayElementContent {
-    override string toString() { result = "array element" }
+  /** An element in a collection at an unknown index. */
+  class UnknownElementContent extends ElementContent, TUnknownElementContent {
+    override string toString() { result = "element" }
+  }
+
+  /** Gets the element content corresponding to constant value `cv`. */
+  ElementContent getElementContent(ConstantValue cv) {
+    result = TKnownElementContent(cv)
+    or
+    not exists(TKnownElementContent(cv)) and
+    result = TUnknownElementContent()
   }
 }
 
@@ -234,8 +242,8 @@ class ContentSet extends TContentSet {
   /** Holds if this content set is the singleton `{c}`. */
   predicate isSingleton(Content c) { this = TSingletonContent(c) }
 
-  /** Holds if this content set represent all `ArrayElementContent`s. */
-  predicate isAnyArrayElement() { this = TAnyArrayElementContent() }
+  /** Holds if this content set represents all `ElementContent`s. */
+  predicate isAnyElement() { this = TAnyElementContent() }
 
   /** Gets a textual representation of this content set. */
   string toString() {
@@ -244,24 +252,24 @@ class ContentSet extends TContentSet {
       result = c.toString()
     )
     or
-    this.isAnyArrayElement() and
+    this.isAnyElement() and
     result = "any array element"
   }
 
   /** Gets a content that may be stored into when storing into this set. */
   Content getAStoreContent() {
     this.isSingleton(result)
     or
-    this.isAnyArrayElement() and
-    result = TUnknownArrayElementContent()
+    this.isAnyElement() and
+    result = TUnknownElementContent()
   }
 
   /** Gets a content that may be read from when reading from this set. */
   Content getAReadContent() {
     this.isSingleton(result)
     or
-    this.isAnyArrayElement() and
-    result instanceof Content::ArrayElementContent
+    this.isAnyElement() and
+    result instanceof Content::ElementContent
   }
 }
 

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

@@ -73,19 +73,19 @@ SummaryComponent interpretComponentSpecific(AccessPathToken c) {
     ppos.isPositionalLowerBound(AccessPath::parseLowerBound(arg))
   )
   or
-  c.getName() = "ArrayElement" and
-  (
-    c.getNumArgument() = 0 and
-    result = FlowSummary::SummaryComponent::arrayElementAny()
+  c.getName() = "Element" and
+  exists(string arg | arg = c.getAnArgument() |
+    arg = "?" and
+    result = FlowSummary::SummaryComponent::elementUnknown()
     or
-    exists(string arg | arg = c.getAnArgument() |
-      arg = "?" and
-      result = FlowSummary::SummaryComponent::arrayElementUnknown()
+    arg = "any" and
+    result = FlowSummary::SummaryComponent::elementAny()
+    or
+    exists(ConstantValue cv | result = FlowSummary::SummaryComponent::elementKnown(cv) |
+      cv.isInt(AccessPath::parseInt(arg))
       or
-      exists(int i |
-        i = AccessPath::parseInt(c.getAnArgument()) and
-        result = FlowSummary::SummaryComponent::arrayElementKnown(i)
-      )
+      not exists(AccessPath::parseInt(arg)) and
+      cv.serialize() = c.getAnArgument()
     )
   )
 }

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

@@ -103,9 +103,9 @@ private module Cached {
     // allow flow out of a _tainted_ collection. This is needed in order to support taint-
     // tracking configurations where the source is a collection.
     exists(DataFlow::ContentSet c | readStep(nodeFrom, c, nodeTo) |
-      c.isSingleton(any(DataFlow::Content::ArrayElementContent aec))
+      c.isSingleton(any(DataFlow::Content::ElementContent ec))
       or
-      c.isAnyArrayElement()
+      c.isAnyElement()
     )
   }
 

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

@@ -62,7 +62,7 @@ private class SplatSummary extends SummarizedCallable {
     (
       // *1 = [1]
       input = "Argument[self]" and
-      output = "ReturnValue.ArrayElement[0]"
+      output = "ReturnValue.Element[0]"
       or
       // *[1] = [1]
       input = "Argument[self]" and