Merge pull request #340 from github/mergeback

Merge rc/3.3 into main

.github/workflows/build.yml

@@ -2,9 +2,13 @@ name: Build / Release
 
 on:
   push:
-    branches: [main]
+    branches:
+      - main
+      - 'rc/*'
   pull_request:
-    branches: [main]
+    branches:
+      - main
+      - 'rc/*'
   workflow_dispatch:
     inputs:
       tag:

.github/workflows/dataset_measure.yml

@@ -2,11 +2,15 @@ name: Collect database stats
 
 on:
   push:
-    branches: [main]
+    branches:
+      - main
+      - 'rc/*'
     paths:
       - ql/lib/ruby.dbscheme
   pull_request:
-    branches: [main]
+    branches:
+      - main
+      - 'rc/*'
     paths:
       - ql/lib/ruby.dbscheme
   workflow_dispatch:

.github/workflows/qhelp.yml

@@ -2,7 +2,9 @@ name: Query help preview
 
 on:
   pull_request:
-    branches: [main]
+    branches:
+      - main
+      - 'rc/*'
     paths:
       - "**/*.qhelp"
 

.github/workflows/qltest.yml

@@ -2,9 +2,13 @@ name: Run QL Tests
 
 on:
   push:
-    branches: [main]
+    branches:
+      - main
+      - 'rc/*'
   pull_request:
-    branches: [main]
+    branches:
+      - main
+      - 'rc/*'
 
 env:
   CARGO_TERM_COLOR: always

.github/workflows/sync_files.yml

@@ -2,9 +2,13 @@ name: Check synchronized files
 
 on:
   push:
-    branches: [main]
+    branches:
+      - main
+      - 'rc/*'
   pull_request:
-    branches: [main]
+    branches:
+      - main
+      - 'rc/*'
 
 jobs:
   sync:

Cargo.lock

