Merge branch 'change/adjust-extracted-files-diagnostics' of https://github.com/sidshank/codeql into change/adjust-extracted-files-diagnostics

ruby/ql/lib/CHANGELOG.md

@@ -1,3 +1,11 @@
+## 0.8.6
+
+### Minor Analysis Improvements
+
+* Parsing of division operators (`/`) at the end of a line has been improved. Before they were wrongly interpreted as the start of a regular expression literal (`/.../`) leading to syntax errors.
+* Parsing of `case` statements that are formatted with the value expression on a different line than the `case` keyword  has been improved and should no longer lead to syntax errors.
+* Ruby now makes use of the shared type tracking library, exposed as `codeql.ruby.typetracking.TypeTracking`. The existing type tracking library, `codeql.ruby.typetracking.TypeTracker`, has consequently been deprecated.
+
 ## 0.8.5
 
 No user-facing changes.

ruby/ql/lib/change-notes/2023-11-21-new-type-tracking-lib.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* Ruby now makes use of the shared type tracking library, exposed as `codeql.ruby.typetracking.TypeTracking`. The existing type tracking library, `codeql.ruby.typetracking.TypeTracker`, has consequently been deprecated.

ruby/ql/lib/change-notes/released/0.8.6.md

@@ -0,0 +1,7 @@
+## 0.8.6
+
+### Minor Analysis Improvements
+
+* Parsing of division operators (`/`) at the end of a line has been improved. Before they were wrongly interpreted as the start of a regular expression literal (`/.../`) leading to syntax errors.
+* Parsing of `case` statements that are formatted with the value expression on a different line than the `case` keyword  has been improved and should no longer lead to syntax errors.
+* Ruby now makes use of the shared type tracking library, exposed as `codeql.ruby.typetracking.TypeTracking`. The existing type tracking library, `codeql.ruby.typetracking.TypeTracker`, has consequently been deprecated.

ruby/ql/lib/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.8.5
+lastReleaseVersion: 0.8.6

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

@@ -208,7 +208,9 @@ private predicate moduleFlowsToMethodCallReceiver(RelevantCall call, Module m, s
   flowsToMethodCallReceiver(call, trackModuleAccess(m), method)
 }
 
