Ruby: More filter flow steps

Add a jump step from the last self post-update node in a method to the self parameter of the
next method.

Author: hmac

ruby/ql/lib/codeql/ruby/frameworks/actioncontroller/Filters.qll

@@ -428,6 +428,32 @@ module Filters {
     )
   }
 
+  /**
+   * Holds if `n` is a post-update node for `self` in method `m`.
+   */
+  predicate selfPostUpdate(DataFlow::PostUpdateNode n, Method m) {
+    n.getPreUpdateNode().asExpr() instanceof SelfVariableAccessCfgNode and
+    m = n.getPreUpdateNode().asExpr().getExpr().getEnclosingCallable()
+  }
+
+  /**
+   * Holds if `n` is the syntactically last dataflow node in `m` that satisfies `selfPostUpdate`.
+   * In the example below, `n` is the post-update node for `bar`.
+   * ```rb
+   * def m
+   *   foo
+   *   bar
+   * end
+   * ```
+   */
+  predicate lastSelfPostUpdate(DataFlow::PostUpdateNode n, Method m) {
+    selfPostUpdate(n, m) and
+    not exists(DataFlow::PostUpdateNode n2 |
+      selfPostUpdate(n2, m) and
+      n.getPreUpdateNode().asExpr().getASuccessor+() = n2.getPreUpdateNode().asExpr()
+    )
+  }
+
   /**
    * Holds if `a` is the syntactically last access of the self variable `v` in method `m`.
    */
