add the new YAML/PLIST sinks into the existing rb/unsafe-deserialization query

Author: am0o0

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

@@ -75,13 +75,41 @@ module UnsafeDeserialization {
   }
 
   /**
-   * An argument in a call to `YAML.load`, considered a sink
+   * An argument in a call to `YAML.unsafe_*` and `YAML.load_stream` , considered sinks
    * for unsafe deserialization. The `YAML` module is an alias of `Psych` in
    * recent versions of Ruby.
    */
   class YamlLoadArgument extends Sink {
     YamlLoadArgument() {
-      this = API::getTopLevelMember(["YAML", "Psych"]).getAMethodCall("load").getArgument(0)
+      this =
+        API::getTopLevelMember(["YAML", "Psych"])
+            .getAMethodCall(["unsafe_load_file", "unsafe_load", "load_stream"])
+            .getArgument(0)
+      or
+      this =
+        API::getTopLevelMember(["YAML", "Psych"])
+            .getAMethodCall(["unsafe_load", "load_stream"])
+            .getKeywordArgument("yaml")
+      or
+      this =
+        API::getTopLevelMember(["YAML", "Psych"])
+            .getAMethodCall("unsafe_load_file")
+            .getKeywordArgument("filename")
+    }
+  }
+
+  /**
+   * An argument in a call to `YAML.parse*`, considered sinks
+   * for unsafe deserialization if there is a call to `to_ruby` on returned value of them,
+   * so this need some additional taint steps. The `YAML` module is an alias of `Psych` in
+   * recent versions of Ruby.
+   */
+  class YamlParseArgument extends Sink {
+    YamlParseArgument() {
+      this =
+        API::getTopLevelMember(["YAML", "Psych"])
+            .getAMethodCall(["parse", "parse_stream", "parse_file"])
+            .getAMethodCall("to_ruby")
     }
   }
 
@@ -208,4 +236,31 @@ module UnsafeDeserialization {
       )
     }
   }
+
+  /**
+   * check whether an input argument has desired "key: value" input or not.
+   */
+  predicate checkkeyValue(CfgNodes::ExprNodes::PairCfgNode p, string key, string value) {
+    p.getKey().getConstantValue().isStringlikeValue(key) and
+    DataFlow::exprNode(p.getValue()).getALocalSource().getConstantValue().toString() = value
+  }
+
+  /**
+   * An argument in a call to `Plist.parse_xml` where the marshal is `true` (which is
+   * the default), considered a sink for unsafe deserialization.
+   */
+  class UnsafePlistParsexmlArgument extends Sink {
+    UnsafePlistParsexmlArgument() {
+      exists(DataFlow::CallNode plistParsexml |
+        plistParsexml = API::getTopLevelMember("Plist").getAMethodCall("parse_xml")
+      |
+        this = [plistParsexml.getArgument(0), plistParsexml.getKeywordArgument("filename_or_xml")] and
+        // Exclude calls that explicitly pass a safe mode option.
+        checkkeyValue(plistParsexml.getArgument(1).asExpr(), "marshal", "true")
+        or
+        this = [plistParsexml.getArgument(0), plistParsexml.getKeywordArgument("filename_or_xml")] and
+        plistParsexml.getNumberOfArguments() = 1
+      )
+    }
+  }
 }

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

@@ -9,6 +9,7 @@
 private import codeql.ruby.AST
 private import codeql.ruby.DataFlow
 private import codeql.ruby.TaintTracking
+private import codeql.ruby.ApiGraphs
 import UnsafeDeserializationCustomizations
 
 /**
@@ -23,6 +24,27 @@ class Configuration extends TaintTracking::Configuration {
 
   override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeDeserialization::Sink }
 
+  override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    exists(DataFlow::CallNode yaml_parser_methods |
+      yaml_parser_methods =
+        API::getTopLevelMember(["YAML", "Psych"]).getAMethodCall(["parse", "parse_stream"]) and
+      (
+        nodeFrom = yaml_parser_methods.getArgument(0) or
+        nodeFrom = yaml_parser_methods.getKeywordArgument("yaml")
+      ) and
+      nodeTo = yaml_parser_methods.getAMethodCall("to_ruby")
+    )
+    or
+    exists(DataFlow::CallNode yaml_parser_methods |
+      yaml_parser_methods = API::getTopLevelMember(["YAML", "Psych"]).getAMethodCall("parse_file") and
+      (
+        nodeFrom = yaml_parser_methods.getArgument(0) or
+        nodeFrom = yaml_parser_methods.getKeywordArgument("filename")
+      ) and
+      nodeTo = yaml_parser_methods.getAMethodCall("to_ruby")
+    )
+  }
+
   override predicate isSanitizer(DataFlow::Node node) {
     super.isSanitizer(node) or
     node instanceof UnsafeDeserialization::Sanitizer