From ccb5d375b8ca65a34bfad29227f02170564d1d83 Mon Sep 17 00:00:00 2001 From: Ben Murphy Date: Fri, 8 Feb 2013 02:48:22 +0000 Subject: [PATCH] JDOM XXE Protection Conflicts: activesupport/test/xml_mini/jdom_engine_test.rb --- activesupport/lib/active_support/xml_mini/jdom.rb | 6 ++++ activesupport/test/fixtures/xml/jdom_doctype.dtd | 1 + activesupport/test/fixtures/xml/jdom_entities.txt | 1 + activesupport/test/fixtures/xml/jdom_include.txt | 1 + activesupport/test/xml_mini/jdom_engine_test.rb | 39 +++++++++++++++++++++-- 5 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 activesupport/test/fixtures/xml/jdom_doctype.dtd create mode 100644 activesupport/test/fixtures/xml/jdom_entities.txt create mode 100644 activesupport/test/fixtures/xml/jdom_include.txt diff --git a/activesupport/lib/active_support/xml_mini/jdom.rb b/activesupport/lib/active_support/xml_mini/jdom.rb index 0dee0f3..45a315e 100644 --- a/activesupport/lib/active_support/xml_mini/jdom.rb +++ b/activesupport/lib/active_support/xml_mini/jdom.rb @@ -38,6 +38,12 @@ module ActiveSupport {} else @dbf = DocumentBuilderFactory.new_instance + # secure processing of java xml + # http://www.ibm.com/developerworks/xml/library/x-tipcfsx/index.html + @dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false) + @dbf.setFeature("http://xml.org/sax/features/external-general-entities", false) + @dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false) + @dbf.setFeature(javax.xml.XMLConstants::FEATURE_SECURE_PROCESSING, true) xml_string_reader = StringReader.new(data) xml_input_source = InputSource.new(xml_string_reader) doc = @dbf.new_document_builder.parse(xml_input_source) diff --git a/activesupport/test/fixtures/xml/jdom_doctype.dtd b/activesupport/test/fixtures/xml/jdom_doctype.dtd new file mode 100644 index 0000000..8948049 --- /dev/null +++ b/activesupport/test/fixtures/xml/jdom_doctype.dtd @@ -0,0 +1 @@ + diff --git a/activesupport/test/fixtures/xml/jdom_entities.txt b/activesupport/test/fixtures/xml/jdom_entities.txt new file mode 100644 index 0000000..0337fda --- /dev/null +++ b/activesupport/test/fixtures/xml/jdom_entities.txt @@ -0,0 +1 @@ + diff --git a/activesupport/test/fixtures/xml/jdom_include.txt b/activesupport/test/fixtures/xml/jdom_include.txt new file mode 100644 index 0000000..239ca3a --- /dev/null +++ b/activesupport/test/fixtures/xml/jdom_include.txt @@ -0,0 +1 @@ +include me diff --git a/activesupport/test/xml_mini/jdom_engine_test.rb b/activesupport/test/xml_mini/jdom_engine_test.rb index 7f809e7..ec81ada 100644 --- a/activesupport/test/xml_mini/jdom_engine_test.rb +++ b/activesupport/test/xml_mini/jdom_engine_test.rb @@ -3,9 +3,11 @@ if RUBY_PLATFORM =~ /java/ require 'active_support/xml_mini' require 'active_support/core_ext/hash/conversions' - class JDOMEngineTest < Test::Unit::TestCase + class JDOMEngineTest < ActiveSupport::TestCase include ActiveSupport + FILES_DIR = File.dirname(__FILE__) + '/../fixtures/xml' + def setup @default_backend = XmlMini.backend XmlMini.backend = 'JDOM' @@ -30,10 +32,41 @@ if RUBY_PLATFORM =~ /java/ assert_equal 'image/png', file.content_type end + def test_not_allowed_to_expand_entities_to_files + attack_xml = <<-EOT + + ]> + x&a; + EOT + assert_equal 'x', Hash.from_xml(attack_xml)["member"] + end + + def test_not_allowed_to_expand_parameter_entities_to_files + attack_xml = <<-EOT + + %b; + ]> + x&a; + EOT + assert_raise Java::OrgXmlSax::SAXParseException do + assert_equal 'x', Hash.from_xml(attack_xml)["member"] + end + end + + + def test_not_allowed_to_load_external_doctypes + attack_xml = <<-EOT + + x&a; + EOT + assert_equal 'x', Hash.from_xml(attack_xml)["member"] + end + def test_exception_thrown_on_expansion_attack - assert_raise NativeException do + assert_raise Java::OrgXmlSax::SAXParseException do attack_xml = <<-EOT - -- 1.8.1.1