@@ -608,8 +608,7 @@ dependencies = [
 [[package]]
 name = "tree-sitter-ruby"
 version = "0.19.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "38b8d9d12b864a5f587052046be0bb8c7ae005df605b3150224472c41705268d"
+source = "git+https://github.com/tree-sitter/tree-sitter-ruby.git?rev=bb6a42e42b048627a74a127d3e0184c1eef01de9#bb6a42e42b048627a74a127d3e0184c1eef01de9"
 dependencies = [
  "cc",
  "tree-sitter",

extractor/Cargo.toml

@@ -11,7 +11,7 @@ flate2 = "1.0"
 node-types = { path = "../node-types" }
 tree-sitter = "0.19"
 tree-sitter-embedded-template = "0.19"
-tree-sitter-ruby = "0.19"
+tree-sitter-ruby = { git = "https://github.com/tree-sitter/tree-sitter-ruby.git", rev = "bb6a42e42b048627a74a127d3e0184c1eef01de9" }
 clap = "2.33"
 tracing = "0.1"
 tracing-subscriber = { version = "0.2", features = ["env-filter"] }

generator/Cargo.toml

@@ -12,4 +12,4 @@ node-types = { path = "../node-types" }
 tracing = "0.1"
 tracing-subscriber = { version = "0.2", features = ["env-filter"] }
 tree-sitter-embedded-template = "0.19"
-tree-sitter-ruby = "0.19"
+tree-sitter-ruby = { git = "https://github.com/tree-sitter/tree-sitter-ruby.git", rev = "bb6a42e42b048627a74a127d3e0184c1eef01de9" }

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

@@ -197,3 +197,19 @@ class BlockArgument extends Expr, TBlockArgument {
     pred = "getValue" and result = this.getValue()
   }
 }
+
+/**
+ * A `...` expression that contains forwarded arguments.
+ * ```rb
+ * foo(...)
+ * ```
+ */
+class ForwardedArguments extends Expr, TForwardArgument {
+  private Ruby::ForwardArgument g;
+
+  ForwardedArguments() { this = TForwardArgument(g) }
+
+  final override string getAPrimaryQlClass() { result = "ForwardedArguments" }
+
+  final override string toString() { result = "..." }
+}

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

@@ -233,3 +233,16 @@ class SplatParameter extends NamedParameter, TSplatParameter {
 
   final override string getName() { result = g.getName().getValue() }
 }
+
+/**
+ * A special `...` parameter that forwards positional/keyword/block arguments:
+ * ```rb
+ * def foo(...)
+ * end
+ * ```
+ */
+class ForwardParameter extends Parameter, TForwardParameter {
+  final override string getAPrimaryQlClass() { result = "ForwardParameter" }
+
+  final override string toString() { result = "..." }
+}

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

@@ -132,6 +132,8 @@ private module Cached {
     TFloatLiteral(Ruby::Float g) { not any(Ruby::Rational r).getChild() = g } or
     TForExpr(Ruby::For g) or
     TForIn(Ruby::In g) or // TODO REMOVE
+    TForwardParameter(Ruby::ForwardParameter g) or
+    TForwardArgument(Ruby::ForwardArgument g) or
     TGEExpr(Ruby::Binary g) { g instanceof @ruby_binary_rangleequal } or
     TGTExpr(Ruby::Binary g) { g instanceof @ruby_binary_rangle } or
     TGlobalVariableAccessReal(Ruby::GlobalVariable g, AST::GlobalVariable v) {
@@ -340,6 +342,8 @@ private module Cached {
     n = TFloatLiteral(result) or
     n = TForExpr(result) or
     n = TForIn(result) or // TODO REMOVE
+    n = TForwardArgument(result) or
+    n = TForwardParameter(result) or
     n = TGEExpr(result) or
     n = TGTExpr(result) or
     n = TGlobalVariableAccessReal(result, _) or
@@ -550,7 +554,8 @@ class TSelf = TSelfReal or TSelfSynth;
 class TExpr =
   TSelf or TArgumentList or TRescueClause or TRescueModifierExpr or TPair or TStringConcatenation or
       TCall or TBlockArgument or TConstantAccess or TControlExpr or TWhenExpr or TLiteral or
-      TCallable or TVariableAccess or TStmtSequence or TOperation or TSimpleParameter;
+      TCallable or TVariableAccess or TStmtSequence or TOperation or TSimpleParameter or
+      TForwardArgument;
 
 class TSplatExpr = TSplatExprReal or TSplatExprSynth;
 
@@ -677,7 +682,7 @@ class TReturningStmt = TReturnStmt or TBreakStmt or TNextStmt;
 
 class TParameter =
   TPatternParameter or TBlockParameter or THashSplatParameter or TKeywordParameter or
-      TOptionalParameter or TSplatParameter;
+      TOptionalParameter or TSplatParameter or TForwardParameter;
 
 class TPatternParameter = TSimpleParameter or TTuplePatternParameter;
 

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

@@ -742,6 +742,18 @@ module Ruby {
     }
   }
 
+  /** A class representing `forward_argument` tokens. */
+  class ForwardArgument extends @ruby_token_forward_argument, Token {
+    /** Gets the name of the primary QL class for this element. */
+    override string getAPrimaryQlClass() { result = "ForwardArgument" }
+  }
+
+  /** A class representing `forward_parameter` tokens. */
+  class ForwardParameter extends @ruby_token_forward_parameter, Token {
+    /** Gets the name of the primary QL class for this element. */
+    override string getAPrimaryQlClass() { result = "ForwardParameter" }
+  }
+
   /** A class representing `global_variable` tokens. */
   class GlobalVariable extends @ruby_token_global_variable, Token {
     /** Gets the name of the primary QL class for this element. */

ql/lib/codeql/ruby/security/UnsafeDeserializationCustomizations.qll

@@ -67,6 +67,113 @@ module UnsafeDeserialization {
     }
   }
 
+  private string getAKnownOjModeName(boolean isSafe) {
+    result = ["compat", "custom", "json", "null", "rails", "strict", "wab"] and isSafe = true
+    or
+    result = "object" and isSafe = false
+  }
+
+  private predicate isOjModePair(Pair p, string modeValue) {
+    p.getKey().getValueText() = "mode" and
+    exists(DataFlow::LocalSourceNode symbolLiteral, DataFlow::Node value |
+      symbolLiteral.asExpr().getExpr().(SymbolLiteral).getValueText() = modeValue and
+      symbolLiteral.flowsTo(value) and
+      value.asExpr().getExpr() = p.getValue()
+    )
+  }
+
+  /**
+   * A node representing a hash that contains the key `:mode`.
+   */
+  private class OjOptionsHashWithModeKey extends DataFlow::Node {
+    private string modeValue;
+
+    OjOptionsHashWithModeKey() {
+      exists(DataFlow::LocalSourceNode options |
+        options.flowsTo(this) and
+        isOjModePair(options.asExpr().getExpr().(HashLiteral).getAKeyValuePair(), modeValue)
+      )
+    }
+
+    /**
+     * Holds if this hash node contains a `:mode` key whose value is one known
+     * to be `isSafe` with untrusted data.
+     */
+    predicate hasKnownMode(boolean isSafe) { modeValue = getAKnownOjModeName(isSafe) }
+
+    /**
+     * Holds if this hash node contains a `:mode` key whose value is one of the
+     * `Oj` modes known to be safe to use with untrusted data.
+     */
+    predicate hasSafeMode() { this.hasKnownMode(true) }
+  }
+
+  /**
+   * A call node that sets `Oj.default_options`.
+   *
+   * ```rb
+   * Oj.default_options = { allow_blank: true, mode: :compat }
+   * ```
+   */
+  private class SetOjDefaultOptionsCall extends DataFlow::CallNode {
+    SetOjDefaultOptionsCall() {
+      this = API::getTopLevelMember("Oj").getAMethodCall("default_options=")
+    }
+
+    /**
+     * Gets the value being assigned to `Oj.default_options`.
+     */
+    DataFlow::Node getValue() {
+      result.asExpr() =
+        this.getArgument(0).asExpr().(CfgNodes::ExprNodes::AssignExprCfgNode).getRhs()
+    }
+  }
+
+  /**
+   * A call to `Oj.load`.
+   */
+  private class OjLoadCall extends DataFlow::CallNode {
+    OjLoadCall() { this = API::getTopLevelMember("Oj").getAMethodCall("load") }
+
+    /**
+     * Holds if this call to `Oj.load` includes an explicit options hash
+     * argument that sets the mode to one that is known to be `isSafe`.
+     */
+    predicate hasExplicitKnownMode(boolean isSafe) {
+      exists(DataFlow::Node arg, int i | i >= 1 and arg = this.getArgument(i) |
+        arg.(OjOptionsHashWithModeKey).hasKnownMode(isSafe)
+        or
+        isOjModePair(arg.asExpr().getExpr(), getAKnownOjModeName(isSafe))
+      )
+    }
+  }
+
+  /**
+   * An argument in a call to `Oj.load` where the mode is `:object` (which is
+   * the default), considered a sink for unsafe deserialization.
+   */
+  class UnsafeOjLoadArgument extends Sink {
+    UnsafeOjLoadArgument() {
+      exists(OjLoadCall ojLoad |
+        this = ojLoad.getArgument(0) and
+        // Exclude calls that explicitly pass a safe mode option.
+        not ojLoad.hasExplicitKnownMode(true) and
+        (
+          // Sinks to include:
+          // - Calls with an explicit, unsafe mode option.
+          ojLoad.hasExplicitKnownMode(false)
+          or
+          // - Calls with no explicit mode option, unless there exists a call
+          // anywhere to set the default options to a known safe mode.
+          not ojLoad.hasExplicitKnownMode(_) and
+          not exists(SetOjDefaultOptionsCall setOpts |
+            setOpts.getValue().(OjOptionsHashWithModeKey).hasSafeMode()
+          )
+        )
+      )
+    }
+  }
+
   /**
    * `Base64.decode64` propagates taint from its argument to its return value.
    */

ql/lib/ruby.dbscheme

@@ -69,7 +69,7 @@ ruby_alias_def(
   int loc: @location ref
 );
 
-@ruby_argument_list_child_type = @ruby_block_argument | @ruby_break | @ruby_call | @ruby_hash_splat_argument | @ruby_next | @ruby_pair | @ruby_return | @ruby_splat_argument | @ruby_underscore_arg | @ruby_yield
+@ruby_argument_list_child_type = @ruby_block_argument | @ruby_break | @ruby_call | @ruby_hash_splat_argument | @ruby_next | @ruby_pair | @ruby_return | @ruby_splat_argument | @ruby_token_forward_argument | @ruby_underscore_arg | @ruby_yield
 
 #keyset[ruby_argument_list, index]
 ruby_argument_list_child(
@@ -83,7 +83,7 @@ ruby_argument_list_def(
   int loc: @location ref
 );
 
-@ruby_array_child_type = @ruby_block_argument | @ruby_break | @ruby_call | @ruby_hash_splat_argument | @ruby_next | @ruby_pair | @ruby_return | @ruby_splat_argument | @ruby_underscore_arg | @ruby_yield
+@ruby_array_child_type = @ruby_block_argument | @ruby_break | @ruby_call | @ruby_hash_splat_argument | @ruby_next | @ruby_pair | @ruby_return | @ruby_splat_argument | @ruby_token_forward_argument | @ruby_underscore_arg | @ruby_yield
 
 #keyset[ruby_array, index]
 ruby_array_child(
@@ -236,7 +236,7 @@ ruby_block_parameter_def(
   int loc: @location ref
 );
 
-@ruby_block_parameters_child_type = @ruby_block_parameter | @ruby_destructured_parameter | @ruby_hash_splat_parameter | @ruby_keyword_parameter | @ruby_optional_parameter | @ruby_splat_parameter | @ruby_token_identifier
+@ruby_block_parameters_child_type = @ruby_block_parameter | @ruby_destructured_parameter | @ruby_hash_splat_parameter | @ruby_keyword_parameter | @ruby_optional_parameter | @ruby_splat_parameter | @ruby_token_forward_parameter | @ruby_token_identifier
 
 #keyset[ruby_block_parameters, index]
 ruby_block_parameters_child(
@@ -376,7 +376,7 @@ ruby_destructured_left_assignment_def(
   int loc: @location ref
 );
 
-@ruby_destructured_parameter_child_type = @ruby_block_parameter | @ruby_destructured_parameter | @ruby_hash_splat_parameter | @ruby_keyword_parameter | @ruby_optional_parameter | @ruby_splat_parameter | @ruby_token_identifier
+@ruby_destructured_parameter_child_type = @ruby_block_parameter | @ruby_destructured_parameter | @ruby_hash_splat_parameter | @ruby_keyword_parameter | @ruby_optional_parameter | @ruby_splat_parameter | @ruby_token_forward_parameter | @ruby_token_identifier
 
 #keyset[ruby_destructured_parameter, index]
 ruby_destructured_parameter_child(
@@ -423,7 +423,7 @@ ruby_do_block_def(
   int loc: @location ref
 );
 
-@ruby_element_reference_child_type = @ruby_block_argument | @ruby_break | @ruby_call | @ruby_hash_splat_argument | @ruby_next | @ruby_pair | @ruby_return | @ruby_splat_argument | @ruby_underscore_arg | @ruby_yield
+@ruby_element_reference_child_type = @ruby_block_argument | @ruby_break | @ruby_call | @ruby_hash_splat_argument | @ruby_next | @ruby_pair | @ruby_return | @ruby_splat_argument | @ruby_token_forward_argument | @ruby_underscore_arg | @ruby_yield
 
 #keyset[ruby_element_reference, index]
 ruby_element_reference_child(
@@ -643,7 +643,7 @@ ruby_lambda_def(
   int loc: @location ref
 );
 
-@ruby_lambda_parameters_child_type = @ruby_block_parameter | @ruby_destructured_parameter | @ruby_hash_splat_parameter | @ruby_keyword_parameter | @ruby_optional_parameter | @ruby_splat_parameter | @ruby_token_identifier
+@ruby_lambda_parameters_child_type = @ruby_block_parameter | @ruby_destructured_parameter | @ruby_hash_splat_parameter | @ruby_keyword_parameter | @ruby_optional_parameter | @ruby_splat_parameter | @ruby_token_forward_parameter | @ruby_token_identifier
 
 #keyset[ruby_lambda_parameters, index]
 ruby_lambda_parameters_child(
@@ -691,7 +691,7 @@ ruby_method_def(
   int loc: @location ref
 );
 
-@ruby_method_parameters_child_type = @ruby_block_parameter | @ruby_destructured_parameter | @ruby_hash_splat_parameter | @ruby_keyword_parameter | @ruby_optional_parameter | @ruby_splat_parameter | @ruby_token_identifier
+@ruby_method_parameters_child_type = @ruby_block_parameter | @ruby_destructured_parameter | @ruby_hash_splat_parameter | @ruby_keyword_parameter | @ruby_optional_parameter | @ruby_splat_parameter | @ruby_token_forward_parameter | @ruby_token_identifier
 
 #keyset[ruby_method_parameters, index]
 ruby_method_parameters_child(
@@ -1220,22 +1220,24 @@ case @ruby_token.kind of
 | 7 = @ruby_token_escape_sequence
 | 8 = @ruby_token_false
 | 9 = @ruby_token_float
-| 10 = @ruby_token_global_variable
-| 11 = @ruby_token_hash_key_symbol
-| 12 = @ruby_token_heredoc_beginning
-| 13 = @ruby_token_heredoc_content
-| 14 = @ruby_token_heredoc_end
-| 15 = @ruby_token_identifier
-| 16 = @ruby_token_instance_variable
-| 17 = @ruby_token_integer
-| 18 = @ruby_token_nil
-| 19 = @ruby_token_operator
-| 20 = @ruby_token_self
-| 21 = @ruby_token_simple_symbol
-| 22 = @ruby_token_string_content
-| 23 = @ruby_token_super
-| 24 = @ruby_token_true
-| 25 = @ruby_token_uninterpreted
+| 10 = @ruby_token_forward_argument
+| 11 = @ruby_token_forward_parameter
+| 12 = @ruby_token_global_variable
+| 13 = @ruby_token_hash_key_symbol
+| 14 = @ruby_token_heredoc_beginning
+| 15 = @ruby_token_heredoc_content
+| 16 = @ruby_token_heredoc_end
+| 17 = @ruby_token_identifier
+| 18 = @ruby_token_instance_variable
+| 19 = @ruby_token_integer
+| 20 = @ruby_token_nil
+| 21 = @ruby_token_operator
+| 22 = @ruby_token_self
+| 23 = @ruby_token_simple_symbol
+| 24 = @ruby_token_string_content
+| 25 = @ruby_token_super
+| 26 = @ruby_token_true
+| 27 = @ruby_token_uninterpreted
 ;
 
 

ql/lib/upgrades/31a238d080f3dd563d7225fc0458254617d1e5ba/ruby_tokeninfo.ql

@@ -0,0 +1,14 @@
+private class RubyToken extends @ruby_token {
+  string toString() { none() }
+}
+
+private class Location extends @location {
+  string toString() { none() }
+}
+
+bindingset[kind]
+private int newKind(int kind) { if kind >= 10 then result = kind + 2 else result = kind }
+
+from RubyToken id, int kind, string value, Location loc
+where ruby_tokeninfo(id, kind, value, loc)
+select id, newKind(kind), value, loc

ql/lib/upgrades/31a238d080f3dd563d7225fc0458254617d1e5ba/upgrade.properties

@@ -0,0 +1,3 @@
+description: Re-number @ruby_token.kind
+compatibility: full
+ruby_tokeninfo.rel: run ruby_tokeninfo.qlo

ql/src/queries/security/cwe-502/UnsafeDeserialization.qhelp

@@ -21,15 +21,18 @@ deserialization of arbitrary objects.
 
 <example>
 <p>
-The following example calls the <code>Marshal.load</code>, <code>JSON.load</code>, and
-<code>YAML.load</code> methods on data from an HTTP request. Since these methods
-are capable of deserializing to arbitrary objects, this is inherently unsafe.
+The following example calls the <code>Marshal.load</code>,
+<code>JSON.load</code>, <code>YAML.load</code>, and <code>Oj.load</code> methods
+on data from an HTTP request. Since these methods are capable of deserializing
+to arbitrary objects, this is inherently unsafe.
 </p>
 <sample src="examples/UnsafeDeserializationBad.rb"/>
 <p>
 Using <code>JSON.parse</code> and <code>YAML.safe_load</code> instead, as in the
-following example, removes the vulnerability. Note that there is no safe way to
-deserialize untrusted data using <code>Marshal</code>.
+following example, removes the vulnerability. Similarly, calling
+<code>Oj.load</code> with any mode other than <code>:object</code> is safe, as
+is calling <code>Oj.safe_load</code>. Note that there is no safe way to deserialize
+untrusted data using <code>Marshal</code>.
 </p>
 <sample src="examples/UnsafeDeserializationGood.rb"/>
 </example>

ql/src/queries/security/cwe-502/examples/UnsafeDeserializationBad.rb

@@ -1,5 +1,6 @@
 require 'json'
 require 'yaml'
+require 'oj'
 
 class UserController < ActionController::Base
   def marshal_example
@@ -17,4 +18,9 @@ class UserController < ActionController::Base
     object = YAML.load params[:yaml]
     # ...
   end
+
+  def oj_example
+    object = Oj.load params[:json]
+    # ...
+  end
 end

ql/src/queries/security/cwe-502/examples/UnsafeDeserializationGood.rb

@@ -10,4 +10,11 @@ class UserController < ActionController::Base
     object = YAML.safe_load params[:yaml]
     # ...
   end
+
+  def safe_oj_example
+    object = Oj.load params[:yaml], { mode: :strict }
+    # or
+    object = Oj.safe_load params[:yaml]
+    # ...
+  end
 end

ql/test/library-tests/ast/Ast.expected

@@ -611,6 +611,50 @@ calls/calls.rb:
 #  320|             getReceiver: [Self] self
 #  320|         getAnOperand/getArgument/getRightOperand: [IntegerLiteral] 1
 #  320|     getAnOperand/getRightOperand: [IntegerLiteral] 2
+#  323|   getStmt: [Method] foo
+#  323|     getStmt: [MethodCall] call to bar
+#  323|       getReceiver: [Self] self
+#  324|   getStmt: [Method] foo
+#  324|     getStmt: [MethodCall] call to bar
+#  324|       getReceiver: [Self] self
+#  325|   getStmt: [Method] foo
+#  325|     getParameter: [SimpleParameter] x
+#  325|       getDefiningAccess: [LocalVariableAccess] x
+#  325|     getStmt: [MethodCall] call to bar
+#  325|       getReceiver: [Self] self
+#  326|   getStmt: [SingletonMethod] foo
+#  326|     getObject: [ConstantReadAccess] Object
+#  326|     getStmt: [MethodCall] call to bar
+#  326|       getReceiver: [Self] self
+#  327|   getStmt: [SingletonMethod] foo
+#  327|     getObject: [ConstantReadAccess] Object
+#  327|     getParameter: [SimpleParameter] x
+#  327|       getDefiningAccess: [LocalVariableAccess] x
+#  327|     getStmt: [MethodCall] call to bar
+#  327|       getReceiver: [Self] self
+#  328|   getStmt: [Method] foo
+#  328|     getStmt: [RescueModifierExpr] ... rescue ...
+#  328|       getBody: [MethodCall] call to bar
+#  328|         getReceiver: [Self] self
+#  328|       getHandler: [ParenthesizedExpr] ( ... )
+#  328|         getStmt: [MethodCall] call to print
+#  328|           getReceiver: [Self] self
+#  328|           getArgument: [StringLiteral] "error"
+#  328|             getComponent: [StringTextComponent] error
+#  331|   getStmt: [Method] foo
+#  331|     getParameter: [ForwardParameter] ...
+#  332|     getStmt: [SuperCall] call to super
+#  332|       getArgument: [ForwardedArguments] ...
+#  335|   getStmt: [Method] foo
+#  335|     getParameter: [SimpleParameter] a
+#  335|       getDefiningAccess: [LocalVariableAccess] a
+#  335|     getParameter: [SimpleParameter] b
+#  335|       getDefiningAccess: [LocalVariableAccess] b
+#  335|     getParameter: [ForwardParameter] ...
+#  336|     getStmt: [MethodCall] call to bar
+#  336|       getReceiver: [Self] self
+#  336|       getArgument: [LocalVariableAccess] b
+#  336|       getArgument: [ForwardedArguments] ...
 control/cases.rb:
 #    1| [Toplevel] cases.rb
 #    2|   getStmt: [AssignExpr] ... = ...

ql/test/library-tests/ast/ValueText.expected

@@ -55,6 +55,7 @@
 | calls/calls.rb:320:9:320:9 | __synth__1 | 0 |
 | calls/calls.rb:320:31:320:31 | 1 | 1 |
 | calls/calls.rb:320:37:320:37 | 2 | 2 |
+| calls/calls.rb:328:31:328:37 | "error" | error |
 | constants/constants.rb:3:19:3:27 | "const_a" | const_a |
 | constants/constants.rb:6:15:6:23 | "const_b" | const_b |
 | constants/constants.rb:17:12:17:18 | "Hello" | Hello |

ql/test/library-tests/ast/calls/calls.expected

@@ -86,6 +86,10 @@ callsWithArguments
 | calls.rb:320:1:320:32 | call to []= | []= | 4 | calls.rb:320:34:320:35 | __synth__4 |
 | calls.rb:320:21:320:31 | ... + ... | + | 0 | calls.rb:320:31:320:31 | 1 |
 | calls.rb:320:34:320:35 | ... * ... | * | 0 | calls.rb:320:37:320:37 | 2 |
+| calls.rb:328:25:328:37 | call to print | print | 0 | calls.rb:328:31:328:37 | "error" |
+| calls.rb:332:3:332:12 | call to super | super | 0 | calls.rb:332:9:332:11 | ... |
+| calls.rb:336:3:336:13 | call to bar | bar | 0 | calls.rb:336:7:336:7 | b |
+| calls.rb:336:3:336:13 | call to bar | bar | 1 | calls.rb:336:10:336:12 | ... |
 callsWithReceiver
 | calls.rb:2:1:2:5 | call to foo | calls.rb:2:1:2:5 | self |
 | calls.rb:5:1:5:10 | call to bar | calls.rb:5:1:5:3 | Foo |
@@ -316,6 +320,14 @@ callsWithReceiver
 | calls.rb:320:21:320:27 | call to boo | calls.rb:320:21:320:23 | call to foo |
 | calls.rb:320:21:320:31 | ... + ... | calls.rb:320:21:320:27 | call to boo |
 | calls.rb:320:34:320:35 | ... * ... | calls.rb:320:1:320:32 | call to [] |
+| calls.rb:323:11:323:13 | call to bar | calls.rb:323:11:323:13 | self |
+| calls.rb:324:13:324:15 | call to bar | calls.rb:324:13:324:15 | self |
+| calls.rb:325:14:325:16 | call to bar | calls.rb:325:14:325:16 | self |
+| calls.rb:326:18:326:20 | call to bar | calls.rb:326:18:326:20 | self |
+| calls.rb:327:22:327:24 | call to bar | calls.rb:327:22:327:24 | self |
+| calls.rb:328:13:328:15 | call to bar | calls.rb:328:13:328:15 | self |
+| calls.rb:328:25:328:37 | call to print | calls.rb:328:25:328:37 | self |
+| calls.rb:336:3:336:13 | call to bar | calls.rb:336:3:336:13 | self |
 callsWithBlock
 | calls.rb:17:1:17:17 | call to foo | calls.rb:17:5:17:17 | { ... } |
 | calls.rb:20:1:22:3 | call to foo | calls.rb:20:5:22:3 | do ... end |
@@ -339,6 +351,7 @@ superCalls
 | calls.rb:292:5:292:30 | call to super |
 | calls.rb:293:5:293:33 | call to super |
 | calls.rb:305:5:305:9 | call to super |
+| calls.rb:332:3:332:12 | call to super |
 superCallsWithArguments
 | calls.rb:288:5:288:16 | call to super | 0 | calls.rb:288:11:288:16 | "blah" |
 | calls.rb:289:5:289:17 | call to super | 0 | calls.rb:289:11:289:11 | 1 |
@@ -348,6 +361,7 @@ superCallsWithArguments
 | calls.rb:292:5:292:30 | call to super | 1 | calls.rb:292:14:292:14 | 5 |
 | calls.rb:293:5:293:33 | call to super | 0 | calls.rb:293:11:293:11 | 6 |
 | calls.rb:293:5:293:33 | call to super | 1 | calls.rb:293:14:293:14 | 7 |
+| calls.rb:332:3:332:12 | call to super | 0 | calls.rb:332:9:332:11 | ... |
 superCallsWithBlock
 | calls.rb:290:5:290:23 | call to super | calls.rb:290:11:290:23 | { ... } |
 | calls.rb:291:5:291:26 | call to super | calls.rb:291:11:291:26 | do ... end |

ql/test/library-tests/ast/calls/calls.rb

@@ -318,3 +318,20 @@ a, *foo[5] = [1, 2, 3]
 self.count += 1
 foo[0] += 1
 foo.bar[0, foo.baz, foo.boo + 1] *= 2
+
+# endless method definitions
+def foo = bar
+def foo() = bar
+def foo(x) = bar
+def Object.foo = bar
+def Object.foo (x) = bar
+def foo() = bar rescue (print "error")
+
+# forward parameter and forwarded arguments
+def foo(...)
+  super(...)
+end
+
+def foo(a, b, ...)
+  bar(b, ...)
+end

ql/test/query-tests/diagnostics/ExtractionErrors.expected

@@ -1,5 +1,2 @@
 | src/not_ruby.rb:5:25:5:26 | parse error | Extraction failed in src/not_ruby.rb with error parse error | 2 |
-| src/unsupported_feature.rb:2:18:2:20 | parse error | Extraction failed in src/unsupported_feature.rb with error parse error | 2 |
-| src/unsupported_feature.rb:3:13:3:15 | parse error | Extraction failed in src/unsupported_feature.rb with error parse error | 2 |
-| src/unsupported_feature.rb:6:15:6:17 | parse error | Extraction failed in src/unsupported_feature.rb with error parse error | 2 |
-| src/unsupported_feature.rb:7:20:7:22 | parse error | Extraction failed in src/unsupported_feature.rb with error parse error | 2 |
+| src/unsupported_feature.rb:1:6:2:11 | parse error | Extraction failed in src/unsupported_feature.rb with error parse error | 2 |

ql/test/query-tests/diagnostics/src/unsupported_feature.rb

@@ -1,17 +1,3 @@
-class Foo
-  def initialize(...)
-    do_init(...)
-  end
-
-  def do_init(...)
-    really_do_init(...)
-  end
-
-  def really_do_init(bar, baz:, &block)
-    puts bar
-    puts baz
-    block.call
-  end
-end
-
-Foo.new("hello", baz: "world") { || puts "!" }
+case foo
+  in 3 then 5
+end

ql/test/query-tests/security/cwe-502/UnsafeDeserialization.expected

@@ -1,24 +0,0 @@
-edges
-| UnsafeDeserialization.rb:8:39:8:44 | call to params :  | UnsafeDeserialization.rb:9:27:9:41 | serialized_data |
-| UnsafeDeserialization.rb:14:39:14:44 | call to params :  | UnsafeDeserialization.rb:15:30:15:44 | serialized_data |
-| UnsafeDeserialization.rb:20:17:20:22 | call to params :  | UnsafeDeserialization.rb:21:24:21:32 | json_data |
-| UnsafeDeserialization.rb:26:17:26:22 | call to params :  | UnsafeDeserialization.rb:27:27:27:35 | json_data |
-| UnsafeDeserialization.rb:38:17:38:22 | call to params :  | UnsafeDeserialization.rb:39:24:39:32 | yaml_data |
-nodes
-| UnsafeDeserialization.rb:8:39:8:44 | call to params :  | semmle.label | call to params :  |
-| UnsafeDeserialization.rb:9:27:9:41 | serialized_data | semmle.label | serialized_data |
-| UnsafeDeserialization.rb:14:39:14:44 | call to params :  | semmle.label | call to params :  |
-| UnsafeDeserialization.rb:15:30:15:44 | serialized_data | semmle.label | serialized_data |
-| UnsafeDeserialization.rb:20:17:20:22 | call to params :  | semmle.label | call to params :  |
-| UnsafeDeserialization.rb:21:24:21:32 | json_data | semmle.label | json_data |
-| UnsafeDeserialization.rb:26:17:26:22 | call to params :  | semmle.label | call to params :  |
-| UnsafeDeserialization.rb:27:27:27:35 | json_data | semmle.label | json_data |
-| UnsafeDeserialization.rb:38:17:38:22 | call to params :  | semmle.label | call to params :  |
-| UnsafeDeserialization.rb:39:24:39:32 | yaml_data | semmle.label | yaml_data |
-subpaths
-#select
-| UnsafeDeserialization.rb:9:27:9:41 | serialized_data | UnsafeDeserialization.rb:8:39:8:44 | call to params :  | UnsafeDeserialization.rb:9:27:9:41 | serialized_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:8:39:8:44 | call to params | user input |
-| UnsafeDeserialization.rb:15:30:15:44 | serialized_data | UnsafeDeserialization.rb:14:39:14:44 | call to params :  | UnsafeDeserialization.rb:15:30:15:44 | serialized_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:14:39:14:44 | call to params | user input |
-| UnsafeDeserialization.rb:21:24:21:32 | json_data | UnsafeDeserialization.rb:20:17:20:22 | call to params :  | UnsafeDeserialization.rb:21:24:21:32 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:20:17:20:22 | call to params | user input |
-| UnsafeDeserialization.rb:27:27:27:35 | json_data | UnsafeDeserialization.rb:26:17:26:22 | call to params :  | UnsafeDeserialization.rb:27:27:27:35 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:26:17:26:22 | call to params | user input |
-| UnsafeDeserialization.rb:39:24:39:32 | yaml_data | UnsafeDeserialization.rb:38:17:38:22 | call to params :  | UnsafeDeserialization.rb:39:24:39:32 | yaml_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:38:17:38:22 | call to params | user input |

ql/test/query-tests/security/cwe-502/UnsafeDeserialization.rb

@@ -1,47 +0,0 @@
-require "base64"
-require "json"
-require "yaml"
-
-class UsersController < ActionController::Base
-  # BAD
-  def route0
-    serialized_data = Base64.decode64 params[:key]
-    object = Marshal.load serialized_data
-  end
-
-  # BAD
-  def route1
-    serialized_data = Base64.decode64 params[:key]
-    object = Marshal.restore serialized_data
-  end
-
-  # BAD
-  def route2
-    json_data = params[:key]
-    object = JSON.load json_data
-  end
-
-  # BAD
-  def route3
-    json_data = params[:key]
-    object = JSON.restore json_data
-  end
-
-  # GOOD - JSON.parse is safe to use on untrusted data
-  def route4
-    json_data = params[:key]
-    object = JSON.parse json_data
-  end
-
-  # BAD
-  def route5
-    yaml_data = params[:key]
-    object = YAML.load yaml_data
-  end
-
-  # GOOD
-  def route6
-    yaml_data = params[:key]
-    object = YAML.safe_load yaml_data
-  end
-end

ql/test/query-tests/security/cwe-502/oj-global-options/OjGlobalOptions.rb

@@ -0,0 +1,16 @@
+require "oj"
+
+class UsersController < ActionController::Base
+  # GOOD - Oj.load is safe when any mode other than :object is set globally
+  def route0
+    json_data = params[:key]
+    object = Oj.load json_data
+  end
+
+  # BAD - the safe mode set globally is overridden with an unsafe mode passed as
+  # a call argument
+  def route1
+    json_data = params[:key]
+    object = Oj.load json_data, mode: :object
+  end
+end

ql/test/query-tests/security/cwe-502/oj-global-options/Startup.rb

@@ -0,0 +1,5 @@
+require "oj"
+
+# Set the default mode for the Oj library to use the :compat mode, which makes
+# Oj.load safe for untrusted data.
+Oj.default_options = { :mode => :compat }

ql/test/query-tests/security/cwe-502/oj-global-options/UnsafeDeserialization.expected

@@ -0,0 +1,8 @@
+edges
+| OjGlobalOptions.rb:13:17:13:22 | call to params :  | OjGlobalOptions.rb:14:22:14:30 | json_data |
+nodes
+| OjGlobalOptions.rb:13:17:13:22 | call to params :  | semmle.label | call to params :  |
+| OjGlobalOptions.rb:14:22:14:30 | json_data | semmle.label | json_data |
+subpaths
+#select
+| OjGlobalOptions.rb:14:22:14:30 | json_data | OjGlobalOptions.rb:13:17:13:22 | call to params :  | OjGlobalOptions.rb:14:22:14:30 | json_data | Unsafe deserialization of $@. | OjGlobalOptions.rb:13:17:13:22 | call to params | user input |

ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.expected

@@ -0,0 +1,35 @@
+edges
+| UnsafeDeserialization.rb:9:39:9:44 | call to params :  | UnsafeDeserialization.rb:10:27:10:41 | serialized_data |
+| UnsafeDeserialization.rb:15:39:15:44 | call to params :  | UnsafeDeserialization.rb:16:30:16:44 | serialized_data |
+| UnsafeDeserialization.rb:21:17:21:22 | call to params :  | UnsafeDeserialization.rb:22:24:22:32 | json_data |
+| UnsafeDeserialization.rb:27:17:27:22 | call to params :  | UnsafeDeserialization.rb:28:27:28:35 | json_data |
+| UnsafeDeserialization.rb:39:17:39:22 | call to params :  | UnsafeDeserialization.rb:40:24:40:32 | yaml_data |
+| UnsafeDeserialization.rb:51:17:51:22 | call to params :  | UnsafeDeserialization.rb:52:22:52:30 | json_data |
+| UnsafeDeserialization.rb:51:17:51:22 | call to params :  | UnsafeDeserialization.rb:53:22:53:30 | json_data |
+| UnsafeDeserialization.rb:58:17:58:22 | call to params :  | UnsafeDeserialization.rb:68:23:68:31 | json_data |
+nodes
+| UnsafeDeserialization.rb:9:39:9:44 | call to params :  | semmle.label | call to params :  |
+| UnsafeDeserialization.rb:10:27:10:41 | serialized_data | semmle.label | serialized_data |
+| UnsafeDeserialization.rb:15:39:15:44 | call to params :  | semmle.label | call to params :  |
+| UnsafeDeserialization.rb:16:30:16:44 | serialized_data | semmle.label | serialized_data |
+| UnsafeDeserialization.rb:21:17:21:22 | call to params :  | semmle.label | call to params :  |
+| UnsafeDeserialization.rb:22:24:22:32 | json_data | semmle.label | json_data |
+| UnsafeDeserialization.rb:27:17:27:22 | call to params :  | semmle.label | call to params :  |
+| UnsafeDeserialization.rb:28:27:28:35 | json_data | semmle.label | json_data |
+| UnsafeDeserialization.rb:39:17:39:22 | call to params :  | semmle.label | call to params :  |
+| UnsafeDeserialization.rb:40:24:40:32 | yaml_data | semmle.label | yaml_data |
+| UnsafeDeserialization.rb:51:17:51:22 | call to params :  | semmle.label | call to params :  |
+| UnsafeDeserialization.rb:52:22:52:30 | json_data | semmle.label | json_data |
+| UnsafeDeserialization.rb:53:22:53:30 | json_data | semmle.label | json_data |
+| UnsafeDeserialization.rb:58:17:58:22 | call to params :  | semmle.label | call to params :  |
+| UnsafeDeserialization.rb:68:23:68:31 | json_data | semmle.label | json_data |
+subpaths
+#select
+| UnsafeDeserialization.rb:10:27:10:41 | serialized_data | UnsafeDeserialization.rb:9:39:9:44 | call to params :  | UnsafeDeserialization.rb:10:27:10:41 | serialized_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:9:39:9:44 | call to params | user input |
+| UnsafeDeserialization.rb:16:30:16:44 | serialized_data | UnsafeDeserialization.rb:15:39:15:44 | call to params :  | UnsafeDeserialization.rb:16:30:16:44 | serialized_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:15:39:15:44 | call to params | user input |
+| UnsafeDeserialization.rb:22:24:22:32 | json_data | UnsafeDeserialization.rb:21:17:21:22 | call to params :  | UnsafeDeserialization.rb:22:24:22:32 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:21:17:21:22 | call to params | user input |
+| UnsafeDeserialization.rb:28:27:28:35 | json_data | UnsafeDeserialization.rb:27:17:27:22 | call to params :  | UnsafeDeserialization.rb:28:27:28:35 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:27:17:27:22 | call to params | user input |
+| UnsafeDeserialization.rb:40:24:40:32 | yaml_data | UnsafeDeserialization.rb:39:17:39:22 | call to params :  | UnsafeDeserialization.rb:40:24:40:32 | yaml_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:39:17:39:22 | call to params | user input |
+| UnsafeDeserialization.rb:52:22:52:30 | json_data | UnsafeDeserialization.rb:51:17:51:22 | call to params :  | UnsafeDeserialization.rb:52:22:52:30 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:51:17:51:22 | call to params | user input |
+| UnsafeDeserialization.rb:53:22:53:30 | json_data | UnsafeDeserialization.rb:51:17:51:22 | call to params :  | UnsafeDeserialization.rb:53:22:53:30 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:51:17:51:22 | call to params | user input |
+| UnsafeDeserialization.rb:68:23:68:31 | json_data | UnsafeDeserialization.rb:58:17:58:22 | call to params :  | UnsafeDeserialization.rb:68:23:68:31 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:58:17:58:22 | call to params | user input |

ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.qlref

@@ -0,0 +1 @@
+queries/security/cwe-502/UnsafeDeserialization.ql

ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.rb

@@ -0,0 +1,76 @@
+require "base64"
+require "json"
+require "oj"
+require "yaml"
+
+class UsersController < ActionController::Base
+  # BAD
+  def route0
+    serialized_data = Base64.decode64 params[:key]
+    object = Marshal.load serialized_data
+  end
+
+  # BAD
+  def route1
+    serialized_data = Base64.decode64 params[:key]
+    object = Marshal.restore serialized_data
+  end
+
+  # BAD
+  def route2
+    json_data = params[:key]
+    object = JSON.load json_data
+  end
+
+  # BAD
+  def route3
+    json_data = params[:key]
+    object = JSON.restore json_data
+  end
+
+  # GOOD - JSON.parse is safe to use on untrusted data
+  def route4
+    json_data = params[:key]
+    object = JSON.parse json_data
+  end
+
+  # BAD
+  def route5
+    yaml_data = params[:key]
+    object = YAML.load yaml_data
+  end
+
+  # GOOD
+  def route6
+    yaml_data = params[:key]
+    object = YAML.safe_load yaml_data
+  end
+
+  # BAD - Oj.load is unsafe in its default :object mode
+  def route7
+    json_data = params[:key]
+    object = Oj.load json_data
+    object = Oj.load json_data, mode: :object
+  end
+
+  # GOOD - Oj.load is safe in any other mode
+  def route8
+    json_data = params[:key]
+    # Test the different ways the options hash can be passed
+    options = { allow_blank: true, mode: :rails }
+    object1 = Oj.load json_data, options
+    object2 = Oj.load json_data, mode: :strict
+    object3 = Oj.load json_data, :allow_blank => true, :mode => :compat
+
+    # TODO: false positive; we aren't detecting flow from `:json` to the call argument.
+    more_options = { allow_blank: true }
+    more_options[:mode] = :json
+    object4 = Oj.load json_data, more_options
+  end
+
+  # GOOD
+  def route9
+    json_data = params[:key]
+    object = Oj.safe_load json_data
+  end
+end