drive-by: delete code-execution sinks from unsafe-deserialization, we risked duplicate alerts

Author: erik-krogh

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

@@ -229,27 +229,4 @@ module UnsafeDeserialization {
       toNode = callNode
     )
   }
-
-  /**
-   * A argument in a call to `Module.const_get`, considered as a sink for unsafe
-   * deserialization.
-   *
-   * Calls to `Module.const_get` can return arbitrary classes which can then be
-   * instantiated.
-   */
-  class ConstGetCallArgument extends Sink {
-    ConstGetCallArgument() { this = any(Module::ModuleConstGetCallCodeExecution c).getCode() }
-  }
-
-  /**
-   * A argument in a call to `ActiveJob::Serializers.deserialize`, considered as
-   * a sink for unsafe deserialization.
-   *
-   * This is roughly equivalent to a call to `Module.const_get`.
-   */
-  class ActiveJobSerializersDeserializeArgument extends Sink {
-    ActiveJobSerializersDeserializeArgument() {
-      this = any(ActiveJob::Serializers::DeserializeCall c).getCode()
-    }
-  }
 }

ruby/ql/test/library-tests/dataflow/local/TaintStep.expected

@@ -2804,6 +2804,7 @@
 | file://:0:0:0:0 | parameter position 0 of ActionController::Parameters#merge! | file://:0:0:0:0 | [summary] to write: argument self in ActionController::Parameters#merge! |
 | file://:0:0:0:0 | parameter position 0 of ActionController::Parameters#merge! | file://:0:0:0:0 | [summary] to write: return (return) in ActionController::Parameters#merge! |
 | file://:0:0:0:0 | parameter position 0 of Arel.sql | file://:0:0:0:0 | [summary] to write: return (return) in Arel.sql |
+| file://:0:0:0:0 | parameter position 0 of Base64.decode64() | file://:0:0:0:0 | [summary] to write: return (return) in Base64.decode64() |
 | file://:0:0:0:0 | parameter position 0 of File.absolute_path | file://:0:0:0:0 | [summary] to write: return (return) in File.absolute_path |
 | file://:0:0:0:0 | parameter position 0 of File.dirname | file://:0:0:0:0 | [summary] to write: return (return) in File.dirname |
 | file://:0:0:0:0 | parameter position 0 of File.expand_path | file://:0:0:0:0 | [summary] to write: return (return) in File.expand_path |

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

@@ -18,8 +18,6 @@ edges
 | UnsafeDeserialization.rb:81:11:81:22 | ...[...] :  | UnsafeDeserialization.rb:82:34:82:36 | xml |
 | UnsafeDeserialization.rb:87:17:87:22 | call to params :  | UnsafeDeserialization.rb:87:17:87:28 | ...[...] :  |
 | UnsafeDeserialization.rb:87:17:87:28 | ...[...] :  | UnsafeDeserialization.rb:88:25:88:33 | yaml_data |
-| UnsafeDeserialization.rb:93:30:93:35 | call to params :  | UnsafeDeserialization.rb:93:30:93:43 | ...[...] |
-| UnsafeDeserialization.rb:99:48:99:53 | call to params :  | UnsafeDeserialization.rb:99:48:99:61 | ...[...] |
 nodes
 | UnsafeDeserialization.rb:10:39:10:44 | call to params :  | semmle.label | call to params :  |
 | UnsafeDeserialization.rb:10:39:10:50 | ...[...] :  | semmle.label | ...[...] :  |
@@ -49,15 +47,11 @@ nodes
 | UnsafeDeserialization.rb:87:17:87:22 | call to params :  | semmle.label | call to params :  |
 | UnsafeDeserialization.rb:87:17:87:28 | ...[...] :  | semmle.label | ...[...] :  |
 | UnsafeDeserialization.rb:88:25:88:33 | yaml_data | semmle.label | yaml_data |
-| UnsafeDeserialization.rb:93:30:93:35 | call to params :  | semmle.label | call to params :  |
-| 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 |
+| UnsafeDeserialization.rb:92:24:92:34 | call to read | semmle.label | call to read |
+| UnsafeDeserialization.rb:95:24:95:33 | call to gets | semmle.label | call to gets |
+| UnsafeDeserialization.rb:98:24:98:32 | call to read | semmle.label | call to read |
+| UnsafeDeserialization.rb:101:24:101:27 | call to gets | semmle.label | call to gets |
+| UnsafeDeserialization.rb:104:24:104: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 |
@@ -70,10 +64,8 @@ subpaths
 | UnsafeDeserialization.rb:69:23:69:31 | json_data | UnsafeDeserialization.rb:59:17:59:22 | call to params :  | UnsafeDeserialization.rb:69:23:69:31 | json_data | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:59:17:59:22 | call to params | user-provided value |
 | UnsafeDeserialization.rb:82:34:82:36 | xml | UnsafeDeserialization.rb:81:11:81:16 | call to params :  | UnsafeDeserialization.rb:82:34:82:36 | xml | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:81:11:81:16 | call to params | user-provided value |
 | 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 |
+| UnsafeDeserialization.rb:92:24:92:34 | call to read | UnsafeDeserialization.rb:92:24:92:34 | call to read | UnsafeDeserialization.rb:92:24:92:34 | call to read | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:92:24:92:34 | call to read | value from stdin |
+| UnsafeDeserialization.rb:95:24:95:33 | call to gets | UnsafeDeserialization.rb:95:24:95:33 | call to gets | UnsafeDeserialization.rb:95:24:95:33 | call to gets | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:95:24:95:33 | call to gets | value from stdin |
+| UnsafeDeserialization.rb:98:24:98:32 | call to read | UnsafeDeserialization.rb:98:24:98:32 | call to read | UnsafeDeserialization.rb:98:24:98:32 | call to read | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:98:24:98:32 | call to read | value from stdin |
+| UnsafeDeserialization.rb:101:24:101:27 | call to gets | UnsafeDeserialization.rb:101:24:101:27 | call to gets | UnsafeDeserialization.rb:101:24:101:27 | call to gets | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:101:24:101:27 | call to gets | value from stdin |
+| UnsafeDeserialization.rb:104:24:104:32 | call to readlines | UnsafeDeserialization.rb:104:24:104:32 | call to readlines | UnsafeDeserialization.rb:104:24:104:32 | call to readlines | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:104:24:104:32 | call to readlines | value from stdin |

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

@@ -88,18 +88,6 @@ def route11
     object = Psych.load yaml_data
   end
 
-  # BAD - user input determines which class is instantiated
-  def route12
-    klass = Module.const_get(params[:class])
-    object = klass.new
-  end
-
-  # BAD - user input determines which class is instantiated
-  def route13
-    klass = ActiveJob::Serializers.deserialize(params[:class])
-    object = klass.new
-  end
-
   def stdin
     object = YAML.load $stdin.read