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

[Georg Faerber]

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.0 +0100
+++ ruby-loofah-2.0.3/debian/changelog  2018-03-24 16:13:55.0 +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   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.patch1970-01-01 
01:00:00.0 +0100
+++ ruby-loofah-2.0.3/debian/patches/CVE-2018-8048.patch2018-03-24 
16:13:55.0 +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
+--- 

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

[Cédric Boutillier]

Hi!

On Sat, Mar 24, 2018 at 04:41:17PM +0100, Georg Faerber wrote:

> Some notes (doing this for the first time..):
> 
> - AFAIK, the delta should be kept as small as possible, that's why I
>   didn't added a description for the patch.

It is better to add DEP-3 header anyway. The size of the pach refers
only to the size of the actual code change, not to metadata, which could
help the security team, and maybe us later, by centralizing in one place
the description, links to upstream and Debian bug, and to the origin of
the patch.

The upstream commit contains tests for this security issue. I think you
should add this part too in your patch. You'll have a way to be (more)
convinced that the fix indeed works.

> 
> - I've closed the bug targeted at unstable via the changelog, again. Not
>   sure if this is the correct way? I've used this approach to keep all
>   information in one place, which is a good thing, IMHO.

It is the correct way.

Cédric


signature.asc
Description: PGP signature