@@ -468,7 +494,8 @@ module Filters {
       exists(Method predMethod, Method succMethod |
         next(predMethod, succMethod) and
         (
-          // Flow from an update of self in `pred` to the self parameter of `succ`
+          // Flow from an update of self (due to an instance variable write) in
+          // `pred` to the self parameter of `succ`
           //
           // def a
           //   @x = 1 ---+
@@ -480,6 +507,18 @@ module Filters {
           lastVariableAccessPostUpdate(pred, predMethod) and
           selfParameter(succ, succMethod)
           or
+          // Flow from an update of self in `pred` to the self parameter of `succ`
+          //
+          // def a
+          //   @x = 1 ---+
+          // end         |
+          //             |
+          // def b  <----+
+          //  ...
+          //
+          lastSelfPostUpdate(pred, predMethod) and
+          selfParameter(succ, succMethod)
+          or
           // When there is no update of self in `pred`, flow from the last access to self to the self parameter of `succ`
           //
           // def a

ruby/ql/test/library-tests/frameworks/action_controller/Filters.expected

@@ -5,20 +5,38 @@ additionalFlowSteps
 | controllers/application_controller.rb:7:5:7:9 | [post] self | controllers/posts_controller.rb:12:3:15:5 | self in index |
 | controllers/application_controller.rb:7:5:7:9 | [post] self | controllers/posts_controller.rb:17:3:18:5 | self in show |
 | controllers/application_controller.rb:7:5:7:9 | [post] self | controllers/posts_controller.rb:20:3:21:5 | self in upvote |
+| controllers/application_controller.rb:7:23:7:29 | [post] self | controllers/comments_controller.rb:74:3:77:5 | self in ensure_user_can_edit_comments |
+| controllers/application_controller.rb:7:23:7:29 | [post] self | controllers/comments_controller.rb:79:3:81:5 | self in set_comment |
+| controllers/application_controller.rb:7:23:7:29 | [post] self | controllers/comments_controller.rb:99:3:100:5 | self in foo |
+| controllers/application_controller.rb:7:23:7:29 | [post] self | controllers/posts_controller.rb:12:3:15:5 | self in index |
+| controllers/application_controller.rb:7:23:7:29 | [post] self | controllers/posts_controller.rb:17:3:18:5 | self in show |
+| controllers/application_controller.rb:7:23:7:29 | [post] self | controllers/posts_controller.rb:20:3:21:5 | self in upvote |
+| controllers/application_controller.rb:11:53:11:59 | [post] self | controllers/application_controller.rb:6:3:8:5 | self in set_user |
+| controllers/application_controller.rb:11:53:11:59 | [post] self | controllers/photos_controller.rb:3:3:6:5 | self in show |
+| controllers/application_controller.rb:11:53:11:59 | [post] self | controllers/posts_controller.rb:25:3:27:5 | self in set_post |
 | controllers/application_controller.rb:11:53:11:59 | self | controllers/application_controller.rb:6:3:8:5 | self in set_user |
 | controllers/application_controller.rb:11:53:11:59 | self | controllers/photos_controller.rb:3:3:6:5 | self in show |
 | controllers/application_controller.rb:11:53:11:59 | self | controllers/posts_controller.rb:25:3:27:5 | self in set_post |
+| controllers/comments_controller.rb:50:5:50:12 | [post] self | controllers/comments_controller.rb:87:3:89:5 | self in check_feature_flags |
 | controllers/comments_controller.rb:50:5:50:12 | self | controllers/comments_controller.rb:87:3:89:5 | self in check_feature_flags |
 | controllers/comments_controller.rb:53:3:54:5 | self in create | controllers/comments_controller.rb:83:3:85:5 | self in log_comment_change |
+| controllers/comments_controller.rb:57:5:61:7 | [post] self | controllers/comments_controller.rb:87:3:89:5 | self in check_feature_flags |
 | controllers/comments_controller.rb:57:5:61:7 | self | controllers/comments_controller.rb:87:3:89:5 | self in check_feature_flags |
 | controllers/comments_controller.rb:58:33:58:48 | self | controllers/comments_controller.rb:87:3:89:5 | self in check_feature_flags |
 | controllers/comments_controller.rb:60:58:60:63 | self | controllers/comments_controller.rb:87:3:89:5 | self in check_feature_flags |
+| controllers/comments_controller.rb:65:15:65:20 | [post] self | controllers/comments_controller.rb:83:3:85:5 | self in log_comment_change |
 | controllers/comments_controller.rb:65:15:65:20 | self | controllers/comments_controller.rb:83:3:85:5 | self in log_comment_change |
+| controllers/comments_controller.rb:69:12:69:18 | [post] self | controllers/comments_controller.rb:83:3:85:5 | self in log_comment_change |
 | controllers/comments_controller.rb:69:12:69:18 | self | controllers/comments_controller.rb:83:3:85:5 | self in log_comment_change |
+| controllers/comments_controller.rb:76:5:76:68 | [post] self | controllers/comments_controller.rb:79:3:81:5 | self in set_comment |
+| controllers/comments_controller.rb:76:5:76:68 | [post] self | controllers/comments_controller.rb:99:3:100:5 | self in foo |
 | controllers/comments_controller.rb:76:5:76:68 | self | controllers/comments_controller.rb:79:3:81:5 | self in set_comment |
 | controllers/comments_controller.rb:76:5:76:68 | self | controllers/comments_controller.rb:99:3:100:5 | self in foo |
 | controllers/comments_controller.rb:80:5:80:12 | [post] self | controllers/comments_controller.rb:99:3:100:5 | self in foo |
+| controllers/comments_controller.rb:80:36:80:41 | [post] self | controllers/comments_controller.rb:99:3:100:5 | self in foo |
+| controllers/comments_controller.rb:84:61:84:68 | [post] self | controllers/comments_controller.rb:87:3:89:5 | self in check_feature_flags |
 | controllers/comments_controller.rb:84:61:84:68 | self | controllers/comments_controller.rb:87:3:89:5 | self in check_feature_flags |
+| controllers/comments_controller.rb:88:5:88:28 | [post] self | controllers/comments_controller.rb:95:3:97:5 | self in this_must_run_last |
 | controllers/comments_controller.rb:88:5:88:28 | self | controllers/comments_controller.rb:95:3:97:5 | self in this_must_run_last |
 | controllers/comments_controller.rb:91:3:93:5 | self in this_must_run_first | controllers/application_controller.rb:10:3:12:5 | self in log_request |
 | controllers/comments_controller.rb:99:3:100:5 | self in foo | controllers/comments_controller.rb:102:3:103:5 | self in bar |
@@ -30,14 +48,19 @@ additionalFlowSteps
 | controllers/photos_controller.rb:5:5:5:6 | [post] self | controllers/photos_controller.rb:8:3:9:5 | self in foo |
 | controllers/posts_controller.rb:20:3:21:5 | self in upvote | controllers/posts_controller.rb:29:3:31:5 | self in log_upvote |
 | controllers/posts_controller.rb:26:5:26:9 | [post] self | controllers/application_controller.rb:6:3:8:5 | self in set_user |
+| controllers/posts_controller.rb:26:23:26:28 | [post] self | controllers/application_controller.rb:6:3:8:5 | self in set_user |
 | filter_flow.rb:14:5:14:8 | [post] self | filter_flow.rb:17:3:18:5 | self in b |
+| filter_flow.rb:14:12:14:17 | [post] self | filter_flow.rb:17:3:18:5 | self in b |
 | filter_flow.rb:17:3:18:5 | self in b | filter_flow.rb:20:3:22:5 | self in c |
 | filter_flow.rb:30:5:30:8 | [post] self | filter_flow.rb:33:3:35:5 | self in b |
+| filter_flow.rb:30:12:30:17 | [post] self | filter_flow.rb:33:3:35:5 | self in b |
 | filter_flow.rb:34:5:34:8 | [post] self | filter_flow.rb:37:3:39:5 | self in c |
 | filter_flow.rb:48:5:48:8 | [post] self | filter_flow.rb:51:3:52:5 | self in b |
 | filter_flow.rb:51:3:52:5 | self in b | filter_flow.rb:54:3:56:5 | self in c |
+| filter_flow.rb:64:16:64:21 | [post] self | filter_flow.rb:67:3:68:5 | self in b |
 | filter_flow.rb:64:16:64:21 | self | filter_flow.rb:67:3:68:5 | self in b |
 | filter_flow.rb:67:3:68:5 | self in b | filter_flow.rb:70:3:72:5 | self in c |
+| filter_flow.rb:80:5:80:8 | [post] self | filter_flow.rb:83:3:84:5 | self in b |
 | filter_flow.rb:80:5:80:8 | self | filter_flow.rb:83:3:84:5 | self in b |
 | filter_flow.rb:83:3:84:5 | self in b | filter_flow.rb:86:3:88:5 | self in c |
 filterChain

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

@@ -1,6 +1,7 @@
 failures
 | filter_flow.rb:21:10:21:13 | @foo | Unexpected result: hasTaintFlow= |
 | filter_flow.rb:71:10:71:17 | call to bar | Unexpected result: hasTaintFlow= |
+| filter_flow.rb:87:10:87:13 | @foo | Unexpected result: hasTaintFlow= |
 edges
 | filter_flow.rb:14:5:14:8 | [post] self [@foo] :  | filter_flow.rb:17:3:18:5 | self in b [@foo] :  |
 | filter_flow.rb:14:12:14:17 | call to params :  | filter_flow.rb:14:12:14:23 | ...[...] :  |
@@ -17,6 +18,13 @@ edges
 | filter_flow.rb:70:3:72:5 | self in c [@foo, @bar] :  | filter_flow.rb:71:10:71:13 | self [@foo, @bar] :  |
 | filter_flow.rb:71:10:71:13 | @foo [@bar] :  | filter_flow.rb:71:10:71:17 | call to bar |
 | filter_flow.rb:71:10:71:13 | self [@foo, @bar] :  | filter_flow.rb:71:10:71:13 | @foo [@bar] :  |
+| filter_flow.rb:80:5:80:8 | [post] self [@foo] :  | filter_flow.rb:83:3:84:5 | self in b [@foo] :  |
+| filter_flow.rb:83:3:84:5 | self in b [@foo] :  | filter_flow.rb:86:3:88:5 | self in c [@foo] :  |
+| filter_flow.rb:86:3:88:5 | self in c [@foo] :  | filter_flow.rb:87:10:87:13 | self [@foo] :  |
+| filter_flow.rb:87:10:87:13 | self [@foo] :  | filter_flow.rb:87:10:87:13 | @foo |
+| filter_flow.rb:91:5:91:8 | [post] self [@foo] :  | filter_flow.rb:80:5:80:8 | [post] self [@foo] :  |
+| filter_flow.rb:91:12:91:17 | call to params :  | filter_flow.rb:91:12:91:23 | ...[...] :  |
+| filter_flow.rb:91:12:91:23 | ...[...] :  | filter_flow.rb:91:5:91:8 | [post] self [@foo] :  |
 | params_flow.rb:3:10:3:15 | call to params :  | params_flow.rb:3:10:3:19 | ...[...] |
 | params_flow.rb:7:10:7:15 | call to params :  | params_flow.rb:7:10:7:23 | call to as_json |
 | params_flow.rb:15:10:15:15 | call to params :  | params_flow.rb:15:10:15:33 | call to permit |
@@ -86,6 +94,14 @@ nodes
 | filter_flow.rb:71:10:71:13 | @foo [@bar] :  | semmle.label | @foo [@bar] :  |
 | filter_flow.rb:71:10:71:13 | self [@foo, @bar] :  | semmle.label | self [@foo, @bar] :  |
 | filter_flow.rb:71:10:71:17 | call to bar | semmle.label | call to bar |
+| filter_flow.rb:80:5:80:8 | [post] self [@foo] :  | semmle.label | [post] self [@foo] :  |
+| filter_flow.rb:83:3:84:5 | self in b [@foo] :  | semmle.label | self in b [@foo] :  |
+| filter_flow.rb:86:3:88:5 | self in c [@foo] :  | semmle.label | self in c [@foo] :  |
+| filter_flow.rb:87:10:87:13 | @foo | semmle.label | @foo |
+| filter_flow.rb:87:10:87:13 | self [@foo] :  | semmle.label | self [@foo] :  |
+| filter_flow.rb:91:5:91:8 | [post] self [@foo] :  | semmle.label | [post] self [@foo] :  |
+| filter_flow.rb:91:12:91:17 | call to params :  | semmle.label | call to params :  |
+| filter_flow.rb:91:12:91:23 | ...[...] :  | semmle.label | ...[...] :  |
 | params_flow.rb:3:10:3:15 | call to params :  | semmle.label | call to params :  |
 | params_flow.rb:3:10:3:19 | ...[...] | semmle.label | ...[...] |
 | params_flow.rb:7:10:7:15 | call to params :  | semmle.label | call to params :  |
@@ -188,6 +204,7 @@ subpaths
 #select
 | filter_flow.rb:21:10:21:13 | @foo | filter_flow.rb:14:12:14:17 | call to params :  | filter_flow.rb:21:10:21:13 | @foo | $@ | filter_flow.rb:14:12:14:17 | call to params :  | call to params :  |
 | filter_flow.rb:71:10:71:17 | call to bar | filter_flow.rb:64:16:64:21 | call to params :  | filter_flow.rb:71:10:71:17 | call to bar | $@ | filter_flow.rb:64:16:64:21 | call to params :  | call to params :  |
+| filter_flow.rb:87:10:87:13 | @foo | filter_flow.rb:91:12:91:17 | call to params :  | filter_flow.rb:87:10:87:13 | @foo | $@ | filter_flow.rb:91:12:91:17 | call to params :  | call to params :  |
 | 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 :  |
 | params_flow.rb:7:10:7:23 | call to as_json | params_flow.rb:7:10:7:15 | call to params :  | params_flow.rb:7:10:7:23 | call to as_json | $@ | params_flow.rb:7:10:7:15 | call to params :  | call to params :  |
 | params_flow.rb:15:10:15:33 | call to permit | params_flow.rb:15:10:15:15 | call to params :  | params_flow.rb:15:10:15:33 | call to permit | $@ | params_flow.rb:15:10:15:15 | call to params :  | call to params :  |