diff options
| -rw-r--r-- | activestorage/CHANGELOG.md | 8 | ||||
| -rw-r--r-- | activestorage/app/models/active_storage/blob.rb | 9 | ||||
| -rw-r--r-- | activestorage/lib/active_storage.rb | 1 | ||||
| -rw-r--r-- | activestorage/lib/active_storage/engine.rb | 11 | ||||
| -rw-r--r-- | activestorage/test/models/blob_test.rb | 9 |
5 files changed, 36 insertions, 2 deletions
diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index c5171e7490..9ce177b462 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,11 @@ +* Force `:attachment` disposition for specific, configurable content types. + This mitigates possible security issues such as XSS or phishing when + serving them inline. A list of such content types is included by default, + and can be configured via `content_types_to_serve_as_binary`. + + *Rosa Gutierrez* + + ## Rails 5.2.0.beta2 (November 28, 2017) ## * Fix the gem adding the migrations files to the package. diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 3b48ee72af..8fb53fa787 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -201,8 +201,8 @@ class ActiveStorage::Blob < ActiveRecord::Base # with users. Instead, the +service_url+ should only be exposed as a redirect from a stable, possibly authenticated URL. # Hiding the +service_url+ behind a redirect also gives you the power to change services without updating all URLs. And # it allows permanent URLs that redirect to the +service_url+ to be cached in the view. - def service_url(expires_in: service.url_expires_in, disposition: "inline") - service.url key, expires_in: expires_in, disposition: disposition, filename: filename, content_type: content_type + def service_url(expires_in: service.url_expires_in, disposition: :inline) + service.url key, expires_in: expires_in, disposition: forcibly_serve_as_binary? ? :attachment : disposition, filename: filename, content_type: content_type end # Returns a URL that can be used to directly upload a file for this blob on the service. This URL is intended to be @@ -325,4 +325,9 @@ class ActiveStorage::Blob < ActiveRecord::Base def analyzer_class ActiveStorage.analyzers.detect { |klass| klass.accept?(self) } || ActiveStorage::Analyzer::NullAnalyzer end + + + def forcibly_serve_as_binary? + ActiveStorage.content_types_to_serve_as_binary.include?(content_type) + end end diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index 01fd0f4415..3bd8ef664f 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -43,4 +43,5 @@ module ActiveStorage mattr_accessor :analyzers, default: [] mattr_accessor :paths, default: {} mattr_accessor :variable_content_types, default: [] + mattr_accessor :content_types_to_serve_as_binary, default: [] end diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index d5c71670ff..1f15ac2d2d 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -18,6 +18,16 @@ module ActiveStorage config.active_storage.analyzers = [ ActiveStorage::Analyzer::ImageAnalyzer, ActiveStorage::Analyzer::VideoAnalyzer ] config.active_storage.paths = ActiveSupport::OrderedOptions.new config.active_storage.variable_content_types = [ "image/png", "image/gif", "image/jpg", "image/jpeg", "image/vnd.adobe.photoshop" ] + config.active_storage.content_types_to_serve_as_binary = [ + "text/html", + "text/javascript", + "image/svg+xml", + "application/postscript", + "application/x-shockwave-flash", + "text/xml", + "application/xml", + "application/xhtml+xml" + ] config.eager_load_namespaces << ActiveStorage @@ -29,6 +39,7 @@ module ActiveStorage ActiveStorage.analyzers = app.config.active_storage.analyzers || [] ActiveStorage.paths = app.config.active_storage.paths || {} ActiveStorage.variable_content_types = app.config.active_storage.variable_content_types || [] + ActiveStorage.content_types_to_serve_as_binary = app.config.active_storage.content_types_to_serve_as_binary || [] end end diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index f94e65ed77..fa8b935da0 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -41,6 +41,15 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase end end + test "urls force attachment as content disposition for content types served as binary" do + blob = create_blob(content_type: "text/html") + + freeze_time do + assert_equal expected_url_for(blob, disposition: :attachment), blob.service_url + assert_equal expected_url_for(blob, disposition: :attachment), blob.service_url(disposition: :inline) + end + end + test "purge deletes file from external service" do blob = create_blob |