Ruby: generalise summaries for ActiveSupport Hash extensions

Author: nickrolfe

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

@@ -592,15 +592,12 @@ private module ParamsSummaries {
   /**
    * `#merge`
    * Returns a new ActionController::Parameters with all keys from other_hash merged into current hash.
-   * `#reverse_merge`
-   * `#with_defaults`
-   * Returns a new ActionController::Parameters with all keys from current hash merged into other_hash.
    */
   private class MergeSummary extends SummarizedCallable {
     MergeSummary() { this = "ActionController::Parameters#merge" }
 
     override MethodCall getACall() {
-      result.getMethodName() = ["merge", "reverse_merge", "with_defaults"] and
+      result.getMethodName() = "merge" and
       exists(ParamsInstance i |
         i.asExpr().getExpr() = [result.getReceiver(), result.getArgument(0)]
       )
@@ -616,15 +613,12 @@ private module ParamsSummaries {
   /**
    * `#merge!`
    * Returns current ActionController::Parameters instance with current hash merged into other_hash.
-   * `#reverse_merge!`
-   * `#with_defaults!`
-   * Returns a new ActionController::Parameters with all keys from current hash merged into other_hash.
    */
   private class MergeBangSummary extends SummarizedCallable {
     MergeBangSummary() { this = "ActionController::Parameters#merge!" }
 
     override MethodCall getACall() {
-      result.getMethodName() = ["merge!", "reverse_merge!", "with_defaults!"] and
+      result.getMethodName() = "merge!" and
       exists(ParamsInstance i |
         i.asExpr().getExpr() = [result.getReceiver(), result.getArgument(0)]
       )

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

@@ -120,6 +120,32 @@ module ActiveSupport {
         }
       }
 
+      /**
+       * Flow summary for `reverse_merge`, and its alias `with_defaults`.
+       */
+      private class ReverseMergeSummary extends SimpleSummarizedCallable {
+        ReverseMergeSummary() { this = ["reverse_merge", "with_defaults"] }
+
+        override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
+          input = ["Argument[self]", "Argument[0]"] and
+          output = "ReturnValue" and
+          preservesValue = false
+        }
+      }
+
+      /**
+       * Flow summary for `reverse_merge!`, and its aliases `with_defaults!` and `reverse_update`.
+       */
+      private class ReverseMergeBangSummary extends SimpleSummarizedCallable {
+        ReverseMergeBangSummary() { this = ["reverse_merge!", "with_defaults!", "reverse_update"] }
+
+        override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
+          input = ["Argument[self]", "Argument[0]"] and
+          output = ["ReturnValue", "Argument[self]"] and
+          preservesValue = false
+        }
+      }
+
       private class TransformSummary extends SimpleSummarizedCallable {
         TransformSummary() {
           this =

ruby/ql/test/library-tests/frameworks/action_controller/params-flow.expected

@@ -44,6 +44,10 @@ edges
 | params_flow.rb:145:32:145:37 | call to params :  | params_flow.rb:145:10:145:38 | call to with_defaults! |
 | params_flow.rb:148:5:148:5 | [post] p :  | params_flow.rb:149:10:149:10 | p |
 | params_flow.rb:148:22:148:27 | call to params :  | params_flow.rb:148:5:148:5 | [post] p :  |
+| params_flow.rb:153:10:153:15 | call to params :  | params_flow.rb:153:10:153:44 | call to reverse_update |
+| params_flow.rb:154:32:154:37 | call to params :  | params_flow.rb:154:10:154:38 | call to reverse_update |
+| params_flow.rb:157:5:157:5 | [post] p :  | params_flow.rb:158:10:158:10 | p |
+| params_flow.rb:157:22:157:27 | call to params :  | params_flow.rb:157:5:157:5 | [post] p :  |
 nodes
 | params_flow.rb:3:10:3:15 | call to params :  | semmle.label | call to params :  |
 | params_flow.rb:3:10:3:19 | ...[...] | semmle.label | ...[...] |
@@ -130,6 +134,13 @@ nodes
 | params_flow.rb:148:5:148:5 | [post] p :  | semmle.label | [post] p :  |
 | params_flow.rb:148:22:148:27 | call to params :  | semmle.label | call to params :  |
 | params_flow.rb:149:10:149:10 | p | semmle.label | p |
+| params_flow.rb:153:10:153:15 | call to params :  | semmle.label | call to params :  |
+| params_flow.rb:153:10:153:44 | call to reverse_update | semmle.label | call to reverse_update |
+| params_flow.rb:154:10:154:38 | call to reverse_update | semmle.label | call to reverse_update |
+| params_flow.rb:154:32:154:37 | call to params :  | semmle.label | call to params :  |
+| params_flow.rb:157:5:157:5 | [post] p :  | semmle.label | [post] p :  |
+| params_flow.rb:157:22:157:27 | call to params :  | semmle.label | call to params :  |
+| params_flow.rb:158:10:158:10 | p | semmle.label | p |
 subpaths
 #select
 | params_flow.rb:3:10:3:19 | ...[...] | params_flow.rb:3:10:3:15 | call to params :  | params_flow.rb:3:10:3:19 | ...[...] | $@ | params_flow.rb:3:10:3:15 | call to params :  | call to params :  |
@@ -173,3 +184,6 @@ subpaths
 | params_flow.rb:144:10:144:44 | call to with_defaults! | params_flow.rb:144:10:144:15 | call to params :  | params_flow.rb:144:10:144:44 | call to with_defaults! | $@ | params_flow.rb:144:10:144:15 | call to params :  | call to params :  |
 | params_flow.rb:145:10:145:38 | call to with_defaults! | params_flow.rb:145:32:145:37 | call to params :  | params_flow.rb:145:10:145:38 | call to with_defaults! | $@ | params_flow.rb:145:32:145:37 | call to params :  | call to params :  |
 | params_flow.rb:149:10:149:10 | p | params_flow.rb:148:22:148:27 | call to params :  | params_flow.rb:149:10:149:10 | p | $@ | params_flow.rb:148:22:148:27 | call to params :  | call to params :  |
+| params_flow.rb:153:10:153:44 | call to reverse_update | params_flow.rb:153:10:153:15 | call to params :  | params_flow.rb:153:10:153:44 | call to reverse_update | $@ | params_flow.rb:153:10:153:15 | call to params :  | call to params :  |
+| params_flow.rb:154:10:154:38 | call to reverse_update | params_flow.rb:154:32:154:37 | call to params :  | params_flow.rb:154:10:154:38 | call to reverse_update | $@ | params_flow.rb:154:32:154:37 | call to params :  | call to params :  |
+| params_flow.rb:158:10:158:10 | p | params_flow.rb:157:22:157:27 | call to params :  | params_flow.rb:158:10:158:10 | p | $@ | params_flow.rb:157:22:157:27 | call to params :  | call to params :  |

ruby/ql/test/library-tests/frameworks/action_controller/params_flow.rb

@@ -148,4 +148,13 @@ def m32
     p.with_defaults!(params)
     sink p # $hasTaintFlow
   end
+
+  def m33
+    sink params.reverse_update({a: 1, b: 2}) # $hasTaintFlow
+    sink {a: 1}.reverse_update(params) # $hasTaintFlow
+
+    p = {a: 1}
+    p.reverse_update(params)
+    sink p # $hasTaintFlow
+  end
 end