Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: Disable ENABLE_MARSHALLING by default #39

Open
herwinw opened this issue Aug 7, 2023 · 2 comments
Open

Suggestion: Disable ENABLE_MARSHALLING by default #39

herwinw opened this issue Aug 7, 2023 · 2 comments

Comments

@herwinw
Copy link
Member

herwinw commented Aug 7, 2023

Given the plethora of cases about possible RCE vulnerabilities that have led to the 0.3.3 release, I would like to argue that the default object marshalling is incorrect.

Beside the security related issues, there is also the issue that XML-RPC is a language agnostic protocol, and the Ruby object marshalling is a Ruby only extension. It would not make any sense to serialize a Ruby object when the server is running Perl, Python or any other language that is not Ruby. The other XML-RPC extensions (8 byte integers, nil serialization, nil deserialization) are disabled by default as well.

Of course, I might be totally wrong here. Are there any people who actually use the object serialization of this gem?

@kou
Copy link
Member

kou commented Aug 7, 2023

I can understand what you want to say but it breaks backward compatibility.

And the marshaling isn't happen unless a user writes include XMLRPC::Marshallable explicitly. It acts like that the marshaling is disabled by default (it's not done implicitly).

kou pushed a commit that referenced this issue Aug 24, 2023
Currently the config is made up of a number of constants, changing the
config does require some boilerplate to redefine constants. An example:
```ruby
XMLRPC::Config.module_eval { remove_const(:ENABLE_NIL_PARSER) }
XMLRPC::Config::ENABLE_NIL_PARSER = true
```

This change adds accessor methods to the Config module, so you can
simply write them like this:
```ruby
XMLRPC::Config.enable_nil_parser = true
```

It still uses the constants under water, which means it is fully
backwards compatible (which was the big objection in #39 )
@anakinj
Copy link

anakinj commented Jan 26, 2024

Gonna inject my 50 cents in this discussion since it's a bit related to the Marshalling issues.

Considering the fact that this gem is pretty wildly used, would it make sense to issue an CVE for the pre 0.3.3 version. So security scanners could flag outdated dependencies?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants