diff options
author | Jacob Vosmaer <contact@jacobvosmaer.nl> | 2016-02-24 13:53:30 +0300 |
---|---|---|
committer | Jacob Vosmaer <contact@jacobvosmaer.nl> | 2016-02-24 13:53:30 +0300 |
commit | cf2c5396e014e54db7a3183380a8ed2b77b2e6e1 (patch) | |
tree | 1dbf75efad1006ff2aff91562e573d29455ec457 /app | |
parent | bd71438d6accb61a33b520177aeb92a3614eedb5 (diff) |
Explain why we mangle blob content types
Diffstat (limited to 'app')
-rw-r--r-- | app/controllers/projects/avatars_controller.rb | 4 | ||||
-rw-r--r-- | app/controllers/projects/raw_controller.rb | 13 | ||||
-rw-r--r-- | 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 |