Ruby: Support self,block in Argument/Parameter tokens

Author: asgerf

ruby/ql/lib/codeql/ruby/ApiGraphs.qll

@@ -473,36 +473,6 @@ module API {
     /** Gets a data flow node that flows to the RHS of a def-node. */
     private DataFlow::LocalSourceNode defCand() { result = defCand(TypeBackTracker::end()) }
 
-    private Label::ApiLabel getLabelFromArgumentPosition(DataFlowDispatch::ArgumentPosition pos) {
-      exists(int n |
-        pos.isPositional(n) and
-        result = Label::parameter(n)
-      )
-      or
-      exists(string name |
-        pos.isKeyword(name) and
-        result = Label::keywordParameter(name)
-      )
-      or
-      pos.isBlock() and
-      result = Label::blockParameter()
-    }
-
-    private Label::ApiLabel getLabelFromParameterPosition(DataFlowDispatch::ParameterPosition pos) {
-      exists(int n |
-        pos.isPositional(n) and
-        result = Label::parameter(n)
-      )
-      or
-      exists(string name |
-        pos.isKeyword(name) and
-        result = Label::keywordParameter(name)
-      )
-      or
-      pos.isBlock() and
-      result = Label::blockParameter()
-    }
-
     /**
      * Holds if there should be a `lbl`-edge from the given call to an argument.
      */
@@ -512,7 +482,7 @@ module API {
     ) {
       exists(DataFlowDispatch::ArgumentPosition argPos |
         argument.sourceArgumentOf(call.asExpr(), argPos) and
-        lbl = getLabelFromArgumentPosition(argPos)
+        lbl = Label::getLabelFromArgumentPosition(argPos)
       )
     }
 
@@ -525,7 +495,7 @@ module API {
     ) {
       exists(DataFlowDispatch::ParameterPosition paramPos |
         paramNode.isSourceParameterOf(callable.asExpr().getExpr(), paramPos) and
-        lbl = getLabelFromParameterPosition(paramPos)
+        lbl = Label::getLabelFromParameterPosition(paramPos)
       )
     }
 
@@ -803,5 +773,37 @@ module API {
 
     /** Gets the label for the edge from the root node to a custom entry point of the given name. */
     LabelEntryPoint entryPoint(API::EntryPoint name) { result.getName() = name }
+
+    /** Gets the API graph label corresponding to the given argument position. */
+    Label::ApiLabel getLabelFromArgumentPosition(DataFlowDispatch::ArgumentPosition pos) {
+      exists(int n |
+        pos.isPositional(n) and
+        result = Label::parameter(n)
+      )
+      or
+      exists(string name |
+        pos.isKeyword(name) and
+        result = Label::keywordParameter(name)
+      )
+      or
+      pos.isBlock() and
+      result = Label::blockParameter()
+    }
+
+    /** Gets the API graph label corresponding to the given parameter position. */
+    Label::ApiLabel getLabelFromParameterPosition(DataFlowDispatch::ParameterPosition pos) {
+      exists(int n |
+        pos.isPositional(n) and
+        result = Label::parameter(n)
+      )
+      or
+      exists(string name |
+        pos.isKeyword(name) and
+        result = Label::keywordParameter(name)
+      )
+      or
+      pos.isBlock() and
+      result = Label::blockParameter()
+    }
   }
 }

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

