add stdin as source for unsafe-deserialization

Author: erik-krogh

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

@@ -10,12 +10,16 @@ private import codeql.ruby.DataFlow
 private import codeql.ruby.dataflow.RemoteFlowSources
 private import codeql.ruby.frameworks.ActiveJob
 private import codeql.ruby.frameworks.core.Module
+private import codeql.ruby.frameworks.core.Kernel
 
 module UnsafeDeserialization {
   /**
    * A data flow source for unsafe deserialization vulnerabilities.
    */
-  abstract class Source extends DataFlow::Node { }
+  abstract class Source extends DataFlow::Node {
+    /** Gets a string that describes the source. */
+    string describe() { result = "user-provided value" }
+  }
 
   /**
    * A data flow sink for unsafe deserialization vulnerabilities.
@@ -37,6 +41,36 @@ module UnsafeDeserialization {
   /** A source of remote user input, considered as a flow source for unsafe deserialization. */
   class RemoteFlowSourceAsSource extends Source instanceof RemoteFlowSource { }
 
+  /** A read of data from `STDIN`/`ARGV`, considered as a flow source for unsafe deserialization. */
+  class StdInSource extends UnsafeDeserialization::Source {
+    boolean stdin;
+
+    StdInSource() {
+      this = API::getTopLevelMember(["STDIN", "ARGF"]).getAMethodCall(["gets", "read"]) and
+      stdin = true
+      or
+      // > $stdin == STDIN
+      // => true
+      // but $stdin is special in that it is a global variable and not a constant. `API::getTopLevelMember` only gets constants.
+      exists(DataFlow::Node dollarStdin |
+        dollarStdin.asExpr().getExpr().(GlobalVariableReadAccess).getVariable().getName() = "$stdin" and
+        this = dollarStdin.getALocalSource().getAMethodCall(["gets", "read"])
+      ) and
+      stdin = true
+      or
+      // ARGV.
+      this.asExpr().getExpr().(GlobalVariableReadAccess).getVariable().getName() = "ARGV" and
+      stdin = false
+      or
+      this.(Kernel::KernelMethodCall).getMethodName() = ["gets", "readline", "readlines"] and
+      stdin = true
+    }
+
+    override string describe() {
+      if stdin = true then result = "value from stdin" else result = "value from ARGV"
+    }
+  }
+
   /**
    * An argument in a call to `Marshal.load` or `Marshal.restore`, considered a
    * sink for unsafe deserialization.

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

@@ -11,12 +11,11 @@
  *       external/cwe/cwe-502
  */
 
-import codeql.ruby.AST
-import DataFlow::PathGraph
-import codeql.ruby.DataFlow
+import ruby
 import codeql.ruby.security.UnsafeDeserializationQuery
+import DataFlow::PathGraph
 
 from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
 where cfg.hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "Unsafe deserialization depends on a $@.", source.getNode(),
-  "user-provided value"
+  source.getNode().(UnsafeDeserialization::Source).describe()

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

@@ -53,6 +53,11 @@ nodes
 | UnsafeDeserialization.rb:93:30:93:43 | ...[...] | semmle.label | ...[...] |
 | UnsafeDeserialization.rb:99:48:99:53 | call to params :  | semmle.label | call to params :  |
 | UnsafeDeserialization.rb:99:48:99:61 | ...[...] | semmle.label | ...[...] |
+| UnsafeDeserialization.rb:104:24:104:34 | call to read | semmle.label | call to read |
+| UnsafeDeserialization.rb:107:24:107:33 | call to gets | semmle.label | call to gets |
+| UnsafeDeserialization.rb:110:24:110:32 | call to read | semmle.label | call to read |
+| UnsafeDeserialization.rb:113:24:113:27 | call to gets | semmle.label | call to gets |
+| UnsafeDeserialization.rb:116:24:116:32 | call to readlines | semmle.label | call to readlines |
 subpaths
 #select
 | UnsafeDeserialization.rb:11:27:11:41 | serialized_data | UnsafeDeserialization.rb:10:39:10:44 | call to params :  | UnsafeDeserialization.rb:11:27:11:41 | serialized_data | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:10:39:10:44 | call to params | user-provided value |
@@ -67,3 +72,8 @@ subpaths
 | UnsafeDeserialization.rb:88:25:88:33 | yaml_data | UnsafeDeserialization.rb:87:17:87:22 | call to params :  | UnsafeDeserialization.rb:88:25:88:33 | yaml_data | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:87:17:87:22 | call to params | user-provided value |
 | UnsafeDeserialization.rb:93:30:93:43 | ...[...] | UnsafeDeserialization.rb:93:30:93:35 | call to params :  | UnsafeDeserialization.rb:93:30:93:43 | ...[...] | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:93:30:93:35 | call to params | user-provided value |
 | UnsafeDeserialization.rb:99:48:99:61 | ...[...] | UnsafeDeserialization.rb:99:48:99:53 | call to params :  | UnsafeDeserialization.rb:99:48:99:61 | ...[...] | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:99:48:99:53 | call to params | user-provided value |
+| UnsafeDeserialization.rb:104:24:104:34 | call to read | UnsafeDeserialization.rb:104:24:104:34 | call to read | UnsafeDeserialization.rb:104:24:104:34 | call to read | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:104:24:104:34 | call to read | value from stdin |
+| UnsafeDeserialization.rb:107:24:107:33 | call to gets | UnsafeDeserialization.rb:107:24:107:33 | call to gets | UnsafeDeserialization.rb:107:24:107:33 | call to gets | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:107:24:107:33 | call to gets | value from stdin |
+| UnsafeDeserialization.rb:110:24:110:32 | call to read | UnsafeDeserialization.rb:110:24:110:32 | call to read | UnsafeDeserialization.rb:110:24:110:32 | call to read | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:110:24:110:32 | call to read | value from stdin |
+| UnsafeDeserialization.rb:113:24:113:27 | call to gets | UnsafeDeserialization.rb:113:24:113:27 | call to gets | UnsafeDeserialization.rb:113:24:113:27 | call to gets | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:113:24:113:27 | call to gets | value from stdin |
+| UnsafeDeserialization.rb:116:24:116:32 | call to readlines | UnsafeDeserialization.rb:116:24:116:32 | call to readlines | UnsafeDeserialization.rb:116:24:116:32 | call to readlines | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:116:24:116:32 | call to readlines | value from stdin |

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

@@ -99,4 +99,20 @@ def route13
     klass = ActiveJob::Serializers.deserialize(params[:class])
     object = klass.new
   end
+
+  def stdin
+    object = YAML.load $stdin.read
+
+    # STDIN
+    object = YAML.load STDIN.gets
+
+    # ARGF
+    object = YAML.load ARGF.read
+
+    # Kernel.gets
+    object = YAML.load gets
+
+    # Kernel.readlines
+    object = YAML.load readlines
+  end
 end