Hi security team,

Friendly ping on this? Did you had a chance to do a review?
Anything I could do to fix ruby-loofah in stretch?

Thanks for your work,
cheers,
Georg

On 18-03-25 19:10:40, Georg Faerber wrote:
> 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 <ge...@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 via email to