diff --git a/app/controllers/projects/avatars_controller.rb b/app/controllers/projects/avatars_controller.rb index f7e6bb34443e1daa1ae263cdb1a6c6d4bc9f07b3..b64dbbd89ce9eb0d15f80716961c6d3b4b53e7d6 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 87b4d08da0e75aa9421a703c892e51655e53633b..d9723acb1d96ee8982577bed7ac498533d625119 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 7143a7448699fd9fafa49507b25f211cb5aaf584..7f63a2e2cb4f7938910c54efe5d89c113fec3af9 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