ruby / xmlrpc

The Ruby standard library package 'xmlrpc'
Other
37 stars 26 forks source link

Content-Type is not text/xml leads to error #24

Closed lucaskohstall closed 4 years ago

lucaskohstall commented 4 years ago

If the responses content-type is not text/xml, the client.rb prints out an error, even if the http_header_field is set to another content-type and the request was send correctly.

Wrong content-type (received 'application/xml' but expected 'text/xml')

The reason for this is client.rb line 519, where you will get an error in any case if the content type is not equal text/html

if ct != "text/xml"
        if ct == "text/html"
          raise "Wrong content-type (received '#{ct}' but expected 'text/xml'): \n#{data}"
        else
          raise "Wrong content-type (received '#{ct}' but expected 'text/xml')"
        end
      end
kou commented 4 years ago

Thanks for your report. I've fixed it.

znz commented 4 years ago

XML-RPC spec says The Content-Type is text/xml in Header requirements. So I think XML-RPC does not accept application/xml.

kou commented 4 years ago

This is response case not request case. I think that accepting newer content-type for XML too isn't so bad. It doesn't break compatibility against strict implementation.

Do you think we must keep strict implementation for response?

znz commented 4 years ago

Loose validations sometimes make vulnerabilities. So I afraid it to cause a vulnerability. If some xmlrpc libraries already accept application/xml, I am acceptable for compatibilities.

kou commented 4 years ago

What type of vulnerability do you imagine?

If some xmlrpc libraries already accept application/xml, I am acceptable for compatibilities.

Could you check it and report?

znz commented 4 years ago

I'm afraid of CWE-20. I can't imagine a concrete example.

znz commented 4 years ago

I think that the specification should be changed before changing libraries, if no other libraries use application/xml.

kou commented 4 years ago

I also can't image this causes CWE-20 related vulnerability. If you still think we need to check other libraries, could you do it, report the result and reopen this. I close this until it because nobody shows any vulnerability for now.