From cf2c5396e014e54db7a3183380a8ed2b77b2e6e1 Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <contact@jacobvosmaer.nl>
Date: Wed, 24 Feb 2016 11:53:30 +0100
Subject: [PATCH] Explain why we mangle blob content types
---
app/controllers/projects/avatars_controller.rb | 4 +++-
app/controllers/projects/raw_controller.rb | 13 ++-----------
app/helpers/blob_helper.rb | 18 ++++++++++++++++++
3 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/app/controllers/projects/avatars_controller.rb b/app/controllers/projects/avatars_controller.rb
index f7e6bb34443..b64dbbd89ce 100644
--- a/app/controllers/projects/avatars_controller.rb
+++ b/app/controllers/projects/avatars_controller.rb
@@ -1,4 +1,6 @@
class Projects::AvatarsController < Projects::ApplicationController
+ include BlobHelper
+
before_action :project
def show
@@ -7,7 +9,7 @@ class Projects::AvatarsController < Projects::ApplicationController
headers['X-Content-Type-Options'] = 'nosniff'
headers.store(*Gitlab::Workhorse.send_git_blob(@repository, @blob))
headers['Content-Disposition'] = 'inline'
- headers['Content-Type'] = @blob.content_type
+ headers['Content-Type'] = safe_content_type(@blob)
head :ok # 'render nothing: true' messes up the Content-Type
else
render_404
diff --git a/app/controllers/projects/raw_controller.rb b/app/controllers/projects/raw_controller.rb
index 87b4d08da0e..d9723acb1d9 100644
--- a/app/controllers/projects/raw_controller.rb
+++ b/app/controllers/projects/raw_controller.rb
@@ -1,6 +1,7 @@
# Controller for viewing a file's raw
class Projects::RawController < Projects::ApplicationController
include ExtractsPath
+ include BlobHelper
before_action :require_non_empty_project
before_action :assign_ref_vars
@@ -17,7 +18,7 @@ class Projects::RawController < Projects::ApplicationController
else
headers.store(*Gitlab::Workhorse.send_git_blob(@repository, @blob))
headers['Content-Disposition'] = 'inline'
- headers['Content-Type'] = get_blob_type
+ headers['Content-Type'] = safe_content_type(@blob)
head :ok # 'render nothing: true' messes up the Content-Type
end
else
@@ -27,16 +28,6 @@ class Projects::RawController < Projects::ApplicationController
private
- def get_blob_type
- if @blob.text?
- 'text/plain; charset=utf-8'
- elsif @blob.image?
- @blob.content_type
- else
- 'application/octet-stream'
- end
- end
-
def send_lfs_object
lfs_object = find_lfs_object
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index 7143a744869..7f63a2e2cb4 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -134,4 +134,22 @@ module BlobHelper
blob.data = Loofah.scrub_fragment(blob.data, :strip).to_xml
blob
end
+
+ # If we blindly set the 'real' content type when serving a Git blob we
+ # are enabling XSS attacks. An attacker could upload e.g. a Javascript
+ # file to a Git repository, trick the browser of a victim into
+ # downloading the blob, and then the 'application/javascript' content
+ # type would tell the browser to execute the attacker's Javascript. By
+ # overriding the content type and setting it to 'text/plain' (in the
+ # example of Javascript) we tell the browser of the victim not to
+ # execute untrusted data.
+ def safe_content_type(blob)
+ if blob.text?
+ 'text/plain; charset=utf-8'
+ elsif blob.image?
+ blob.content_type
+ else
+ 'application/octet-stream'
+ end
+ end
end
--
GitLab