Merge pull request #7568 from aibaars/ruby-pattern-matching-taint

Ruby: taint steps for pattern matches

Author: aibaars

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

@@ -97,7 +97,10 @@ private class TPattern =
  * ```
  */
 class CasePattern extends AstNode, TPattern {
-  CasePattern() { casePattern(toGenerated(this)) }
+  CasePattern() {
+    casePattern(toGenerated(this)) or
+    synthChild(any(HashPattern p), _, this)
+  }
 }
 
 /**
@@ -207,7 +210,7 @@ class FindPattern extends CasePattern, TFindPattern {
   CasePattern getAnElement() { result = this.getElement(_) }
 
   /**
-   * Gets the variable for the prefix of this list pattern, if any. For example `init` in:
+   * Gets the variable for the prefix of this find pattern, if any. For example `init` in:
    * ```rb
    * in List[*init, "a", Integer => x, *tail]
    * ```
@@ -217,7 +220,7 @@ class FindPattern extends CasePattern, TFindPattern {
   }
 
   /**
-   * Gets the variable for the suffix of this list pattern, if any. For example `tail` in:
+   * Gets the variable for the suffix of this find pattern, if any. For example `tail` in:
    * ```rb
    * in List[*init, "a", Integer => x, *tail]
    * ```
@@ -267,7 +270,10 @@ class HashPattern extends CasePattern, THashPattern {
   StringlikeLiteral getKey(int n) { toGenerated(result) = this.keyValuePair(n).getKey() }
 
   /** Gets the value of the `n`th pair. */
-  CasePattern getValue(int n) { toGenerated(result) = this.keyValuePair(n).getValue() }
+  CasePattern getValue(int n) {
+    toGenerated(result) = this.keyValuePair(n).getValue() or
+    synthChild(this, n, result)
+  }
 
   /** Gets the value for a given key name. */
   CasePattern getValueByKey(string key) {
@@ -383,6 +389,7 @@ class ParenthesizedPattern extends CasePattern, TParenthesizedPattern {
 
   ParenthesizedPattern() { this = TParenthesizedPattern(g) }
 
+  /** Gets the underlying pattern. */
   final CasePattern getPattern() { toGenerated(result) = g.getChild() }
 
   final override string getAPrimaryQlClass() { result = "ParenthesizedPattern" }

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

@@ -115,6 +115,8 @@ class VariableAccess extends Expr instanceof VariableAccessImpl {
     implicitWriteAccess(toGenerated(this))
     or
     this = any(SimpleParameterSynthImpl p).getDefininingAccess()
+    or
+    this = any(HashPattern p).getValue(_)
   }
 
   final override string toString() { result = VariableAccessImpl.super.toString() }

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

@@ -7,6 +7,7 @@ private import codeql.ruby.ast.internal.Expr
 private import codeql.ruby.ast.internal.Variable
 private import codeql.ruby.ast.internal.Pattern
 private import codeql.ruby.ast.internal.Scope
+private import codeql.ruby.ast.internal.TreeSitter
 private import codeql.ruby.AST
 
 /** A synthesized AST node kind. */
@@ -921,3 +922,35 @@ private module ForLoopDesugar {
     }
   }
 }
+
+/**
+ * ```rb
+ * { a: }
+ * ```
+ * desugars to,
+ * ```rb
+ * { a: a }
+ * ```
+ */
+private module ImplicitHashValueSynthesis {
+  private Ruby::AstNode keyWithoutValue(HashPattern parent, int i) {
+    exists(Ruby::KeywordPattern pair |
+      result = pair.getKey() and
+      result = toGenerated(parent.getKey(i)) and
+      not exists(pair.getValue())
+    )
+  }
+
+  private class ImplicitHashValueSynthesis extends Synthesis {
+    final override predicate child(AstNode parent, int i, Child child) {
+      exists(TVariableReal variable |
+        access(keyWithoutValue(parent, i), variable) and
+        child = SynthChild(LocalVariableAccessRealKind(variable))
+      )
+    }
+
+    final override predicate location(AstNode n, Location l) {
+      exists(HashPattern p, int i | n = p.getValue(i) and l = keyWithoutValue(p, i).getLocation())
+    }
+  }
+}

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

@@ -39,6 +39,8 @@ predicate implicitAssignmentNode(Ruby::AstNode n) {
   or
   n = any(Ruby::HashPattern parent).getChild(_).(Ruby::HashSplatParameter).getName()
   or
+  n = any(Ruby::KeywordPattern parent | not exists(parent.getValue())).getKey()
+  or
   n = any(Ruby::ExceptionVariable ev).getChild()
   or
   n = any(Ruby::For for).getPattern()
@@ -104,11 +106,23 @@ private predicate scopeDefinesParameterVariable(
   )
 }
 
+pragma[nomagic]
+private string variableNameInScope(Ruby::AstNode i, Scope::Range scope) {
+  scope = scopeOf(i) and
+  (
+    result = i.(Ruby::Identifier).getValue()
+    or
+    exists(Ruby::KeywordPattern p | i = p.getKey() and not exists(p.getValue()) |
+      result = i.(Ruby::String).getChild(0).(Ruby::StringContent).getValue() or
+      result = i.(Ruby::HashKeySymbol).getValue()
+    )
+  )
+}
+
 /** Holds if `name` is assigned in `scope` at `i`. */
-private predicate scopeAssigns(Scope::Range scope, string name, Ruby::Identifier i) {
+private predicate scopeAssigns(Scope::Range scope, string name, Ruby::AstNode i) {
   (explicitAssignmentNode(i, _) or implicitAssignmentNode(i)) and
-  name = i.getValue() and
-  scope = scopeOf(i)
+  name = variableNameInScope(i, scope)
 }
 
 cached
@@ -132,11 +146,11 @@ private module Cached {
           other order by other.getLocation().getStartLine(), other.getLocation().getStartColumn()
         )
     } or
-    TLocalVariableReal(Scope::Range scope, string name, Ruby::Identifier i) {
+    TLocalVariableReal(Scope::Range scope, string name, Ruby::AstNode i) {
       scopeDefinesParameterVariable(scope, name, i)
       or
       i =
-        min(Ruby::Identifier other |
+        min(Ruby::AstNode other |
           scopeAssigns(scope, name, other)
         |
           other order by other.getLocation().getStartLine(), other.getLocation().getStartColumn()
@@ -295,13 +309,18 @@ private module Cached {
     i = any(Ruby::WhileModifier x).getBody()
   }
 
+  pragma[nomagic]
+  private predicate hasScopeAndName(VariableReal variable, Scope::Range scope, string name) {
+    variable.getNameImpl() = name and
+    scope = variable.getDeclaringScopeImpl()
+  }
+
   cached
-  predicate access(Ruby::Identifier access, VariableReal variable) {
-    exists(string name |
-      variable.getNameImpl() = name and
-      name = access.getValue()
+  predicate access(Ruby::AstNode access, VariableReal variable) {
+    exists(string name, Scope::Range scope |
+      pragma[only_bind_into](name) = variableNameInScope(access, scope)
     |
-      variable.getDeclaringScopeImpl() = scopeOf(access) and
+      hasScopeAndName(variable, scope, name) and
       not access.getLocation().strictlyBefore(variable.getLocationImpl()) and
       // In case of overlapping parameter names, later parameters should not
       // be considered accesses to the first parameter
@@ -310,15 +329,15 @@ private module Cached {
       else any()
       or
       exists(Scope::Range declScope |
-        variable.getDeclaringScopeImpl() = declScope and
-        inherits(scopeOf(access), name, declScope)
+        hasScopeAndName(variable, declScope, pragma[only_bind_into](name)) and
+        inherits(scope, name, declScope)
       )
     )
   }
 
   private class Access extends Ruby::Token {
     Access() {
-      access(this, _) or
+      access(this.(Ruby::Identifier), _) or
       this instanceof Ruby::GlobalVariable or
       this instanceof Ruby::InstanceVariable or
       this instanceof Ruby::ClassVariable or
@@ -371,7 +390,7 @@ private predicate inherits(Scope::Range scope, string name, Scope::Range outer)
     (
       scopeDefinesParameterVariable(outer, name, _)
       or
-      exists(Ruby::Identifier i |
+      exists(Ruby::AstNode i |
         scopeAssigns(outer, name, i) and
         i.getLocation().strictlyBefore(scope.getLocation())
       )
@@ -420,7 +439,7 @@ private class VariableRealAdapter extends VariableImpl, TVariableReal instanceof
 class LocalVariableReal extends VariableReal, TLocalVariableReal {
   private Scope::Range scope;
   private string name;
-  private Ruby::Identifier i;
+  private Ruby::AstNode i;
 
   LocalVariableReal() { this = TLocalVariableReal(scope, name, i) }