-private Block blockCall(RelevantCall call) { lambdaSourceCall(call, _, trackBlock(result)) }
+private Block blockCall(RelevantCall call) {
+  lambdaSourceCall(call, _, trackBlock(result).(DataFlow::LocalSourceNode).getALocalUse())
+}
 
 pragma[nomagic]
 private predicate superCall(RelevantCall call, Module cls, string method) {
@@ -1088,8 +1090,8 @@ private CfgScope getTargetSingleton(RelevantCall call, string method) {
 }
 
 /**
- * Holds if `ctx` targets `encl`, which is the enclosing callable of `call`, the receiver
- * of `call` is a parameter access, where the corresponding argument of `ctx` is `arg`.
+ * Holds if `ctx` targets the enclosing callable of `call`, the receiver of `call` is a
+ * parameter access, where the corresponding argument of `ctx` is `arg`.
  *
  * `name` is the name of the method being called by `call`, `source` is a
  * `LocalSourceNode` that flows to `arg`, and `paramDef` is the SSA definition for the
@@ -1098,11 +1100,11 @@ private CfgScope getTargetSingleton(RelevantCall call, string method) {
 pragma[nomagic]
 private predicate argMustFlowToReceiver(
   RelevantCall ctx, DataFlow::LocalSourceNode source, DataFlow::Node arg, RelevantCall call,
-  Callable encl, string name
+  string name
 ) {
   exists(
     ParameterNodeImpl p, SsaDefinitionExtNode paramDef, ParameterPosition ppos,
-    ArgumentPosition apos
+    ArgumentPosition apos, Callable encl
   |
     // the receiver of `call` references `p`
     exists(DataFlow::Node receiver |
@@ -1133,7 +1135,7 @@ private predicate argMustFlowToReceiver(
 }
 
 /**
- * Holds if `ctx` targets `encl`, which is the enclosing callable of `new`, and
+ * Holds if `ctx` targets the enclosing callable of `new`, and
  * the receiver of `new` is a parameter access, where the corresponding argument
  * `arg` of `ctx` has type `tp`.
  *
@@ -1141,10 +1143,10 @@ private predicate argMustFlowToReceiver(
  */
 pragma[nomagic]
 private predicate mayBenefitFromCallContextInitialize(
-  RelevantCall ctx, RelevantCall new, DataFlow::Node arg, Callable encl, Module tp, string name
+  RelevantCall ctx, RelevantCall new, DataFlow::Node arg, Module tp, string name
 ) {
   exists(DataFlow::LocalSourceNode source |
-    argMustFlowToReceiver(ctx, pragma[only_bind_into](source), arg, new, encl, "new") and
+    argMustFlowToReceiver(ctx, pragma[only_bind_into](source), arg, new, "new") and
     source = trackModuleAccess(tp) and
     name = "initialize" and
     exists(lookupMethod(tp, name))
@@ -1152,7 +1154,7 @@ private predicate mayBenefitFromCallContextInitialize(
 }
 
 /**
- * Holds if `ctx` targets `encl`, which is the enclosing callable of `call`, and
+ * Holds if `ctx` targets the enclosing callable of `call`, and
  * the receiver of `call` is a parameter access, where the corresponding argument
  * `arg` of `ctx` has type `tp`.
  *
@@ -1161,11 +1163,10 @@ private predicate mayBenefitFromCallContextInitialize(
  */
 pragma[nomagic]
 private predicate mayBenefitFromCallContextInstance(
-  RelevantCall ctx, RelevantCall call, DataFlow::Node arg, Callable encl, Module tp, boolean exact,
-  string name
+  RelevantCall ctx, RelevantCall call, DataFlow::Node arg, Module tp, boolean exact, string name
 ) {
   exists(DataFlow::LocalSourceNode source |
-    argMustFlowToReceiver(ctx, pragma[only_bind_into](source), arg, call, encl,
+    argMustFlowToReceiver(ctx, pragma[only_bind_into](source), arg, call,
       pragma[only_bind_into](name)) and
     source = trackInstance(tp, exact) and
     exists(lookupMethod(tp, pragma[only_bind_into](name)))
@@ -1173,7 +1174,7 @@ private predicate mayBenefitFromCallContextInstance(
 }
 
 /**
- * Holds if `ctx` targets `encl`, which is the enclosing callable of `call`, and
+ * Holds if `ctx` targets  the enclosing callable of `call`, and
  * the receiver of `call` is a parameter access, where the corresponding argument
  * `arg` of `ctx` is a module access targeting a module of type `tp`.
  *
@@ -1182,12 +1183,11 @@ private predicate mayBenefitFromCallContextInstance(
  */
 pragma[nomagic]
 private predicate mayBenefitFromCallContextSingleton(
-  RelevantCall ctx, RelevantCall call, DataFlow::Node arg, Callable encl, Module tp, boolean exact,
-  string name
+  RelevantCall ctx, RelevantCall call, DataFlow::Node arg, Module tp, boolean exact, string name
 ) {
   exists(DataFlow::LocalSourceNode source |
     argMustFlowToReceiver(ctx, pragma[only_bind_into](source), pragma[only_bind_into](arg), call,
-      encl, pragma[only_bind_into](name)) and
+      pragma[only_bind_into](name)) and
     exists(lookupSingletonMethod(tp, pragma[only_bind_into](name), exact))
   |
     source = trackModuleAccess(tp) and
@@ -1208,16 +1208,14 @@ private predicate mayBenefitFromCallContextSingleton(
 
 /**
  * Holds if the set of viable implementations that can be called by `call`
- * might be improved by knowing the call context. This is the case if the
- * receiver accesses a parameter of the enclosing callable `c` (including
- * the implicit `self` parameter).
+ * might be improved by knowing the call context.
  */
-predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable c) {
-  mayBenefitFromCallContextInitialize(_, call.asCall(), _, c.asCallable(), _, _)
+predicate mayBenefitFromCallContext(DataFlowCall call) {
+  mayBenefitFromCallContextInitialize(_, call.asCall(), _, _, _)
   or
-  mayBenefitFromCallContextInstance(_, call.asCall(), _, c.asCallable(), _, _, _)
+  mayBenefitFromCallContextInstance(_, call.asCall(), _, _, _, _)
   or
-  mayBenefitFromCallContextSingleton(_, call.asCall(), _, c.asCallable(), _, _, _)
+  mayBenefitFromCallContextSingleton(_, call.asCall(), _, _, _, _)
 }
 
 /**
@@ -1226,25 +1224,25 @@ predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable c) {
  */
 pragma[nomagic]
 DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
-  mayBenefitFromCallContext(call, _) and
+  mayBenefitFromCallContext(call) and
   (
     // `ctx` can provide a potentially better type bound
     exists(RelevantCall call0, Callable res |
       call0 = call.asCall() and
       res = result.asCallable() and
       exists(Module m, string name |
-        mayBenefitFromCallContextInitialize(ctx.asCall(), pragma[only_bind_into](call0), _, _,
+        mayBenefitFromCallContextInitialize(ctx.asCall(), pragma[only_bind_into](call0), _,
           pragma[only_bind_into](m), pragma[only_bind_into](name)) and
         res = getInitializeTarget(call0) and
         res = lookupMethod(m, name)
         or
         exists(boolean exact |
-          mayBenefitFromCallContextInstance(ctx.asCall(), pragma[only_bind_into](call0), _, _,
+          mayBenefitFromCallContextInstance(ctx.asCall(), pragma[only_bind_into](call0), _,
             pragma[only_bind_into](m), pragma[only_bind_into](exact), pragma[only_bind_into](name)) and
           res = getTargetInstance(call0, name) and
           res = lookupMethod(m, name, exact)
           or
-          mayBenefitFromCallContextSingleton(ctx.asCall(), pragma[only_bind_into](call0), _, _,
+          mayBenefitFromCallContextSingleton(ctx.asCall(), pragma[only_bind_into](call0), _,
             pragma[only_bind_into](m), pragma[only_bind_into](exact), pragma[only_bind_into](name)) and
           res = getTargetSingleton(call0, name) and
           res = lookupSingletonMethod(m, name, exact)
@@ -1257,15 +1255,15 @@ DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
     exists(RelevantCall call0, RelevantCall ctx0, DataFlow::Node arg, string name |
       call0 = call.asCall() and
       ctx0 = ctx.asCall() and
-      argMustFlowToReceiver(ctx0, _, arg, call0, _, name) and
-      not mayBenefitFromCallContextInitialize(ctx0, call0, arg, _, _, _) and
-      not mayBenefitFromCallContextInstance(ctx0, call0, arg, _, _, _, name) and
-      not mayBenefitFromCallContextSingleton(ctx0, call0, arg, _, _, _, name) and
+      argMustFlowToReceiver(ctx0, _, arg, call0, name) and
+      not mayBenefitFromCallContextInitialize(ctx0, call0, arg, _, _) and
+      not mayBenefitFromCallContextInstance(ctx0, call0, arg, _, _, name) and
+      not mayBenefitFromCallContextSingleton(ctx0, call0, arg, _, _, name) and
       result.asCallable() = viableSourceCallable(call0)
     )
     or
     // library calls should always be able to resolve
-    argMustFlowToReceiver(ctx.asCall(), _, _, call.asCall(), _, _) and
+    argMustFlowToReceiver(ctx.asCall(), _, _, call.asCall(), _) and
     result = viableLibraryCallable(call)
   )
 }

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

@@ -27,4 +27,8 @@ module RubyDataFlow implements InputSig {
   predicate neverSkipInPathGraph = Private::neverSkipInPathGraph/1;
 
   Node exprNode(DataFlowExpr e) { result = Public::exprNode(e) }
+
+  predicate mayBenefitFromCallContext = Private::mayBenefitFromCallContext/1;
+
+  predicate viableImplInCallContext = Private::viableImplInCallContext/2;
 }

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

@@ -230,6 +230,8 @@ module LocalFlow {
         or
         p.(KeywordParameter).getDefaultValue() = nodeFrom.asExpr().getExpr()
       )
+    or
+    nodeTo.(BlockArgumentNode).getParameterNode(true) = nodeFrom
   }
 }
 
@@ -497,6 +499,9 @@ private module Cached {
     TSelfParameterNode(MethodBase m) or
     TLambdaSelfReferenceNode(Callable c) { lambdaCreationExpr(_, _, c) } or
     TBlockParameterNode(MethodBase m) or
+    TBlockArgumentNode(CfgNodes::ExprNodes::CallCfgNode yield) {
+      yield = any(BlockParameterNode b).getAYieldCall()
+    } or
     TSynthHashSplatParameterNode(DataFlowCallable c) {
       isParameterNode(_, c, any(ParameterPosition p | p.isKeyword(_)))
     } or
@@ -645,6 +650,8 @@ private module Cached {
     isStoreTargetNode(n)
     or
     TypeTrackingInput::loadStep(_, n, _)
+    or
+    n instanceof BlockArgumentNode
   }
 
   cached
@@ -770,6 +777,8 @@ predicate nodeIsHidden(Node n) {
   n instanceof LambdaSelfReferenceNode
   or
   n instanceof CaptureNode
+  or
+  n instanceof BlockArgumentNode
 }
 
 /** An SSA definition, viewed as a node in a data flow graph. */
@@ -1277,18 +1286,36 @@ module ArgumentNodes {
     }
   }
 
-  class BlockParameterArgumentNode extends BlockParameterNode, ArgumentNode {
-    BlockParameterArgumentNode() { exists(this.getAYieldCall()) }
+  class BlockArgumentNode extends NodeImpl, ArgumentNode, TBlockArgumentNode {
+    CfgNodes::ExprNodes::CallCfgNode yield;
+
+    BlockArgumentNode() { this = TBlockArgumentNode(yield) }
+
+    CfgNodes::ExprNodes::CallCfgNode getYieldCall() { result = yield }
+
+    pragma[nomagic]
+    BlockParameterNode getParameterNode(boolean inSameScope) {
+      result.getAYieldCall() = yield and
+      if nodeGetEnclosingCallable(this) = nodeGetEnclosingCallable(result)
+      then inSameScope = true
+      else inSameScope = false
+    }
 
     // needed for variable capture flow
     override predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition pos) {
-      call = this.getAYieldCall() and
+      call = yield and
       pos.isLambdaSelf()
     }
 
     override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
       this.sourceArgumentOf(call.asCall(), pos)
     }
+
+    override CfgScope getCfgScope() { result = yield.getScope() }
+
+    override Location getLocationImpl() { result = yield.getLocation() }
+
+    override string toStringImpl() { result = "yield block argument" }
   }
 
   private class SummaryArgumentNode extends FlowSummaryNode, ArgumentNode {
@@ -1699,6 +1726,8 @@ predicate jumpStep(Node pred, Node succ) {
     succ.(FlowSummaryNode).getSummaryNode())
   or
   any(AdditionalJumpStep s).step(pred, succ)
+  or
+  succ.(BlockArgumentNode).getParameterNode(false) = pred
 }
 
 private ContentSet getArrayContent(int n) {
@@ -2037,7 +2066,7 @@ private predicate lambdaCallExpr(
  */
 predicate lambdaSourceCall(CfgNodes::ExprNodes::CallCfgNode call, LambdaCallKind kind, Node receiver) {
   kind = TYieldCallKind() and
-  call = receiver.(BlockParameterNode).getAYieldCall()
+  call = receiver.(BlockArgumentNode).getYieldCall()
   or
   kind = TLambdaCallKind() and
   lambdaCallExpr(call, receiver.asExpr())

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

@@ -512,7 +512,7 @@ private module ParamsSummaries {
         "dig", "each", "each_key", "each_pair", "each_value", "except", "keep_if", "merge",
         "merge!", "permit", "reject", "reject!", "require", "reverse_merge", "reverse_merge!",
         "select", "select!", "slice", "slice!", "transform_keys", "transform_keys!",
-        "transform_values", "transform_values!", "with_defaults", "with_defaults!"
+        "transform_values", "transform_values!", "with_defaults", "with_defaults!", "[]"
       ]
   }
 

ruby/ql/lib/codeql/ruby/frameworks/core/Kernel.qll

@@ -43,7 +43,9 @@ module Kernel {
    * ```
    */
   private predicate isPublicKernelMethod(string method) {
-    method in ["class", "clone", "frozen?", "tap", "then", "yield_self", "send"]
+    method in [
+        "class", "clone", "frozen?", "tap", "then", "yield_self", "send", "public_send", "__send__"
+      ]
   }
 
   /**
@@ -167,7 +169,7 @@ module Kernel {
    * ```
    */
   class SendCallCodeExecution extends CodeExecution::Range, KernelMethodCall {
-    SendCallCodeExecution() { this.getMethodName() = "send" }
+    SendCallCodeExecution() { this.getMethodName() = ["send", "public_send", "__send__"] }
 
     override DataFlow::Node getCode() { result = this.getArgument(0) }
 

ruby/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/ruby-all
-version: 0.8.6-dev
+version: 0.8.7-dev
 groups: ruby
 extractor: ruby
 dbscheme: ruby.dbscheme

ruby/ql/lib/upgrades/30e1075bbdc9ce935dbe28dc7175489fe8e69a4c/upgrade.properties

@@ -1,2 +1,3 @@
 description: Removed unused `numlines` relation
 compatibility: full
+numlines.rel: delete

ruby/ql/lib/upgrades/40be81bc2086eb0368f33c770e0a84817bb340c3/upgrade.properties

@@ -163,3 +163,166 @@ ruby_yield_child.rel: reorder yield_child.rel ( int ruby_yield, int child ) ruby
 ruby_yield_def.rel: reorder yield_def.rel ( int id, int loc ) id loc 
 ruby_tokeninfo.rel: reorder tokeninfo.rel ( int id, int kind, int file, int idx, string value, int loc ) id kind file idx value loc 
 ruby_ast_node_parent.rel: reorder ast_node_parent.rel ( int child, int parent, int parent_index) child parent parent_index
+alias_def.rel: delete
+argument_list_child.rel: delete
+argument_list_def.rel: delete
+array_child.rel: delete
+array_def.rel: delete
+assignment_def.rel: delete
+ast_node_parent.rel: delete
+bare_string_child.rel: delete
+bare_string_def.rel: delete
+bare_symbol_child.rel: delete
+bare_symbol_def.rel: delete
+begin_block_child.rel: delete
+begin_block_def.rel: delete
+begin_child.rel: delete
+begin_def.rel: delete
+binary_def.rel: delete
+block_argument_def.rel: delete
+block_child.rel: delete
+block_def.rel: delete
+block_parameter_def.rel: delete
+block_parameters.rel: delete
+block_parameters_child.rel: delete
+block_parameters_def.rel: delete
+break_child.rel: delete
+break_def.rel: delete
+call_arguments.rel: delete
+call_block.rel: delete
+call_def.rel: delete
+call_receiver.rel: delete
+case_child.rel: delete
+case_def.rel: delete
+case_value.rel: delete
+chained_string_child.rel: delete
+chained_string_def.rel: delete
+class_child.rel: delete
+class_def.rel: delete
+class_superclass.rel: delete
+conditional_def.rel: delete
+delimited_symbol_child.rel: delete
+delimited_symbol_def.rel: delete
+destructured_left_assignment_child.rel: delete
+destructured_left_assignment_def.rel: delete
+destructured_parameter_child.rel: delete
+destructured_parameter_def.rel: delete
+do_block_child.rel: delete
+do_block_def.rel: delete
+do_block_parameters.rel: delete
+do_child.rel: delete
+do_def.rel: delete
+element_reference_child.rel: delete
+element_reference_def.rel: delete
+else_child.rel: delete
+else_def.rel: delete
+elsif_alternative.rel: delete
+elsif_consequence.rel: delete
+elsif_def.rel: delete
+end_block_child.rel: delete
+end_block_def.rel: delete
+ensure_child.rel: delete
+ensure_def.rel: delete
+exception_variable_def.rel: delete
+exceptions_child.rel: delete
+exceptions_def.rel: delete
+for_def.rel: delete
+hash_child.rel: delete
+hash_def.rel: delete
+hash_splat_argument_def.rel: delete
+hash_splat_parameter_def.rel: delete
+hash_splat_parameter_name.rel: delete
+heredoc_body_child.rel: delete
+heredoc_body_def.rel: delete
+if_alternative.rel: delete
+if_consequence.rel: delete
+if_def.rel: delete
+if_modifier_def.rel: delete
+in_def.rel: delete
+interpolation_child.rel: delete
+interpolation_def.rel: delete
+keyword_parameter_def.rel: delete
+keyword_parameter_value.rel: delete
+lambda_def.rel: delete
+lambda_parameters.rel: delete
+lambda_parameters_child.rel: delete
+lambda_parameters_def.rel: delete
+left_assignment_list_child.rel: delete
+left_assignment_list_def.rel: delete
+method_child.rel: delete
+method_def.rel: delete
+method_parameters.rel: delete
+method_parameters_child.rel: delete
+method_parameters_def.rel: delete
+module_child.rel: delete
+module_def.rel: delete
+next_child.rel: delete
+next_def.rel: delete
+operator_assignment_def.rel: delete
+optional_parameter_def.rel: delete
+pair_def.rel: delete
+parenthesized_statements_child.rel: delete
+parenthesized_statements_def.rel: delete
+pattern_def.rel: delete
+program_child.rel: delete
+program_def.rel: delete
+range_begin.rel: delete
+range_def.rel: delete
+range_end.rel: delete
+rational_def.rel: delete
+redo_child.rel: delete
+redo_def.rel: delete
+regex_child.rel: delete
+regex_def.rel: delete
+rescue_body.rel: delete
+rescue_def.rel: delete
+rescue_exceptions.rel: delete
+rescue_modifier_def.rel: delete
+rescue_variable.rel: delete
+rest_assignment_child.rel: delete
+rest_assignment_def.rel: delete
+retry_child.rel: delete
+retry_def.rel: delete
+return_child.rel: delete
+return_def.rel: delete
+right_assignment_list_child.rel: delete
+right_assignment_list_def.rel: delete
+scope_resolution_def.rel: delete
+scope_resolution_scope.rel: delete
+setter_def.rel: delete
+singleton_class_child.rel: delete
+singleton_class_def.rel: delete
+singleton_method_child.rel: delete
+singleton_method_def.rel: delete
+singleton_method_parameters.rel: delete
+splat_argument_def.rel: delete
+splat_parameter_def.rel: delete
+splat_parameter_name.rel: delete
+string_array_child.rel: delete
+string_array_def.rel: delete
+string_child.rel: delete
+string_def.rel: delete
+subshell_child.rel: delete
+subshell_def.rel: delete
+superclass_def.rel: delete
+symbol_array_child.rel: delete
+symbol_array_def.rel: delete
+then_child.rel: delete
+then_def.rel: delete
+tokeninfo.rel: delete
+unary_def.rel: delete
+undef_child.rel: delete
+undef_def.rel: delete
+unless_alternative.rel: delete
+unless_consequence.rel: delete
+unless_def.rel: delete
+unless_modifier_def.rel: delete
+until_def.rel: delete
+until_modifier_def.rel: delete
+when_body.rel: delete
+when_def.rel: delete
+when_pattern.rel: delete
+while_def.rel: delete
+while_modifier_def.rel: delete
+yield_child.rel: delete
+yield_def.rel: delete