Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

Commit

Permalink
Remove the automatic loading of URI Adapters
Browse files Browse the repository at this point in the history
Remove the URI adapters. Few people use them by default and they can
allow insight into the internal networks of the server. If you want to
enable them, add (for example) `Paperclip.DataUriAdapter.register` to
your `config/initializers/paperclip.rb` file.

This is related to CVE-2017-0889.
  • Loading branch information
Jon Yurek authored and mike-burns committed Jan 23, 2018
1 parent c794f6d commit 7b9bac8
Show file tree
Hide file tree
Showing 21 changed files with 136 additions and 49 deletions.
1 change: 1 addition & 0 deletions Gemfile
Expand Up @@ -12,4 +12,5 @@ group :development, :test do
gem 'mime-types'
gem 'builder'
gem 'rubocop', require: false
gem 'rspec'
end
24 changes: 23 additions & 1 deletion README.md
Expand Up @@ -17,7 +17,6 @@ https://github.com/thoughtbot/paperclip/releases
<!-- START doctoc generated TOC please keep comment here to allow auto update -->
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->


- [Requirements](#requirements)
- [Ruby and Rails](#ruby-and-rails)
- [Image Processor](#image-processor)
Expand All @@ -43,6 +42,7 @@ https://github.com/thoughtbot/paperclip/releases
- [Vintage Syntax](#vintage-syntax)
- [Storage](#storage)
- [Understanding Storage](#understanding-storage)
- [IO Adapters](#io-adapters)
- [Post Processing](#post-processing)
- [Custom Attachment Processors](#custom-attachment-processors)
- [Events](#events)
Expand Down Expand Up @@ -608,6 +608,28 @@ variables.

---

IO Adapters
-----------

When a file is uploaded or attached, it can be in one of a few different input
forms, from Rails' UploadedFile object to a StringIO to a Tempfile or even a
simple String that is a URL that points to an image.

Paperclip will accept, by default, many of these sources. It also is capable of
handling even more with a little configuration. The IO Adapters that handle
images from non-local sources are not enabled by default. They can be enabled by
adding a line similar to the following into `config/initializers/paperclip.rb`:

```ruby
Paperclip::DataUriAdapter.regsiter
```

It's best to only enable a remote-loading adapter if you need it. Otherwise
there's a chance that someone can gain insight into your internal network
structure using it as a vector.

---

Post Processing
---------------

Expand Down
2 changes: 1 addition & 1 deletion Rakefile
Expand Up @@ -9,7 +9,7 @@ task :default => [:clean, :all]
desc 'Test the paperclip plugin under all supported Rails versions.'
task :all do |t|
if ENV['BUNDLE_GEMFILE']
exec('rake spec cucumber')
exec('rake spec && cucumber')
else
exec("rm -f gemfiles/*.lock")
Rake::Task["appraisal:gemfiles"].execute
Expand Down
15 changes: 14 additions & 1 deletion features/step_definitions/rails_steps.rb
Expand Up @@ -22,6 +22,7 @@
gem "rubysl", :platform => :rbx
"""
And I remove turbolinks
And I comment out lines that contain "action_mailer" in "config/environments/*.rb"
And I empty the application.js file
And I configure the application to use "paperclip" from this project
}
Expand Down Expand Up @@ -49,6 +50,16 @@
end
end

Given /^I comment out lines that contain "([^"]+)" in "([^"]+)"$/ do |contains, glob|
cd(".") do
Dir.glob(glob).each do |file|
transform_file(file) do |content|
content.gsub(/^(.*?#{contains}.*?)$/) { |line| "# #{line}" }
end
end
end
end

Given /^I attach :attachment$/ do
attach_attachment("attachment")
end
Expand Down Expand Up @@ -138,8 +149,10 @@ def attach_attachment(name, definition = nil)

Given /^I start the rails application$/ do
cd(".") do
require "rails"
require "./config/environment"
require "capybara/rails"
require "capybara"
Capybara.app = Rails.application
end
end

Expand Down
1 change: 1 addition & 0 deletions features/support/env.rb
Expand Up @@ -7,5 +7,6 @@
World(RSpec::Matchers)

Before do
aruba.config.command_launcher = ENV.fetch("DEBUG", nil) ? :debug : :spawn
@aruba_timeout_seconds = 120
end
1 change: 1 addition & 0 deletions gemfiles/4.2.gemfile
Expand Up @@ -11,6 +11,7 @@ group :development, :test do
gem "mime-types"
gem "builder"
gem "rubocop", :require => false
gem "rspec"
end

gemspec :path => "../"
1 change: 1 addition & 0 deletions gemfiles/5.0.gemfile
Expand Up @@ -11,6 +11,7 @@ group :development, :test do
gem "mime-types"
gem "builder"
gem "rubocop", :require => false
gem "rspec"
end

gemspec :path => "../"
10 changes: 7 additions & 3 deletions lib/paperclip/io_adapters/attachment_adapter.rb
@@ -1,5 +1,11 @@
module Paperclip
class AttachmentAdapter < AbstractAdapter
def self.register
Paperclip.io_adapters.register self do |target|
Paperclip::Attachment === target || Paperclip::Style === target
end
end

def initialize(target, options = {})
super
@target, @style = case target
Expand Down Expand Up @@ -32,6 +38,4 @@ def copy_to_tempfile(source)
end
end

Paperclip.io_adapters.register Paperclip::AttachmentAdapter do |target|
Paperclip::Attachment === target || Paperclip::Style === target
end
Paperclip::AttachmentAdapter.register
12 changes: 6 additions & 6 deletions lib/paperclip/io_adapters/data_uri_adapter.rb
@@ -1,5 +1,10 @@
module Paperclip
class DataUriAdapter < StringioAdapter
def self.register
Paperclip.io_adapters.register self do |target|
String === target && target =~ Paperclip::DataUriAdapter::REGEXP
end
end

REGEXP = /\Adata:([-\w]+\/[-\w\+\.]+)?;base64,(.*)/m

Expand All @@ -11,12 +16,7 @@ def initialize(target_uri, options = {})

def extract_target(uri)
data_uri_parts = uri.match(REGEXP) || []
StringIO.new(Base64.decode64(data_uri_parts[2] || ''))
StringIO.new(Base64.decode64(data_uri_parts[2] || ""))
end

end
end

Paperclip.io_adapters.register Paperclip::DataUriAdapter do |target|
String === target && target =~ Paperclip::DataUriAdapter::REGEXP
end
10 changes: 7 additions & 3 deletions lib/paperclip/io_adapters/empty_string_adapter.rb
@@ -1,5 +1,11 @@
module Paperclip
class EmptyStringAdapter < AbstractAdapter
def self.register
Paperclip.io_adapters.register self do |target|
target.is_a?(String) && target.empty?
end
end

def nil?
false
end
Expand All @@ -10,6 +16,4 @@ def assignment?
end
end

Paperclip.io_adapters.register Paperclip::EmptyStringAdapter do |target|
target.is_a?(String) && target.empty?
end
Paperclip::EmptyStringAdapter.register
14 changes: 10 additions & 4 deletions lib/paperclip/io_adapters/file_adapter.rb
@@ -1,5 +1,11 @@
module Paperclip
class FileAdapter < AbstractAdapter
def self.register
Paperclip.io_adapters.register self do |target|
File === target || ::Tempfile === target
end
end

def initialize(target, options = {})
super
cache_current_values
Expand All @@ -8,7 +14,9 @@ def initialize(target, options = {})
private

def cache_current_values
self.original_filename = @target.original_filename if @target.respond_to?(:original_filename)
if @target.respond_to?(:original_filename)
self.original_filename = @target.original_filename
end
self.original_filename ||= File.basename(@target.path)
@tempfile = copy_to_tempfile(@target)
@content_type = ContentTypeDetector.new(@target.path).detect
Expand All @@ -17,6 +25,4 @@ def cache_current_values
end
end

Paperclip.io_adapters.register Paperclip::FileAdapter do |target|
File === target || Tempfile === target
end
Paperclip::FileAdapter.register
10 changes: 5 additions & 5 deletions lib/paperclip/io_adapters/http_url_proxy_adapter.rb
@@ -1,15 +1,15 @@
module Paperclip
class HttpUrlProxyAdapter < UriAdapter
def self.register
Paperclip.io_adapters.register self do |target|
String === target && target =~ Paperclip::HttpUrlProxyAdapter::REGEXP
end
end

REGEXP = /\Ahttps?:\/\//

def initialize(target, options = {})
super(URI(URI.escape(target)), options)
end

end
end

Paperclip.io_adapters.register Paperclip::HttpUrlProxyAdapter do |target|
String === target && target =~ Paperclip::HttpUrlProxyAdapter::REGEXP
end
11 changes: 7 additions & 4 deletions lib/paperclip/io_adapters/identity_adapter.rb
@@ -1,5 +1,11 @@
module Paperclip
class IdentityAdapter < AbstractAdapter
def self.register
Paperclip.io_adapters.register Paperclip::IdentityAdapter.new do |target|
Paperclip.io_adapters.registered?(target)
end
end

def initialize
end

Expand All @@ -9,7 +15,4 @@ def new(target, _)
end
end

Paperclip.io_adapters.register Paperclip::IdentityAdapter.new do |target|
Paperclip.io_adapters.registered?(target)
end

Paperclip::IdentityAdapter.register
13 changes: 8 additions & 5 deletions lib/paperclip/io_adapters/nil_adapter.rb
@@ -1,8 +1,13 @@
module Paperclip
class NilAdapter < AbstractAdapter
def initialize(_target, _options = {})
def self.register
Paperclip.io_adapters.register self do |target|
target.nil? || ((Paperclip::Attachment === target) && !target.present?)
end
end

def initialize(_target, _options = {}); end

def original_filename
""
end
Expand All @@ -19,7 +24,7 @@ def nil?
true
end

def read(*args)
def read(*_args)
nil
end

Expand All @@ -29,6 +34,4 @@ def eof?
end
end

Paperclip.io_adapters.register Paperclip::NilAdapter do |target|
target.nil? || ( (Paperclip::Attachment === target) && !target.present? )
end
Paperclip::NilAdapter.register
4 changes: 4 additions & 0 deletions lib/paperclip/io_adapters/registry.rb
Expand Up @@ -12,6 +12,10 @@ def register(handler_class, &block)
@registered_handlers << [block, handler_class]
end

def unregister(handler_class)
@registered_handlers.reject! {|_, klass| klass == handler_class }
end

def handler_for(target)
@registered_handlers.each do |tester, handler|
return handler if tester.call(target)
Expand Down
11 changes: 7 additions & 4 deletions lib/paperclip/io_adapters/stringio_adapter.rb
@@ -1,5 +1,11 @@
module Paperclip
class StringioAdapter < AbstractAdapter
def self.register
Paperclip.io_adapters.register self do |target|
StringIO === target
end
end

def initialize(target, options = {})
super
cache_current_values
Expand All @@ -24,10 +30,7 @@ def copy_to_tempfile(source)
destination.rewind
destination
end

end
end

Paperclip.io_adapters.register Paperclip::StringioAdapter do |target|
StringIO === target
end
Paperclip::StringioAdapter.register
10 changes: 7 additions & 3 deletions lib/paperclip/io_adapters/uploaded_file_adapter.rb
@@ -1,5 +1,11 @@
module Paperclip
class UploadedFileAdapter < AbstractAdapter
def self.register
Paperclip.io_adapters.register self do |target|
target.class.name.include?("UploadedFile")
end
end

def initialize(target, options = {})
super
cache_current_values
Expand Down Expand Up @@ -37,6 +43,4 @@ def determine_content_type
end
end

Paperclip.io_adapters.register Paperclip::UploadedFileAdapter do |target|
target.class.name.include?("UploadedFile")
end
Paperclip::UploadedFileAdapter.register
19 changes: 10 additions & 9 deletions lib/paperclip/io_adapters/uri_adapter.rb
@@ -1,9 +1,15 @@
require 'open-uri'
require "open-uri"

module Paperclip
class UriAdapter < AbstractAdapter
attr_writer :content_type

def self.register
Paperclip.io_adapters.register self do |target|
target.is_a?(URI)
end
end

def initialize(target, options = {})
super
@content = download_content
Expand All @@ -28,9 +34,8 @@ def content_type_from_content
end

def filename_from_content_disposition
if @content.meta.has_key?("content-disposition")
matches = @content.meta["content-disposition"].
match(/filename="([^"]*)"/)
if @content.meta.key?("content-disposition")
matches = @content.meta["content-disposition"].match(/filename="([^"]*)"/)
matches[1] if matches
end
end
Expand All @@ -50,7 +55,7 @@ def download_content
end

def copy_to_tempfile(src)
while data = src.read(16*1024)
while data = src.read(16 * 1024)
destination.write(data)
end
src.close
Expand All @@ -59,7 +64,3 @@ def copy_to_tempfile(src)
end
end
end

Paperclip.io_adapters.register Paperclip::UriAdapter do |target|
target.kind_of?(URI)
end

0 comments on commit 7b9bac8

Please sign in to comment.