Merge pull request #7985 from github/nickrolfe/constant_regexp

Ruby: separate constant propagation of regexps from strings

Author: nickrolfe

ruby/ql/lib/change-notes/2022-03-25-deprecate-getStringOrSymbol.md

@@ -0,0 +1,5 @@
+---
+ category: minorAnalysis
+---
+ *  Whereas `ConstantValue::getString()` previously returned both string and regular-expression values, it now returns only string values. The same applies to `ConstantValue::isString(value)`.
+ *  Regular-expression values can now be accessed with the new predicates `ConstantValue::getRegExp()`, `ConstantValue::isRegExp(value)`, and `ConstantValue::isRegExpWithFlags(value, flags)`.

ruby/ql/lib/change-notes/2022-03-25-regexp-constant-values.md

@@ -0,0 +1,4 @@
+---
+ category: deprecated
+---
+ * `ConstantValue::getStringOrSymbol` and `ConstantValue::isStringOrSymbol`, which return/hold for all string-like values (strings, symbols, and regular expressions), have been renamed to `ConstantValue::getStringlikeValue` and `ConstantValue::isStringlikeValue`, respectively. The old names have been marked as `deprecated`.

ruby/ql/lib/codeql/ruby/Concepts.qll

@@ -268,7 +268,7 @@ module HTTP {
         string getUrlPattern() {
           exists(CfgNodes::ExprNodes::StringlikeLiteralCfgNode strNode |
             this.getUrlPatternArg().getALocalSource() = DataFlow::exprNode(strNode) and
-            result = strNode.getExpr().getConstantValue().getStringOrSymbol()
+            result = strNode.getExpr().getConstantValue().getStringlikeValue()
           )
         }
 
@@ -431,7 +431,7 @@ module HTTP {
         string getMimetype() {
           exists(CfgNodes::ExprNodes::StringlikeLiteralCfgNode strNode |
             this.getMimetypeOrContentTypeArg().getALocalSource() = DataFlow::exprNode(strNode) and
-            result = strNode.getExpr().getConstantValue().getStringOrSymbol().splitAt(";", 0)
+            result = strNode.getExpr().getConstantValue().getStringlikeValue().splitAt(";", 0)
           )
           or
           not exists(this.getMimetypeOrContentTypeArg()) and

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

@@ -8,28 +8,37 @@ private import internal.TreeSitter
 /** A constant value. */
 class ConstantValue extends TConstantValue {
   /** Gets a textual representation of this constant value. */
-  final string toString() {
-    result = this.getInt().toString()
+  final string toString() { this.hasValueWithType(result, _) }
+
+  /** Gets a string describing the type of this constant value. */
+  string getValueType() { this.hasValueWithType(_, result) }
+
+  private predicate hasValueWithType(string value, string type) {
+    value = this.getInt().toString() and type = "int"
     or
-    result = this.getFloat().toString()
+    value = this.getFloat().toString() and type = "float"
     or
     exists(int numerator, int denominator |
       this.isRational(numerator, denominator) and
-      result = numerator + "/" + denominator
+      value = numerator + "/" + denominator and
+      type = "rational"
     )
     or
     exists(float real, float imaginary |
       this.isComplex(real, imaginary) and
-      result = real + "+" + imaginary + "i"
+      value = real + "+" + imaginary + "i" and
+      type = "complex"
     )
     or
-    result = this.getString()
+    value = this.getString() and type = "string"
+    or
+    value = ":" + this.getSymbol() and type = "symbol"
     or
-    result = ":" + this.getSymbol()
+    value = this.getRegExp() and type = "regexp"
     or
-    result = this.getBoolean().toString()
+    value = this.getBoolean().toString() and type = "boolean"
     or
-    this.isNil() and result = "nil"
+    this.isNil() and value = "nil" and type = "nil"
   }
 
   /** Gets the integer value, if this is an integer. */
@@ -62,11 +71,31 @@ class ConstantValue extends TConstantValue {
   /** Holds if this is the symbol value `:s`. */
   predicate isSymbol(string s) { s = this.getSymbol() }
 
-  /** Gets the string or symbol value, if any. */
-  string getStringOrSymbol() { result = [this.getString(), this.getSymbol()] }
+  /** Gets the regexp value, if this is a regexp. */
+  string getRegExp() { this.isRegExpWithFlags(result, _) }
+
+  /** Holds if this is the regexp value `/s/`, ignoring any flags. */
+  predicate isRegExp(string s) { this.isRegExpWithFlags(s, _) }
+
+  /** Holds if this is the regexp value `/s/flags` . */
+  predicate isRegExpWithFlags(string s, string flags) { this = TRegExp(s, flags) }
+
+  /** DEPRECATED: Use `getStringlikeValue` instead. */
+  deprecated string getStringOrSymbol() { result = this.getStringlikeValue() }
 
-  /** Holds if this is the string value `s` or the symbol value `:s`. */
-  predicate isStringOrSymbol(string s) { s = this.getStringOrSymbol() }
+  /** DEPRECATED: Use `isStringlikeValue` instead. */
+  deprecated predicate isStringOrSymbol(string s) { s = this.getStringlikeValue() }
+
+  /** Gets the string/symbol/regexp value, if any. */
+  string getStringlikeValue() { result = [this.getString(), this.getSymbol(), this.getRegExp()] }
+
+  /**
+   * Holds if this is:
+   * - the string value `s`,
+   * - the symbol value `:s`, or
+   * - the regexp value `/s/`.
+   */
+  predicate isStringlikeValue(string s) { s = this.getStringlikeValue() }
 
   /** Gets the Boolean value, if this is a Boolean. */
   boolean getBoolean() { this = TBoolean(result) }
@@ -92,11 +121,17 @@ module ConstantValue {
   /** A constant complex value. */
   class ConstantComplexValue extends ConstantValue, TComplex { }
 
+  /** A constant string-like value. */
+  class ConstantStringlikeValue extends ConstantValue, TStringlike { }
+
   /** A constant string value. */
-  class ConstantStringValue extends ConstantValue, TString { }
+  class ConstantStringValue extends ConstantStringlikeValue, TString { }
 
   /** A constant symbol value. */
-  class ConstantSymbolValue extends ConstantValue, TSymbol { }
+  class ConstantSymbolValue extends ConstantStringlikeValue, TSymbol { }
+
+  /** A constant regexp value. */
+  class ConstantRegExpValue extends ConstantStringlikeValue, TRegExp { }
 
   /** A constant Boolean value. */
   class ConstantBooleanValue extends ConstantValue, TBoolean { }

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

@@ -454,11 +454,11 @@ class StringConcatenation extends Expr, TStringConcatenation {
    */
   final string getConcatenatedValueText() {
     forall(StringLiteral c | c = this.getString(_) |
-      exists(c.getConstantValue().getStringOrSymbol())
+      exists(c.getConstantValue().getStringlikeValue())
     ) and
     result =
       concat(string valueText, int i |
-        valueText = this.getString(i).getConstantValue().getStringOrSymbol()
+        valueText = this.getString(i).getConstantValue().getStringlikeValue()
       |
         valueText order by i
       )

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

@@ -53,7 +53,7 @@ private class MethodModifier extends MethodCall {
   predicate modifiesMethod(Namespace n, string name) {
     this = n.getAStmt() and
     [
-      this.getMethodArgument().getConstantValue().getStringOrSymbol(),
+      this.getMethodArgument().getConstantValue().getStringlikeValue(),
       this.getMethodArgument().(MethodBase).getName()
     ] = name
   }

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

@@ -278,7 +278,7 @@ class HashPattern extends CasePattern, THashPattern {
   /** Gets the value for a given key name. */
   CasePattern getValueByKey(string key) {
     exists(int i |
-      this.getKey(i).getConstantValue().isStringOrSymbol(key) and result = this.getValue(i)
+      this.getKey(i).getConstantValue().isStringlikeValue(key) and result = this.getValue(i)
     )
   }
 

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

@@ -229,9 +229,16 @@ private module Propagation {
     }
 
     pragma[nomagic]
-    string getNonSymbolValue() {
+    string getStringValue() {
       result = this.getValue() and
-      not this.getExpr() instanceof SymbolLiteral
+      not this.getExpr() instanceof SymbolLiteral and
+      not this.getExpr() instanceof RegExpLiteral
+    }
+
+    pragma[nomagic]
+    string getRegExpValue(string flags) {
+      result = this.getValue() and
+      flags = this.getExpr().(RegExpLiteral).getFlagString()
     }
   }
 
@@ -251,7 +258,7 @@ private module Propagation {
       s = left + right
     )
     or
-    s = e.(StringlikeLiteralWithInterpolationCfgNode).getNonSymbolValue()
+    s = e.(StringlikeLiteralWithInterpolationCfgNode).getStringValue()
     or
     // If last statement in the interpolation is a constant or local variable read,
     // we attempt to look up its string value.
@@ -267,13 +274,15 @@ private module Propagation {
     exists(ExprCfgNode last | last = e.(RegExpInterpolationComponentCfgNode).getLastStmt() |
       isInt(last, any(int i | s = i.toString())) or
       isFloat(last, any(float f | s = f.toString())) or
-      isString(last, s)
+      isString(last, s) or
+      isRegExp(last, s, _) // Note: we lose the flags for interpolated regexps here.
     )
   }
 
   private predicate isStringExprNoCfg(Expr e, string s) {
     s = e.(StringlikeLiteralImpl).getStringValue() and
-    not e instanceof SymbolLiteral
+    not e instanceof SymbolLiteral and
+    not e instanceof RegExpLiteral
     or
     s = e.(EncodingLiteralImpl).getValue()
     or
@@ -319,6 +328,31 @@ private module Propagation {
     forex(ExprCfgNode n | n = e.getAControlFlowNode() | isSymbol(n, s))
   }
 
+  predicate isRegExp(ExprCfgNode e, string s, string flags) {
+    isRegExpExprNoCfg(e.getExpr(), s, flags)
+    or
+    isRegExpExpr(e.getExpr().(ConstantReadAccess).getValue(), s, flags)
+    or
+    isRegExp(getSource(e), s, flags)
+    or
+    s = e.(StringlikeLiteralWithInterpolationCfgNode).getRegExpValue(flags)
+  }
+
+  private predicate isRegExpExprNoCfg(Expr e, string s, string flags) {
+    s = e.(StringlikeLiteralImpl).getStringValue() and
+    e.(RegExpLiteral).getFlagString() = flags
+    or
+    isRegExpExprNoCfg(e.(ConstantReadAccess).getValue(), s, flags)
+  }
+
+  predicate isRegExpExpr(Expr e, string s, string flags) {
+    isRegExpExprNoCfg(e, s, flags)
+    or
+    isRegExpExpr(e.(ConstantReadAccess).getValue(), s, flags)
+    or
+    forex(ExprCfgNode n | n = e.getAControlFlowNode() | isRegExp(n, s, flags))
+  }
+
   predicate isBoolean(ExprCfgNode e, boolean b) {
     isBooleanExprNoCfg(e.getExpr(), b)
     or
@@ -388,9 +422,18 @@ private module Cached {
       s = any(StringComponentImpl c).getValue()
     } or
     TSymbol(string s) { isString(_, s) or isSymbolExpr(_, s) } or
+    TRegExp(string s, string flags) {
+      isRegExp(_, s, flags)
+      or
+      isRegExpExpr(_, s, flags)
+      or
+      s = any(StringComponentImpl c).getValue() and flags = ""
+    } or
     TBoolean(boolean b) { b in [false, true] } or
     TNil()
 
+  class TStringlike = TString or TSymbol or TRegExp;
+
   cached
   ConstantValue getConstantValue(ExprCfgNode n) {
     result.isInt(any(int i | isInt(n, i)))
@@ -411,6 +454,8 @@ private module Cached {
     or
     result.isSymbol(any(string s | isSymbol(n, s)))
     or
+    exists(string s, string flags | isRegExp(n, s, flags) and result = TRegExp(s, flags))
+    or
     result.isBoolean(any(boolean b | isBoolean(n, b)))
     or
     result.isNil() and
@@ -437,6 +482,8 @@ private module Cached {
     or
     result.isSymbol(any(string s | isSymbolExpr(e, s)))
     or
+    exists(string s, string flags | isRegExpExpr(e, s, flags) and result = TRegExp(s, flags))
+    or
     result.isBoolean(any(boolean b | isBooleanExpr(e, b)))
     or
     result.isNil() and

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

@@ -189,7 +189,7 @@ class RedirectToCall extends ActionControllerContextCall {
   /** Gets the `ActionControllerActionMethod` to redirect to, if any */
   ActionControllerActionMethod getRedirectActionMethod() {
     exists(string methodName |
-      this.getKeywordArgument("action").getConstantValue().isStringOrSymbol(methodName) and
+      this.getKeywordArgument("action").getConstantValue().isStringlikeValue(methodName) and
       methodName = result.getName() and
       result.getEnclosingModule() = this.getControllerClass()
     )
@@ -225,7 +225,7 @@ pragma[nomagic]
 private predicate actionControllerHasHelperMethodCall(ActionControllerControllerClass c, string name) {
   exists(MethodCall mc |
     mc.getMethodName() = "helper_method" and
-    mc.getAnArgument().getConstantValue().isStringOrSymbol(name) and
+    mc.getAnArgument().getConstantValue().isStringlikeValue(name) and
     mc.getEnclosingModule() = c
   )
 }
@@ -317,7 +317,7 @@ class ActionControllerSkipForgeryProtectionCall extends CSRFProtectionSetting::R
       call.getMethodName() = "skip_forgery_protection"
       or
       call.getMethodName() = "skip_before_action" and
-      call.getAnArgument().getConstantValue().isStringOrSymbol("verify_authenticity_token")
+      call.getAnArgument().getConstantValue().isStringlikeValue("verify_authenticity_token")
     )
   }