@@ -55,15 +55,14 @@ predicate summaryElement(DataFlowCallable c, string input, string output, string
 /**
  * Gets the summary component for specification component `c`, if any.
  *
- * This covers all the Ruby-specific components of a flow summary, and
- * is currently restricted to `"BlockArgument"`.
+ * This covers all the Ruby-specific components of a flow summary.
  */
 bindingset[c]
 SummaryComponent interpretComponentSpecific(AccessPathToken c) {
-  c = "Receiver" and
+  c = "Receiver" and // TODO: replace with Argument[self]
   result = FlowSummary::SummaryComponent::receiver()
   or
-  c = "BlockArgument" and
+  c = "BlockArgument" and // TODO: replace with Argument[block]
   result = FlowSummary::SummaryComponent::block()
   or
   c = "Argument[_]" and
@@ -85,7 +84,7 @@ SummaryComponent interpretComponentSpecific(AccessPathToken c) {
 /** Gets the textual representation of a summary component in the format used for flow summaries. */
 string getComponentSpecificCsv(SummaryComponent sc) {
   sc = TArgumentSummaryComponent(any(ParameterPosition pos | pos.isBlock())) and
-  result = "BlockArgument"
+  result = "BlockArgument" // TODO: replace with Argument[block]
 }
 
 /** Gets the textual representation of a parameter position in the format used for flow summaries. */
@@ -184,6 +183,12 @@ ArgumentPosition parseParamBody(string s) {
     ParsePositions::isParsedParameterPosition(s, i) and
     result.isPositional(i)
   )
+  or
+  s = "self" and
+  result.isSelf()
+  or
+  s = "block" and
+  result.isBlock()
 }
 
 /** Gets the parameter position obtained by parsing `X` in `Argument[X]`. */
@@ -192,4 +197,10 @@ ParameterPosition parseArgBody(string s) {
     ParsePositions::isParsedArgumentPosition(s, i) and
     result.isPositional(i)
   )
+  or
+  s = "self" and
+  result.isSelf()
+  or
+  s = "block" and
+  result.isBlock()
 }

ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsSpecific.qll

@@ -26,6 +26,8 @@ class Unit = DataFlowPrivate::Unit;
 import codeql.ruby.ApiGraphs
 import codeql.ruby.dataflow.internal.AccessPathSyntax as AccessPathSyntax
 private import AccessPathSyntax
+private import codeql.ruby.dataflow.internal.FlowSummaryImplSpecific as FlowSummaryImplSpecific
+private import codeql.ruby.dataflow.internal.DataFlowDispatch as DataFlowDispatch
 
 /**
  * Holds if models describing `package` may be relevant for the analysis of this database.
@@ -107,8 +109,13 @@ API::Node getExtraSuccessorFromNode(API::Node node, AccessPathToken token) {
   token.getName() = "Instance" and
   result = node.getInstance()
   or
-  token.getName() = "BlockArgument" and
+  token.getName() = "BlockArgument" and // TODO: replace with Argument[block]
   result = node.getBlock()
+  or
+  token.getName() = "Parameter" and
+  result =
+    node.getASuccessor(API::Label::getLabelFromArgumentPosition(FlowSummaryImplSpecific::parseParamBody(token
+              .getAnArgument())))
   // Note: The "ArrayElement" token is not implemented yet, as it ultimately requires type-tracking and
   // API graphs to be aware of the steps involving ArrayElement contributed by the standard library model.
   // Type-tracking cannot summarize function calls on its own, so it doesn't benefit from synthesized callables.
@@ -118,7 +125,12 @@ API::Node getExtraSuccessorFromNode(API::Node node, AccessPathToken token) {
  * Gets a Ruby-specific API graph successor of `node` reachable by resolving `token`.
  */
 bindingset[token]
-API::Node getExtraSuccessorFromInvoke(InvokeNode node, AccessPathToken token) { none() }
+API::Node getExtraSuccessorFromInvoke(InvokeNode node, AccessPathToken token) {
+  token.getName() = "Argument" and
+  result =
+    node.getASuccessor(API::Label::getLabelFromParameterPosition(FlowSummaryImplSpecific::parseArgBody(token
+              .getAnArgument())))
+}
 
 /**
  * Holds if `invoke` matches the Ruby-specific call site filter in `token`.
@@ -165,4 +177,7 @@ bindingset[name, argument]
 predicate isExtraValidTokenArgumentInIdentifyingAccessPath(string name, string argument) {
   name = ["Member", "Method"] and
   exists(argument)
+  or
+  name = ["Argument", "Parameter"] and
+  argument = ["self", "block"]
 }

ruby/ql/test/library-tests/dataflow/summaries/Summaries.ql

@@ -42,10 +42,10 @@ private class SummarizedCallableApplyBlock extends SummarizedCallable {
 
   override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
     input = "Argument[0]" and
-    output = "BlockArgument.Parameter[0]" and
+    output = "Argument[block].Parameter[0]" and
     preservesValue = true
     or
-    input = "BlockArgument.ReturnValue" and
+    input = "Argument[block].ReturnValue" and
     output = "ReturnValue" and
     preservesValue = true
   }
@@ -75,9 +75,9 @@ private class StepsFromModel extends ModelInput::SummaryModelCsv {
         ";;Member[Foo].Method[secondArg];Argument[1];ReturnValue;taint",
         ";;Member[Foo].Method[onlyWithoutBlock].WithoutBlock;Argument[0];ReturnValue;taint",
         ";;Member[Foo].Method[onlyWithBlock].WithBlock;Argument[0];ReturnValue;taint",
-        ";;Member[Foo].Method[blockArg].BlockArgument.Parameter[0].Method[preserveTaint];Argument[0];ReturnValue;taint",
+        ";;Member[Foo].Method[blockArg].Argument[block].Parameter[0].Method[preserveTaint];Argument[0];ReturnValue;taint",
         ";any;Method[matchedByName];Argument[0];ReturnValue;taint",
-        ";any;Method[matchedByNameRcv];Receiver;ReturnValue;taint",
+        ";any;Method[matchedByNameRcv];Argument[self];ReturnValue;taint",
       ]
   }
 }