[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: ruby-loofah 2.0.3-2 (stretch) update (CVE-2018-8048)



Hi security team,

On 18-03-22 17:23:48, Moritz Muehlenhoff wrote:
> On Thu, Mar 22, 2018 at 05:21:15PM +0100, Georg Faerber wrote:
> > I would like to fix CVE-2018-8048, which is currently present in
> > ruby-loofah 2.0.3-2 in stretch. Do you prefer an "straight" upload
> > done by you, or should this be instead an upload via stretch-pu?
> > 
> > In any case, I'll prepare a patch.
> 
> Thanks. I think we should fix this via security.debian.org

Please find the debdiff below. Changes pushed to git [1] in branch
stretch/backports.

Please note: The first iteration of the patch didn't included DEP3
headers. Also, I didn't added the new test case. After review of the
Ruby team, I've changed this. I've removed blank lines included in the
upstream commit to keep the delta as small as possible.

I'll prepare an upload regarding #893994 targeted at stretch as well,
once ruby-loofah is fixed, because this is a prerequisite. This is why
the below proposal doesn't include 'private', in contrast to the
upstream patch, to allow public use of this function. This was changed
upstream in a subsequent release, 2.2.2 [1]. I guess that #893994 should
be fixed via an upload / DSA as well, please correct me, if I'm wrong
regarding this.

Please tell me if the below is acceptable, or changes are needed.

Thanks for your work,
cheers,
Georg


[1] https://salsa.debian.org/ruby-team/ruby-loofah
[2] https://github.com/flavorjones/loofah/commit/56e95a6696b1e17a242eb8ebbbab64d613c4f1fe


diff -Nru ruby-loofah-2.0.3/debian/changelog ruby-loofah-2.0.3/debian/changelog
--- ruby-loofah-2.0.3/debian/changelog  2016-01-07 14:22:29.000000000 +0100
+++ ruby-loofah-2.0.3/debian/changelog  2018-03-24 16:13:55.000000000 +0100                                                                                                                                         
@@ -1,3 +1,11 @@
+ruby-loofah (2.0.3-2+deb9u1) stretch-security; urgency=high
+
+  * Introduce upstream patch to address a potential cross-site scripting
+    vulnerability caused by libxml2 >= 2.9.2. (Closes: #893596)
+    (CVE-2018-8048)
+
+ -- Georg Faerber <georg@riseup.net>  Sat, 24 Mar 2018 16:13:55 +0100
+
 ruby-loofah (2.0.3-2) unstable; urgency=medium
 
   * fix-tests-assert.patch: Patch to fix test failures (Closes: #808449) 
diff -Nru ruby-loofah-2.0.3/debian/patches/CVE-2018-8048.patch ruby-loofah-2.0.3/debian/patches/CVE-2018-8048.patch
--- ruby-loofah-2.0.3/debian/patches/CVE-2018-8048.patch    1970-01-01 01:00:00.000000000 +0100
+++ ruby-loofah-2.0.3/debian/patches/CVE-2018-8048.patch    2018-03-24 16:13:55.000000000 +0100
@@ -0,0 +1,99 @@
+Description: Patch to address potential XSS vuln (CVE-2018-8048)
+  libxml2 >= 2.9.2 fails to escape comments within some attributes. It
+  wants to ensure these comments can be treated as "server-side
+  includes", but as a result fails to ensure that serialization is
+  well-formed, resulting in an opportunity for XSS injection of code
+  into a final re-parsed document (presumably in a browser).
+Origin: upstream
+Debian-Bug: #893596
+Applied-Upstream: https://github.com/flavorjones/loofah/commit/4a08c25a603654f2fc505a7d2bf0c35a39870ad7
+Last-Update: 2018-03-25
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+--- a/lib/loofah.rb
++++ b/lib/loofah.rb
+@@ -6,6 +6,7 @@
+ require 'loofah/elements'
+ 
+ require 'loofah/html5/whitelist'
++require 'loofah/html5/libxml2_workarounds'
+ require 'loofah/html5/scrub'
+ 
+ require 'loofah/scrubber'
+--- /dev/null
++++ b/lib/loofah/html5/libxml2_workarounds.rb
+@@ -0,0 +1,12 @@
++require 'set'
++module Loofah
++  module LibxmlWorkarounds
++    BROKEN_ESCAPING_ATTRIBUTES = Set.new %w[
++        href
++        action
++        src
++        name
++      ]
++    BROKEN_ESCAPING_ATTRIBUTES_QUALIFYING_TAG = {"name" => "a"}
++  end
++end
+--- a/lib/loofah/html5/scrub.rb
++++ b/lib/loofah/html5/scrub.rb
+@@ -54,6 +54,7 @@
+           node.attribute_nodes.each do |attr_node|
+             node.remove_attribute(attr_node.name) if attr_node.value !~ /[^[:space:]]/
+           end
++          force_correct_attribute_escaping! node
+         end
+ 
+         def scrub_css_attribute node
+@@ -89,6 +90,18 @@
+           style = clean.join(' ')
+         end
+ 
++        def force_correct_attribute_escaping! node
++          return unless Nokogiri::VersionInfo.instance.libxml2?
++          node.attribute_nodes.each do |attr_node|
++            next unless LibxmlWorkarounds::BROKEN_ESCAPING_ATTRIBUTES.include?(attr_node.name)
++            tag_name = LibxmlWorkarounds::BROKEN_ESCAPING_ATTRIBUTES_QUALIFYING_TAG[attr_node.name]
++            next unless tag_name.nil? || tag_name == node.name
++            encoding = attr_node.value.encoding
++            attr_node.value = attr_node.value.gsub(/[ "]/) do |m|
++              '%' + m.unpack('H2' * m.bytesize).join('%').upcase
++            end.force_encoding(encoding)
++          end
++        end
+       end
+ 
+     end
+--- a/test/integration/test_ad_hoc.rb
++++ b/test/integration/test_ad_hoc.rb
+@@ -173,4 +173,30 @@
+     html = "<p>Foo</p>\n<p>Bar</p>"
+     assert_equal "Foo\nBar", Loofah.scrub_document(html, :prune).text
+   end
++  [
++      {tag: "a",   attr: "href"},
++      {tag: "div", attr: "href"},
++      {tag: "a",   attr: "action"},
++      {tag: "div", attr: "action"},
++      {tag: "a",   attr: "src"},
++      {tag: "div", attr: "src"},
++      {tag: "a",   attr: "name"},
++      {tag: "div", attr: "name", unescaped: true},
++    ].each do |config|
++      define_method "test_uri_escaping_of_#{config[:attr]}_attr_in_#{config[:tag]}_tag" do
++        html = %{<#{config[:tag]} #{config[:attr]}='examp<!--" unsafeattr=foo()>-->le.com'>test</#{config[:tag]}>}
++        reparsed = Loofah.fragment(Loofah.fragment(html).scrub!(:prune).to_html)
++        attributes = reparsed.at_css(config[:tag]).attribute_nodes
++        assert_equal [config[:attr]], attributes.collect(&:name)
++        if Nokogiri::VersionInfo.new.libxml2?
++          if config[:unescaped]
++            assert_equal %{examp<!--" unsafeattr=foo()>-->le.com}, attributes.first.value
++          else
++            assert_equal %{examp<!--%22%20unsafeattr=foo()>-->le.com}, attributes.first.value
++          end
++        else
++          assert_equal %{examp<!--%22 unsafeattr=foo()>-->le.com}, attributes.first.value
++        end
++      end
++    end
+ end
diff -Nru ruby-loofah-2.0.3/debian/patches/series ruby-loofah-2.0.3/debian/patches/series
--- ruby-loofah-2.0.3/debian/patches/series 2016-01-07 14:18:08.000000000 +0100
+++ ruby-loofah-2.0.3/debian/patches/series 2018-03-24 16:13:55.000000000 +0100
@@ -1,2 +1,3 @@
+CVE-2018-8048.patch
 fix-tests-assert.patch
 dont_require_lib_files.patch

Attachment: signature.asc
Description: Digital signature


Reply to: