Bug#861870: gitlab: CVE-2017-8778

[Tomasz Buchert]

On 05/05/17 20:46, Tomasz Buchert wrote:
> On 05/05/17 06:19, Salvatore Bonaccorso wrote:
> > [...]
>
> Hi Salvatore,
> the fix for this issue seems to be here:
> https://gitlab.com/winniehell/gitlab-ce/commit/dd944bf14f4a0fd555db32d5833325fa459d9565
>
> I'll try to apply it to stretch's gitlab.
> Tomasz

Interestingly, the CVE has been fixed for unstable just an hour ago or so:
https://anonscm.debian.org/cgit/pkg-ruby-extras/gitlab.git/commit/?id=7241318db49ec356f31dac96345a4ff730d313f0

I've reapplied this for the stretch version and I attach the
debdiff. I'm going to request an unblock for this.

For some reason I couldn't push my branch to 
ssh://git.debian.org/git/pkg-ruby-extras/gitlab.git.
Probably I should become ruby-extras team member or something. For this reason 
I also attach
the commits from my branch.

Cheers,
Tomasz
diff -Nru gitlab-8.13.11+dfsg1/debian/changelog gitlab-8.13.11+dfsg1/debian/changelog
--- gitlab-8.13.11+dfsg1/debian/changelog	2017-04-21 12:32:25.0 +0200
+++ gitlab-8.13.11+dfsg1/debian/changelog	2017-05-05 21:23:50.0 +0200
@@ -1,3 +1,9 @@
+gitlab (8.13.11+dfsg1-4) testing-proposed-updates; urgency=medium
+
+  * Fix CVE-2017-8778
+
+ -- Tomasz Buchert   Fri, 05 May 2017 21:23:50 +0200
+
 gitlab (8.13.11+dfsg1-3) unstable; urgency=medium
 
   * Quote variable in test -n (Thanks to Benjamin Drung)
diff -Nru gitlab-8.13.11+dfsg1/debian/patches/cve-2017-8778.patch gitlab-8.13.11+dfsg1/debian/patches/cve-2017-8778.patch
--- gitlab-8.13.11+dfsg1/debian/patches/cve-2017-8778.patch	1970-01-01 01:00:00.0 +0100
+++ gitlab-8.13.11+dfsg1/debian/patches/cve-2017-8778.patch	2017-05-05 21:14:50.0 +0200
@@ -0,0 +1,99 @@
+From: Debian Ruby Extras Maintainers
+ 
+Date: Fri, 5 May 2017 21:00:42 +0200
+Subject: cve-2017-8778
+
+---
+ app/uploaders/file_uploader.rb  |  2 +-
+ app/uploaders/uploader_helper.rb|  8 
+ spec/controllers/uploads_controller_spec.rb | 22 ++
+ spec/factories/notes.rb |  6 +-
+ 4 files changed, 36 insertions(+), 2 deletions(-)
+
+diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
+index 3ac6030..407606a 100644
+--- a/app/uploaders/file_uploader.rb
 b/app/uploaders/file_uploader.rb
+@@ -36,7 +36,7 @@ class FileUploader < CarrierWave::Uploader::Base
+ escaped_filename = filename.gsub("]", "\\]")
+ 
+ markdown = "[#{escaped_filename}](#{self.secure_url})"
+-markdown.prepend("!") if image_or_video?
++markdown.prepend("!") if image_or_video? || dangerous?
+ 
+ {
+   alt:  filename,
+diff --git a/app/uploaders/uploader_helper.rb b/app/uploaders/uploader_helper.rb
+index b10ad71..5a9c0b7 100644
+--- a/app/uploaders/uploader_helper.rb
 b/app/uploaders/uploader_helper.rb
+@@ -7,11 +7,19 @@ module UploaderHelper
+   # on IE >= 9.
+   # http://archive.sublimevideo.info/20150912/docs.sublimevideo.net/troubleshooting.html
+   VIDEO_EXT = %w[mp4 m4v mov webm ogv]
++  # These extension types can contain dangerous code and should only be embedded inline with
++  # proper filtering. They should always be tagged as "Content-Disposition: attachment", not "inline".
++  DANGEROUS_EXT = %w[svg]
++
+ 
+   def image?
+ extension_match?(IMAGE_EXT)
+   end
+ 
++  def dangerous?
++extension_match?(DANGEROUS_EXT)
++  end
++
+   def video?
+ extension_match?(VIDEO_EXT)
+   end
+diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
+index 69124ab..8ea9c71 100644
+--- a/spec/controllers/uploads_controller_spec.rb
 b/spec/controllers/uploads_controller_spec.rb
+@@ -4,6 +4,28 @@ describe UploadsController do
+   let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) }
+ 
+   describe "GET show" do
++context 'Content-Disposition security measures' do
++  let(:project) { create(:empty_project, :public) }
++
++  context 'for PNG files' do
++it 'returns Content-Disposition: inline' do
++  note = create(:note, :with_attachment, project: project)
++  get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.png'
++
++  expect(response['Content-Disposition']).to start_with('inline;')
++end
++  end
++
++  context 'for SVG files' do
++it 'returns Content-Disposition: attachment' do
++  note = create(:note, :with_svg_attachment, project: project)
++  get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.svg'
++
++  expect(response['Content-Disposition']).to start_with('attachment;')
++end
++  end
++end
++
+ context "when viewing a user avatar" do
+   context "when signed in" do
+ before do
+diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb
+index a10ba62..b60b9f6 100644
+--- 

Bug#861870: gitlab: CVE-2017-8778

[Tomasz Buchert]

On 05/05/17 06:19, Salvatore Bonaccorso wrote:
> [...]

Hi Salvatore,
the fix for this issue seems to be here:
https://gitlab.com/winniehell/gitlab-ce/commit/dd944bf14f4a0fd555db32d5833325fa459d9565

I'll try to apply it to stretch's gitlab.
Tomasz


signature.asc
Description: PGP signature

Bug#861870: gitlab: CVE-2017-8778

[Salvatore Bonaccorso]

Source: gitlab
Version: 8.13.11+dfsg1-3
Severity: grave
Tags: upstream security
Forwarded: https://gitlab.com/gitlab-org/gitlab-ce/issues/27471

Hi,

the following vulnerability was published for gitlab. Please note I
was not able to verfy that affects back 8.13.11, and the merge request
has restricted access. Can you confirm 8.13.11+dfsg1-3 is affected as
well?

CVE-2017-8778[0]:
| GitLab before 8.14.9, 8.15.x before 8.15.6, and 8.16.x before 8.16.5
| has XSS via a SCRIPT element in an issue attachment or avatar that is
| an SVG document.

If you fix the vulnerability please also make sure to include the
CVE (Common Vulnerabilities & Exposures) id in your changelog entry.

For further information see:

[0] https://security-tracker.debian.org/tracker/CVE-2017-8778
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-8778

Please adjust the affected versions in the BTS as needed.

Regards,
Salvatore