From f86ddfd36538667cd0c484a62825569a36ef2a2c Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 12 Sep 2015 20:54:06 -0700
Subject: [PATCH] Render sanitized SVG images
Closes https://github.com/gitlabhq/gitlabhq/issues/9265
---
CHANGELOG | 1 +
Gemfile | 3 +++
Gemfile.lock | 1 +
app/helpers/blob_helper.rb | 12 +++++++++
app/views/projects/blob/_blob.html.haml | 5 +++-
features/project/source/browse_files.feature | 10 +++++++
features/steps/project/source/browse_files.rb | 17 ++++++++++++
spec/fixtures/logo_sample.svg | 27 +++++++++++++++++++
8 files changed, 75 insertions(+), 1 deletion(-)
create mode 100644 spec/fixtures/logo_sample.svg
diff --git a/CHANGELOG b/CHANGELOG
index 58efbe2db7ba2..d7d07cd1c4632 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -4,6 +4,7 @@ v 8.5.0 (unreleased)
- Ensure rake tasks that don't need a DB connection can be run without one
- Add "visibility" flag to GET /projects api endpoint
- Ignore binary files in code search to prevent Error 500 (Stan Hu)
+ - Render sanitized SVG images (Stan Hu)
- Upgrade gitlab_git to 7.2.23 to fix commit message mentions in first branch push
- New UI for pagination
- Don't prevent sign out when 2FA enforcement is enabled and user hasn't yet
diff --git a/Gemfile b/Gemfile
index a09d44f8bfdc4..c9d428a1798b8 100644
--- a/Gemfile
+++ b/Gemfile
@@ -179,6 +179,9 @@ gem "underscore-rails", "~> 1.8.0"
gem "sanitize", '~> 2.0'
gem 'babosa', '~> 1.0.2'
+# Sanitizes SVG input
+gem "loofah", "~> 2.0.3"
+
# Protect against bruteforcing
gem "rack-attack", '~> 4.3.1'
diff --git a/Gemfile.lock b/Gemfile.lock
index ec92964df25cf..b4f7587c41955 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -953,6 +953,7 @@ DEPENDENCIES
jquery-ui-rails (~> 5.0.0)
kaminari (~> 0.16.3)
letter_opener (~> 1.1.2)
+ loofah (~> 2.0.3)
mail_room (~> 0.6.1)
method_source (~> 0.8)
minitest (~> 5.7.0)
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index 694c03206bd6c..1696792792262 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -126,4 +126,16 @@ def blob_size(blob)
blob.size
end
end
+
+ def blob_svg?(blob)
+ blob.language && blob.language.name == 'SVG'
+ end
+
+ # SVGs can contain malicious JavaScript; only include whitelisted
+ # elements and attributes. Note that this whitelist is by no means complete
+ # and may omit some elements.
+ def sanitize_svg(blob)
+ blob.data = Loofah.scrub_fragment(blob.data, :strip).to_xml
+ blob
+ end
end
diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml
index 3d8d88834e2a2..2c5b8dc4356ab 100644
--- a/app/views/projects/blob/_blob.html.haml
+++ b/app/views/projects/blob/_blob.html.haml
@@ -35,7 +35,10 @@
- if blob.lfs_pointer?
= render "download", blob: blob
- elsif blob.text?
- = render "text", blob: blob
+ - if blob_svg?(blob)
+ = render "image", blob: sanitize_svg(blob)
+ - else
+ = render "text", blob: blob
- elsif blob.image?
= render "image", blob: blob
- else
diff --git a/features/project/source/browse_files.feature b/features/project/source/browse_files.feature
index a8c276b949ea5..1e09dbc4c8ffa 100644
--- a/features/project/source/browse_files.feature
+++ b/features/project/source/browse_files.feature
@@ -320,3 +320,13 @@ Feature: Project Source Browse Files
Then I should see download link and object size
And I should not see lfs pointer details
And I should see buttons for allowed commands
+
+ @javascript
+ Scenario: I preview an SVG file
+ Given I click on "Upload file" link in repo
+ And I upload a new SVG file
+ And I fill the upload file commit message
+ And I fill the new branch name
+ And I click on "Upload file"
+ Given I visit the SVG file
+ Then I can see the new rendered SVG image
diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb
index d08935aa1010a..13caddc44a49b 100644
--- a/features/steps/project/source/browse_files.rb
+++ b/features/steps/project/source/browse_files.rb
@@ -351,6 +351,19 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
expect(page).to have_content "You're not allowed to make changes to this project directly. A fork of this project has been created that you can make changes in, so you can submit a merge request."
end
+ # SVG files
+ step 'I upload a new SVG file' do
+ drop_in_dropzone test_svg_file
+ end
+
+ step 'I visit the SVG file' do
+ visit namespace_project_blob_path(@project.namespace, @project, 'new_branch_name/logo_sample.svg')
+ end
+
+ step 'I can see the new rendered SVG image' do
+ expect(find('.file-content')).to have_css('img')
+ end
+
private
def set_new_content
@@ -410,4 +423,8 @@ def test_text_file
def test_image_file
File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')
end
+
+ def test_svg_file
+ File.join(Rails.root, 'spec', 'fixtures', 'logo_sample.svg')
+ end
end
diff --git a/spec/fixtures/logo_sample.svg b/spec/fixtures/logo_sample.svg
new file mode 100644
index 0000000000000..883e7e6cf9254
--- /dev/null
+++ b/spec/fixtures/logo_sample.svg
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<svg width="210px" height="210px" viewBox="0 0 210 210" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:sketch="http://www.bohemiancoding.com/sketch/ns">
+ <!-- Generator: Sketch 3.3.2 (12043) - http://www.bohemiancoding.com/sketch -->
+ <title>Slice 1</title>
+ <desc>Created with Sketch.</desc>
+ <script>alert('FAIL')</script>
+ <defs></defs>
+ <g id="Page-1" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd" sketch:type="MSPage">
+ <g id="logo" sketch:type="MSLayerGroup" transform="translate(0.000000, 10.000000)">
+ <g id="Page-1" sketch:type="MSShapeGroup">
+ <g id="Fill-1-+-Group-24">
+ <g id="Group-24">
+ <g id="Group">
+ <path d="M105.0614,193.655 L105.0614,193.655 L143.7014,74.734 L66.4214,74.734 L105.0614,193.655 L105.0614,193.655 Z" id="Fill-4" fill="#E24329" class="tanuki-shape"></path>
+ <path d="M105.0614,193.6548 L66.4214,74.7338 L12.2684,74.7338 L105.0614,193.6548 L105.0614,193.6548 Z" id="Fill-8" fill="#FC6D26" class="tanuki-shape"></path>
+ <path d="M12.2685,74.7341 L12.2685,74.7341 L0.5265,110.8731 C-0.5445,114.1691 0.6285,117.7801 3.4325,119.8171 L105.0615,193.6551 L12.2685,74.7341 L12.2685,74.7341 Z" id="Fill-12" fill="#FCA326" class="tanuki-shape"></path>
+ <path d="M12.2685,74.7342 L66.4215,74.7342 L43.1485,3.1092 C41.9515,-0.5768 36.7375,-0.5758 35.5405,3.1092 L12.2685,74.7342 L12.2685,74.7342 Z" id="Fill-16" fill="#E24329" class="tanuki-shape"></path>
+ <path d="M105.0614,193.6548 L143.7014,74.7338 L197.8544,74.7338 L105.0614,193.6548 L105.0614,193.6548 Z" id="Fill-18" fill="#FC6D26" class="tanuki-shape"></path>
+ <path d="M197.8544,74.7341 L197.8544,74.7341 L209.5964,110.8731 C210.6674,114.1691 209.4944,117.7801 206.6904,119.8171 L105.0614,193.6551 L197.8544,74.7341 L197.8544,74.7341 Z" id="Fill-20" fill="#FCA326" class="tanuki-shape"></path>
+ <path d="M197.8544,74.7342 L143.7014,74.7342 L166.9744,3.1092 C168.1714,-0.5768 173.3854,-0.5758 174.5824,3.1092 L197.8544,74.7342 L197.8544,74.7342 Z" id="Fill-22" fill="#E24329" class="tanuki-shape"></path>
+ </g>
+ </g>
+ </g>
+ </g>
+ </g>
+ </g>
+</svg>
--
GitLab