Skip to content
Snippets Groups Projects
Commit cf2c5396 authored by Jacob Vosmaer's avatar Jacob Vosmaer
Browse files

Explain why we mangle blob content types

parent bd71438d
No related branches found
No related tags found
No related merge requests found
class Projects::AvatarsController < Projects::ApplicationController class Projects::AvatarsController < Projects::ApplicationController
include BlobHelper
before_action :project before_action :project
   
def show def show
Loading
@@ -7,7 +9,7 @@ class Projects::AvatarsController < Projects::ApplicationController
Loading
@@ -7,7 +9,7 @@ class Projects::AvatarsController < Projects::ApplicationController
headers['X-Content-Type-Options'] = 'nosniff' headers['X-Content-Type-Options'] = 'nosniff'
headers.store(*Gitlab::Workhorse.send_git_blob(@repository, @blob)) headers.store(*Gitlab::Workhorse.send_git_blob(@repository, @blob))
headers['Content-Disposition'] = 'inline' 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 head :ok # 'render nothing: true' messes up the Content-Type
else else
render_404 render_404
Loading
Loading
# Controller for viewing a file's raw # Controller for viewing a file's raw
class Projects::RawController < Projects::ApplicationController class Projects::RawController < Projects::ApplicationController
include ExtractsPath include ExtractsPath
include BlobHelper
   
before_action :require_non_empty_project before_action :require_non_empty_project
before_action :assign_ref_vars before_action :assign_ref_vars
Loading
@@ -17,7 +18,7 @@ class Projects::RawController < Projects::ApplicationController
Loading
@@ -17,7 +18,7 @@ class Projects::RawController < Projects::ApplicationController
else else
headers.store(*Gitlab::Workhorse.send_git_blob(@repository, @blob)) headers.store(*Gitlab::Workhorse.send_git_blob(@repository, @blob))
headers['Content-Disposition'] = 'inline' 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 head :ok # 'render nothing: true' messes up the Content-Type
end end
else else
Loading
@@ -27,16 +28,6 @@ class Projects::RawController < Projects::ApplicationController
Loading
@@ -27,16 +28,6 @@ class Projects::RawController < Projects::ApplicationController
   
private 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 def send_lfs_object
lfs_object = find_lfs_object lfs_object = find_lfs_object
   
Loading
Loading
Loading
@@ -134,4 +134,22 @@ module BlobHelper
Loading
@@ -134,4 +134,22 @@ module BlobHelper
blob.data = Loofah.scrub_fragment(blob.data, :strip).to_xml blob.data = Loofah.scrub_fragment(blob.data, :strip).to_xml
blob blob
end 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 end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment