From 0521ffe175243a5bd9071c2f9bb231ced31d3ece Mon Sep 17 00:00:00 2001 From: amammad Date: Fri, 24 Feb 2023 13:13:06 +0100 Subject: [PATCH] v1.4 correct dirs uppercase issue --- .../YAMLUnsafeYamlDeserialization.qhelp | 20 +++++ .../cwe-502/YAMLUnsafeYamlDeserialization.ql | 88 +++++++++++++++++++ .../cwe-502/YAMLUnsafeYamlDeserialization.rb | 22 +++++ .../YAMLUnsafeYamlDeserialization.expected | 34 +++++++ .../YAMLUnsafeYamlDeserialization.qlref | 1 + .../cwe-502/YAMLUnsafeYamlDeserialization.rb | 22 +++++ 6 files changed, 187 insertions(+) create mode 100644 ruby/ql/src/experimental/cwe-502/YAMLUnsafeYamlDeserialization.qhelp create mode 100644 ruby/ql/src/experimental/cwe-502/YAMLUnsafeYamlDeserialization.ql create mode 100644 ruby/ql/src/experimental/cwe-502/YAMLUnsafeYamlDeserialization.rb create mode 100644 ruby/ql/test/query-tests/experimental/Security/cwe-502/YAMLUnsafeYamlDeserialization.expected create mode 100644 ruby/ql/test/query-tests/experimental/Security/cwe-502/YAMLUnsafeYamlDeserialization.qlref create mode 100644 ruby/ql/test/query-tests/experimental/Security/cwe-502/YAMLUnsafeYamlDeserialization.rb diff --git a/ruby/ql/src/experimental/cwe-502/YAMLUnsafeYamlDeserialization.qhelp b/ruby/ql/src/experimental/cwe-502/YAMLUnsafeYamlDeserialization.qhelp new file mode 100644 index 00000000000..821747e0b52 --- /dev/null +++ b/ruby/ql/src/experimental/cwe-502/YAMLUnsafeYamlDeserialization.qhelp @@ -0,0 +1,20 @@ + + + +

+ Processing an unvalidated user input can allow an attacker to execute arbitrary code in your application. + Unsafe deserializing the malicious serialized yaml document through the Psych (YAML) library, making it possible to execute some code or execute arbitrary code with the help of a complete gadget chain. +

+
+ +

+ After Psych(YAML) 4.0.0, the load method is same as safe_load method. + This vulnerability can be prevented by using YAML.load (same as YAML.safe_load), YAML.load_file (same as YAML.safe_load_file) instead of YAML.unsafe_* methods. + Be careful that YAML.load_stream don't use safe_load method, Also Be careful the to_ruby method of Psych get called on a trusted parsed (YAML.parse*) yaml document. +

+
+ +

In the example below, you can see safe and unsafe methods get called by a remote user input. You can give correct authorization to users, or you can use safe methods for loading yaml documents.

