Merge branch 'security-fj-bumping-sanitize-gem' into 'master'

[master] Update sanitize gem to 4.6.5 to fix HTML injection vulnerability

See merge request gitlab/gitlabhq!2399

6 files changed, 35 insertions, 11 deletions

diff --git a/Gemfile b/Gemfile
index 945b5486437..93c6115eeec 100644
--- a/Gemfile
+++ b/Gemfile
@@ -230,7 +230,7 @@ gem 'ruby-fogbugz', '~> 0.2.1'
 gem 'kubeclient', '~> 3.1.0'
 
 # Sanitize user input
-gem 'sanitize', '~> 2.0'
+gem 'sanitize', '~> 4.6.5'
 gem 'babosa', '~> 1.0.2'
 
 # Sanitizes SVG input
diff --git a/Gemfile.lock b/Gemfile.lock
index 3c619b2e534..f55965d2113 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -295,13 +295,13 @@ GEM
       flowdock (~> 0.7)
       gitlab-grit (>= 2.4.1)
       multi_json
-    gitlab-gollum-lib (4.2.7.4)
+    gitlab-gollum-lib (4.2.7.5)
       gemojione (~> 3.2)
       github-markup (~> 1.6)
       gollum-grit_adapter (~> 1.0)
       nokogiri (>= 1.6.1, < 2.0)
       rouge (~> 3.1)
-      sanitize (~> 2.1)
+      sanitize (~> 4.6.4)
       stringex (~> 2.6)
     gitlab-gollum-rugged_adapter (0.4.4.1)
       mime-types (>= 1.15)
@@ -514,6 +514,8 @@ GEM
     netrc (0.11.0)
     nokogiri (1.8.2)
       mini_portile2 (~> 2.3.0)
+    nokogumbo (1.5.0)
+      nokogiri
     numerizer (0.1.1)
     oauth (0.5.4)
     oauth2 (1.4.0)
@@ -804,8 +806,10 @@ GEM
       et-orbi (~> 1.0)
     rugged (0.27.2)
     safe_yaml (1.0.4)
-    sanitize (2.1.0)
+    sanitize (4.6.5)
+      crass (~> 1.0.2)
       nokogiri (>= 1.4.4)
+      nokogumbo (~> 1.4)
     sass (3.5.5)
       sass-listen (~> 4.0.0)
     sass-listen (4.0.0)
@@ -1151,7 +1155,7 @@ DEPENDENCIES
   ruby_parser (~> 3.8)
   rufus-scheduler (~> 3.4)
   rugged (~> 0.27)
-  sanitize (~> 2.0)
+  sanitize (~> 4.6.5)
   sass-rails (~> 5.0.6)
   scss_lint (~> 0.56.0)
   seed-fu (~> 2.3.7)
diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock
index 679318b9be5..52388f17c7c 100644
--- a/Gemfile.rails5.lock
+++ b/Gemfile.rails5.lock
@@ -298,13 +298,13 @@ GEM
       flowdock (~> 0.7)
       gitlab-grit (>= 2.4.1)
       multi_json
-    gitlab-gollum-lib (4.2.7.4)
+    gitlab-gollum-lib (4.2.7.5)
       gemojione (~> 3.2)
       github-markup (~> 1.6)
       gollum-grit_adapter (~> 1.0)
       nokogiri (>= 1.6.1, < 2.0)
       rouge (~> 3.1)
-      sanitize (~> 2.1)
+      sanitize (~> 4.6.4)
       stringex (~> 2.6)
     gitlab-gollum-rugged_adapter (0.4.4.1)
       mime-types (>= 1.15)
@@ -518,6 +518,8 @@ GEM
     nio4r (2.3.1)
     nokogiri (1.8.2)
       mini_portile2 (~> 2.3.0)
+    nokogumbo (1.5.0)
+      nokogiri
     numerizer (0.1.1)
     oauth (0.5.4)
     oauth2 (1.4.0)
@@ -813,8 +815,10 @@ GEM
       et-orbi (~> 1.0)
     rugged (0.27.1)
     safe_yaml (1.0.4)
-    sanitize (2.1.0)
+    sanitize (4.6.5)
+      crass (~> 1.0.2)
       nokogiri (>= 1.4.4)
+      nokogumbo (~> 1.4)
     sass (3.5.5)
       sass-listen (~> 4.0.0)
     sass-listen (4.0.0)
@@ -1162,7 +1166,7 @@ DEPENDENCIES
   ruby_parser (~> 3.8)
   rufus-scheduler (~> 3.4)
   rugged (~> 0.27)
-  sanitize (~> 2.0)
+  sanitize (~> 4.6.5)
   sass-rails (~> 5.0.6)
   scss_lint (~> 0.56.0)
   seed-fu (~> 2.3.7)
diff --git a/changelogs/unreleased/security-fj-bumping-sanitize-gem.yml b/changelogs/unreleased/security-fj-bumping-sanitize-gem.yml
new file mode 100644
index 00000000000..bec1033425d
--- /dev/null
+++ b/changelogs/unreleased/security-fj-bumping-sanitize-gem.yml
@@ -0,0 +1,5 @@
+---
+title: Update sanitize gem to 4.6.5 to fix HTML injection vulnerability
+merge_request:
+author:
+type: security
diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb
index 6786b9d07b6..afc2ca4e362 100644
--- a/lib/banzai/filter/sanitization_filter.rb
+++ b/lib/banzai/filter/sanitization_filter.rb
@@ -25,10 +25,11 @@ module Banzai
         # Only push these customizations once
         return if customized?(whitelist[:transformers])
 
-        # Allow table alignment; we whitelist specific style properties in a
+        # Allow table alignment; we whitelist specific text-align values in a
         # transformer below
         whitelist[:attributes]['th'] = %w(style)
         whitelist[:attributes]['td'] = %w(style)
+        whitelist[:css] = { properties: ['text-align'] }
 
         # Allow span elements
         whitelist[:elements].push('span')
diff --git a/spec/lib/banzai/filter/sanitization_filter_spec.rb b/spec/lib/banzai/filter/sanitization_filter_spec.rb
index 17a620ef603..d930c608b18 100644
--- a/spec/lib/banzai/filter/sanitization_filter_spec.rb
+++ b/spec/lib/banzai/filter/sanitization_filter_spec.rb
@@ -93,6 +93,16 @@ describe Banzai::Filter::SanitizationFilter do
       expect(doc.at_css('td')['style']).to eq 'text-align: center'
     end
 
+    it 'disallows `text-align` property in `style` attribute on other elements' do
+      html = <<~HTML
+        <div style="text-align: center">Text</div>
+      HTML
+
+      doc = filter(html)
+
+      expect(doc.at_css('div')['style']).to be_nil
+    end
+
     it 'allows `span` elements' do
       exp = act = %q{<span>Hello</span>}
       expect(filter(act).to_html).to eq exp
@@ -224,7 +234,7 @@ describe Banzai::Filter::SanitizationFilter do
 
       'protocol-based JS injection: spaces and entities' => {
         input:  '<a href=" &#14;  javascript:alert(\'XSS\');">foo</a>',
-        output: '<a href="">foo</a>'
+        output: '<a href>foo</a>'
       },
 
       'protocol whitespace' => {