diff --git a/NEWS.md b/NEWS.md index e35f9a5..d8495b7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,15 @@ # News +## 0.3.3 + +### Fixes + + * Fixed a security vulnerability when `ENABLE_MARSHELLING` is set to true + +### Thanks + + * Hassen DHAHBI(plenum) + ## 0.3.2 ### Improvements diff --git a/lib/xmlrpc/parser.rb b/lib/xmlrpc/parser.rb index a58da33..5069e0c 100644 --- a/lib/xmlrpc/parser.rb +++ b/lib/xmlrpc/parser.rb @@ -113,7 +113,7 @@ def self.struct(hash) begin mod = Module klass.split("::").each {|const| mod = mod.const_get(const.strip)} - + return hash unless mod.included_modules.include?(Marshallable) obj = mod.allocate hash.delete "___class___" diff --git a/test/test_marshal.rb b/test/test_marshal.rb index 0f8e2be..a8d9d4e 100644 --- a/test/test_marshal.rb +++ b/test/test_marshal.rb @@ -13,6 +13,13 @@ def initialize(name) end end + # for test_load_call_class_not_marshallable + class Person2 + attr_reader :name + def initialize(name) + @name = name + end + end def test1_dump_response assert_nothing_raised(NameError) { @@ -107,5 +114,23 @@ def test_no_params_tag assert_equal(expect, str) end + # tests for vulnerability of unsafe deserialization when ENABLE_MARSHALLING is set to true + def test_load_call_class_marshallable + # return of load call should contain an instance of Person + input_xml = %{myMethod___class___TestXMLRPC::Test_Marshal::PersonnameJohn Doe\n} + m = XMLRPC::Marshal.load_call(input_xml) + assert_kind_of( Person, m[1][0] ) + assert_instance_of( Person, m[1][0] ) + end + + def test_load_call_class_not_marshallable + # return of load call should contain hash instance since Person2 does not include XMLRPC::Marshallable + hash_exp = Hash.new + input_xml = %{myMethod___class___TestXMLRPC::Test_Marshal::Person2nameJohn Doe\n} + m= XMLRPC::Marshal.load_call(input_xml) + assert_kind_of( Hash, m[1][0] ) + assert_instance_of( Hash, m[1][0] ) + end + end end