+ +
+
\ No newline at end of file diff --git a/ruby/ql/src/experimental/cwe-502/YAMLUnsafeYamlDeserialization.ql b/ruby/ql/src/experimental/cwe-502/YAMLUnsafeYamlDeserialization.ql new file mode 100644 index 00000000000..a3f42a9c84e --- /dev/null +++ b/ruby/ql/src/experimental/cwe-502/YAMLUnsafeYamlDeserialization.ql @@ -0,0 +1,88 @@ +/** + * @name Deserialization of user-controlled data by YAML + * @description Deserializing user-controlled data may allow attackers to + * execute arbitrary code. + * @kind path-problem + * @problem.severity warning + * @security-severity 9.8 + * @precision high + * @id rb/YAML-unsafe-deserialization + * @tags security + * experimental + * external/cwe/cwe-502 + */ + +import codeql.ruby.ApiGraphs +import codeql.ruby.DataFlow +import codeql.ruby.TaintTracking +import DataFlow::PathGraph +import codeql.ruby.security.UnsafeDeserializationCustomizations + +abstract class YamlSink extends DataFlow::Node { } + +class YamlUnsafeLoadArgument extends YamlSink { + YamlUnsafeLoadArgument() { + 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") + or + this = + API::getTopLevelMember(["YAML", "Psych"]) + .getAMethodCall(["parse", "parse_stream", "parse_file"]) + .getAMethodCall("to_ruby") + } +} + +class Configuration extends TaintTracking::Configuration { + Configuration() { this = "UnsafeYAMLDeserialization" } + + override predicate isSource(DataFlow::Node source) { + // to detect CVE-2022-32224, we should uncomment following line instead of current UnsafeDeserialization::Source + // source instanceof DataFlow::LocalSourceNode + source instanceof UnsafeDeserialization::Source + } + + override predicate isSink(DataFlow::Node sink) { + // after changing the isSource for detecting CVE-2022-32224 + // uncomment the following line only see the CVE sink not other files similar sinks + // sink.getLocation().getFile().toString().matches("%yaml_column%") and + sink instanceof YamlSink + } + + 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") + ) + } +} + +from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink +where config.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "This file extraction depends on a $@.", source.getNode(), + "potentially untrusted source" diff --git a/ruby/ql/src/experimental/cwe-502/YAMLUnsafeYamlDeserialization.rb b/ruby/ql/src/experimental/cwe-502/YAMLUnsafeYamlDeserialization.rb new file mode 100644 index 00000000000..6e836a0a049 --- /dev/null +++ b/ruby/ql/src/experimental/cwe-502/YAMLUnsafeYamlDeserialization.rb @@ -0,0 +1,22 @@ +require 'yaml' +class UsersController < ActionController::Base + def example + # safe + Psych.load(params[:yaml_string]) + Psych.load_file(params[:yaml_file]) + Psych.parse_stream(params[:yaml_string]) + Psych.parse(params[:yaml_string]) + Psych.parse_file(params[:yaml_file]) + # unsafe + Psych.unsafe_load(params[:yaml_string]) + Psych.unsafe_load_file(params[:yaml_file]) + Psych.load_stream(params[:yaml_string]) + parse_output = Psych.parse_stream(params[:yaml_string]) + parse_output.to_ruby + Psych.parse(params[:yaml_string]).to_ruby + Psych.parse_file(params[:yaml_file]).to_ruby + + end +end + + diff --git a/ruby/ql/test/query-tests/experimental/Security/cwe-502/YAMLUnsafeYamlDeserialization.expected b/ruby/ql/test/query-tests/experimental/Security/cwe-502/YAMLUnsafeYamlDeserialization.expected new file mode 100644 index 00000000000..c01afdfbe69 --- /dev/null +++ b/ruby/ql/test/query-tests/experimental/Security/cwe-502/YAMLUnsafeYamlDeserialization.expected @@ -0,0 +1,34 @@ +edges +| YAMLUnsafeYamlDeserialization.rb:11:23:11:28 | call to params : | YAMLUnsafeYamlDeserialization.rb:11:23:11:42 | ...[...] | +| YAMLUnsafeYamlDeserialization.rb:12:28:12:33 | call to params : | YAMLUnsafeYamlDeserialization.rb:12:28:12:45 | ...[...] | +| YAMLUnsafeYamlDeserialization.rb:13:23:13:28 | call to params : | YAMLUnsafeYamlDeserialization.rb:13:23:13:42 | ...[...] | +| YAMLUnsafeYamlDeserialization.rb:14:39:14:44 | call to params : | YAMLUnsafeYamlDeserialization.rb:14:39:14:58 | ...[...] : | +| YAMLUnsafeYamlDeserialization.rb:14:39:14:58 | ...[...] : | YAMLUnsafeYamlDeserialization.rb:15:5:15:24 | call to to_ruby | +| YAMLUnsafeYamlDeserialization.rb:16:17:16:22 | call to params : | YAMLUnsafeYamlDeserialization.rb:16:17:16:36 | ...[...] : | +| YAMLUnsafeYamlDeserialization.rb:16:17:16:36 | ...[...] : | YAMLUnsafeYamlDeserialization.rb:16:5:16:45 | call to to_ruby | +| YAMLUnsafeYamlDeserialization.rb:17:22:17:27 | call to params : | YAMLUnsafeYamlDeserialization.rb:17:22:17:39 | ...[...] : | +| YAMLUnsafeYamlDeserialization.rb:17:22:17:39 | ...[...] : | YAMLUnsafeYamlDeserialization.rb:17:5:17:48 | call to to_ruby | +nodes +| YAMLUnsafeYamlDeserialization.rb:11:23:11:28 | call to params : | semmle.label | call to params : | +| YAMLUnsafeYamlDeserialization.rb:11:23:11:42 | ...[...] | semmle.label | ...[...] | +| YAMLUnsafeYamlDeserialization.rb:12:28:12:33 | call to params : | semmle.label | call to params : | +| YAMLUnsafeYamlDeserialization.rb:12:28:12:45 | ...[...] | semmle.label | ...[...] | +| YAMLUnsafeYamlDeserialization.rb:13:23:13:28 | call to params : | semmle.label | call to params : | +| YAMLUnsafeYamlDeserialization.rb:13:23:13:42 | ...[...] | semmle.label | ...[...] | +| YAMLUnsafeYamlDeserialization.rb:14:39:14:44 | call to params : | semmle.label | call to params : | +| YAMLUnsafeYamlDeserialization.rb:14:39:14:58 | ...[...] : | semmle.label | ...[...] : | +| YAMLUnsafeYamlDeserialization.rb:15:5:15:24 | call to to_ruby | semmle.label | call to to_ruby | +| YAMLUnsafeYamlDeserialization.rb:16:5:16:45 | call to to_ruby | semmle.label | call to to_ruby | +| YAMLUnsafeYamlDeserialization.rb:16:17:16:22 | call to params : | semmle.label | call to params : | +| YAMLUnsafeYamlDeserialization.rb:16:17:16:36 | ...[...] : | semmle.label | ...[...] : | +| YAMLUnsafeYamlDeserialization.rb:17:5:17:48 | call to to_ruby | semmle.label | call to to_ruby | +| YAMLUnsafeYamlDeserialization.rb:17:22:17:27 | call to params : | semmle.label | call to params : | +| YAMLUnsafeYamlDeserialization.rb:17:22:17:39 | ...[...] : | semmle.label | ...[...] : | +subpaths +#select +| YAMLUnsafeYamlDeserialization.rb:11:23:11:42 | ...[...] | YAMLUnsafeYamlDeserialization.rb:11:23:11:28 | call to params : | YAMLUnsafeYamlDeserialization.rb:11:23:11:42 | ...[...] | This file extraction depends on a $@. | YAMLUnsafeYamlDeserialization.rb:11:23:11:28 | call to params | potentially untrusted source | +| YAMLUnsafeYamlDeserialization.rb:12:28:12:45 | ...[...] | YAMLUnsafeYamlDeserialization.rb:12:28:12:33 | call to params : | YAMLUnsafeYamlDeserialization.rb:12:28:12:45 | ...[...] | This file extraction depends on a $@. | YAMLUnsafeYamlDeserialization.rb:12:28:12:33 | call to params | potentially untrusted source | +| YAMLUnsafeYamlDeserialization.rb:13:23:13:42 | ...[...] | YAMLUnsafeYamlDeserialization.rb:13:23:13:28 | call to params : | YAMLUnsafeYamlDeserialization.rb:13:23:13:42 | ...[...] | This file extraction depends on a $@. | YAMLUnsafeYamlDeserialization.rb:13:23:13:28 | call to params | potentially untrusted source | +| YAMLUnsafeYamlDeserialization.rb:15:5:15:24 | call to to_ruby | YAMLUnsafeYamlDeserialization.rb:14:39:14:44 | call to params : | YAMLUnsafeYamlDeserialization.rb:15:5:15:24 | call to to_ruby | This file extraction depends on a $@. | YAMLUnsafeYamlDeserialization.rb:14:39:14:44 | call to params | potentially untrusted source | +| YAMLUnsafeYamlDeserialization.rb:16:5:16:45 | call to to_ruby | YAMLUnsafeYamlDeserialization.rb:16:17:16:22 | call to params : | YAMLUnsafeYamlDeserialization.rb:16:5:16:45 | call to to_ruby | This file extraction depends on a $@. | YAMLUnsafeYamlDeserialization.rb:16:17:16:22 | call to params | potentially untrusted source | +| YAMLUnsafeYamlDeserialization.rb:17:5:17:48 | call to to_ruby | YAMLUnsafeYamlDeserialization.rb:17:22:17:27 | call to params : | YAMLUnsafeYamlDeserialization.rb:17:5:17:48 | call to to_ruby | This file extraction depends on a $@. | YAMLUnsafeYamlDeserialization.rb:17:22:17:27 | call to params | potentially untrusted source | diff --git a/ruby/ql/test/query-tests/experimental/Security/cwe-502/YAMLUnsafeYamlDeserialization.qlref b/ruby/ql/test/query-tests/experimental/Security/cwe-502/YAMLUnsafeYamlDeserialization.qlref new file mode 100644 index 00000000000..7e9e87117f9 --- /dev/null +++ b/ruby/ql/test/query-tests/experimental/Security/cwe-502/YAMLUnsafeYamlDeserialization.qlref @@ -0,0 +1 @@ +experimental/CWE-502/YAMLUnsafeYamlDeserialization.ql \ No newline at end of file diff --git a/ruby/ql/test/query-tests/experimental/Security/cwe-502/YAMLUnsafeYamlDeserialization.rb b/ruby/ql/test/query-tests/experimental/Security/cwe-502/YAMLUnsafeYamlDeserialization.rb new file mode 100644 index 00000000000..6e836a0a049 --- /dev/null +++ b/ruby/ql/test/query-tests/experimental/Security/cwe-502/YAMLUnsafeYamlDeserialization.rb @@ -0,0 +1,22 @@ +require 'yaml' +class UsersController < ActionController::Base + def example + # safe + Psych.load(params[:yaml_string]) + Psych.load_file(params[:yaml_file]) + Psych.parse_stream(params[:yaml_string]) + Psych.parse(params[:yaml_string]) + Psych.parse_file(params[:yaml_file]) + # unsafe + Psych.unsafe_load(params[:yaml_string]) + Psych.unsafe_load_file(params[:yaml_file]) + Psych.load_stream(params[:yaml_string]) + parse_output = Psych.parse_stream(params[:yaml_string]) + parse_output.to_ruby + Psych.parse(params[:yaml_string]).to_ruby + Psych.parse_file(params[:yaml_file]).to_ruby + + end +end + +