From 364bc883ba3056c19f29002958046cab84492523 Mon Sep 17 00:00:00 2001 From: Alex Ford Date: Fri, 23 Sep 2022 15:54:15 +0100 Subject: [PATCH] Ruby: add YAML.load_file as an unsafe deserialization sink --- .../change-notes/2022-09-23-yaml-load-file.md | 4 ++++ .../UnsafeDeserializationCustomizations.qll | 9 ++++++--- .../UnsafeDeserialization.expected | 18 ++++++++++++++++++ .../UnsafeDeserialization.rb | 18 ++++++++++++++++++ 4 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 ruby/ql/lib/change-notes/2022-09-23-yaml-load-file.md diff --git a/ruby/ql/lib/change-notes/2022-09-23-yaml-load-file.md b/ruby/ql/lib/change-notes/2022-09-23-yaml-load-file.md new file mode 100644 index 00000000000..3bb4345a3fc --- /dev/null +++ b/ruby/ql/lib/change-notes/2022-09-23-yaml-load-file.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added `YAML.load_file` as a potential sink for unsafe deserialization. diff --git a/ruby/ql/lib/codeql/ruby/security/UnsafeDeserializationCustomizations.qll b/ruby/ql/lib/codeql/ruby/security/UnsafeDeserializationCustomizations.qll index 91b3dd80606..f56544a8d8b 100644 --- a/ruby/ql/lib/codeql/ruby/security/UnsafeDeserializationCustomizations.qll +++ b/ruby/ql/lib/codeql/ruby/security/UnsafeDeserializationCustomizations.qll @@ -48,12 +48,15 @@ module UnsafeDeserialization { } /** - * An argument in a call to `YAML.load`, considered a sink for unsafe - * deserialization. + * An argument in a call to `YAML.load` or `YAML.load_file`, considered a sink + * for unsafe deserialization. As the `YAML` module is an alias of `Psych` in */ class YamlLoadArgument extends Sink { YamlLoadArgument() { - this = API::getTopLevelMember("YAML").getAMethodCall("load").getArgument(0) + this = + API::getTopLevelMember(["YAML", "Psych"]) + .getAMethodCall(["load", "load_file"]) + .getArgument(0) } } diff --git a/ruby/ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.expected b/ruby/ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.expected index f81f114ae8f..4cdab3af4dc 100644 --- a/ruby/ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.expected +++ b/ruby/ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.expected @@ -16,6 +16,12 @@ edges | UnsafeDeserialization.rb:58:17:58:28 | ...[...] : | UnsafeDeserialization.rb:68:23:68:31 | json_data | | UnsafeDeserialization.rb:80:11:80:16 | call to params : | UnsafeDeserialization.rb:80:11:80:22 | ...[...] : | | UnsafeDeserialization.rb:80:11:80:22 | ...[...] : | UnsafeDeserialization.rb:81:34:81:36 | xml | +| UnsafeDeserialization.rb:86:17:86:22 | call to params : | UnsafeDeserialization.rb:86:17:86:28 | ...[...] : | +| UnsafeDeserialization.rb:86:17:86:28 | ...[...] : | UnsafeDeserialization.rb:87:29:87:37 | yaml_path | +| UnsafeDeserialization.rb:92:17:92:22 | call to params : | UnsafeDeserialization.rb:92:17:92:28 | ...[...] : | +| UnsafeDeserialization.rb:92:17:92:28 | ...[...] : | UnsafeDeserialization.rb:93:25:93:33 | yaml_data | +| UnsafeDeserialization.rb:98:17:98:22 | call to params : | UnsafeDeserialization.rb:98:17:98:28 | ...[...] : | +| UnsafeDeserialization.rb:98:17:98:28 | ...[...] : | UnsafeDeserialization.rb:99:30:99:38 | yaml_path | nodes | UnsafeDeserialization.rb:9:39:9:44 | call to params : | semmle.label | call to params : | | UnsafeDeserialization.rb:9:39:9:50 | ...[...] : | semmle.label | ...[...] : | @@ -42,6 +48,15 @@ nodes | UnsafeDeserialization.rb:80:11:80:16 | call to params : | semmle.label | call to params : | | UnsafeDeserialization.rb:80:11:80:22 | ...[...] : | semmle.label | ...[...] : | | UnsafeDeserialization.rb:81:34:81:36 | xml | semmle.label | xml | +| UnsafeDeserialization.rb:86:17:86:22 | call to params : | semmle.label | call to params : | +| UnsafeDeserialization.rb:86:17:86:28 | ...[...] : | semmle.label | ...[...] : | +| UnsafeDeserialization.rb:87:29:87:37 | yaml_path | semmle.label | yaml_path | +| UnsafeDeserialization.rb:92:17:92:22 | call to params : | semmle.label | call to params : | +| UnsafeDeserialization.rb:92:17:92:28 | ...[...] : | semmle.label | ...[...] : | +| UnsafeDeserialization.rb:93:25:93:33 | yaml_data | semmle.label | yaml_data | +| UnsafeDeserialization.rb:98:17:98:22 | call to params : | semmle.label | call to params : | +| UnsafeDeserialization.rb:98:17:98:28 | ...[...] : | semmle.label | ...[...] : | +| UnsafeDeserialization.rb:99:30:99:38 | yaml_path | semmle.label | yaml_path | 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 depends on a $@. | UnsafeDeserialization.rb:9:39:9:44 | call to params | user-provided value | @@ -53,3 +68,6 @@ subpaths | 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 depends on a $@. | UnsafeDeserialization.rb:51:17:51:22 | call to params | user-provided value | | 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 depends on a $@. | UnsafeDeserialization.rb:58:17:58:22 | call to params | user-provided value | | UnsafeDeserialization.rb:81:34:81:36 | xml | UnsafeDeserialization.rb:80:11:80:16 | call to params : | UnsafeDeserialization.rb:81:34:81:36 | xml | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:80:11:80:16 | call to params | user-provided value | +| UnsafeDeserialization.rb:87:29:87:37 | yaml_path | UnsafeDeserialization.rb:86:17:86:22 | call to params : | UnsafeDeserialization.rb:87:29:87:37 | yaml_path | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:86:17:86:22 | call to params | user-provided value | +| UnsafeDeserialization.rb:93:25:93:33 | yaml_data | UnsafeDeserialization.rb:92:17:92:22 | call to params : | UnsafeDeserialization.rb:93:25:93:33 | yaml_data | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:92:17:92:22 | call to params | user-provided value | +| UnsafeDeserialization.rb:99:30:99:38 | yaml_path | UnsafeDeserialization.rb:98:17:98:22 | call to params : | UnsafeDeserialization.rb:99:30:99:38 | yaml_path | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:98:17:98:22 | call to params | user-provided value | diff --git a/ruby/ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.rb b/ruby/ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.rb index fc6bc198d41..27bb1c35f34 100644 --- a/ruby/ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.rb +++ b/ruby/ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.rb @@ -80,4 +80,22 @@ class UsersController < ActionController::Base xml = params[:key] hash = Hash.from_trusted_xml(xml) end + + # BAD + def route11 + yaml_path = params[:key] + object = YAML.load_file yaml_path + end + + # BAD + def route12 + yaml_data = params[:key] + object = Psych.load yaml_data + end + + # BAD + def route13 + yaml_path = params[:key] + object = Psych.load_file yaml_path + end end