Commit 263a4981 by Jon Yurek

Don't spoof-check files with no extensions

There's nothing they can be spoofing, and there's little danger of a
webserver accidentally sending a dangerous Content-Type with a file that
has no extension. If the browser chooses to interpret it in a dangerous
manner, no server-side library on earth can change that.

This isn't specifically related to RSpec, but is built off the branch
and is part of an effort to get the CI build green.
parent 743e2e00
......@@ -101,6 +101,14 @@ module Paperclip
File.extname(attachment.original_filename).gsub(/\A\.+/, "")
end
# Returns the dot+extension of the file. e.g. ".jpg" for "file.jpg"
# If the style has a format defined, it will return the format instead
# of the actual extension. If the extension is empty, no dot is added.
def dotextension attachment, style_name
ext = extension(attachment, style_name)
ext.empty? ? "" : ".#{ext}"
end
# Returns an extension based on the content type. e.g. "jpeg" for
# "image/jpeg". If the style has a specified format, it will override the
# content-type detection.
......
......@@ -13,7 +13,7 @@ module Paperclip
def cache_current_values
@content_type = ContentTypeDetector.new(@tempfile.path).detect
original_filename = @target.original_filename if @target.respond_to?(:original_filename)
original_filename ||= "data.#{extension_for(@content_type)}"
original_filename ||= "data"
self.original_filename = original_filename.strip
@size = @target.size
end
......@@ -26,11 +26,6 @@ module Paperclip
destination
end
def extension_for(content_type)
type = MIME::Types[content_type].first
type && type.extensions.first
end
end
end
......
......@@ -10,7 +10,7 @@ module Paperclip
end
def spoofed?
if @name.present? && media_type_mismatch? && mapping_override_mismatch?
if has_name? && has_extension? && media_type_mismatch? && mapping_override_mismatch?
Paperclip.log("Content Type Spoof: Filename #{File.basename(@name)} (#{supplied_file_content_types}), content type discovered from file command: #{calculated_content_type}. See documentation to allow this combination.")
true
end
......@@ -18,6 +18,14 @@ module Paperclip
private
def has_name?
@name.present?
end
def has_extension?
File.extname(@name).present?
end
def media_type_mismatch?
! supplied_file_media_types.include?(calculated_media_type)
end
......
......@@ -337,7 +337,11 @@ describe Paperclip::Attachment do
context "An attachment with :hash interpolations" do
before do
@file = StringIO.new("...\n")
@file = File.open(fixture_file("5k.png"))
end
after do
@file.close
end
it "raises if no secret is provided" do
......@@ -360,15 +364,15 @@ describe Paperclip::Attachment do
end
it "results in the correct interpolation" do
assert_equal "dummies/avatars/original/data.txt",
assert_equal "dummies/avatars/original/5k.png",
@attachment.send(:interpolate, @attachment.options[:hash_data])
assert_equal "dummies/avatars/thumb/data.txt",
assert_equal "dummies/avatars/thumb/5k.png",
@attachment.send(:interpolate, @attachment.options[:hash_data], :thumb)
end
it "results in a correct hash" do
assert_equal "e1079a5c34ddbd197ebd0280d07952d98a57fb30", @attachment.path
assert_equal "d740189bd3e49ef226fab84c8d45f7ae4126d043", @attachment.path(:thumb)
assert_equal "0a59e9142bba11576de1d353d8747b1acad5ad34", @attachment.path
assert_equal "b39a062c1e62e85a6c785ed00cf3bebf5f850e2b", @attachment.path(:thumb)
end
end
end
......
......@@ -71,6 +71,20 @@ describe Paperclip::Interpolations do
assert_equal "jpe", interpolations.content_type_extension(attachment, :style)
end
it "returns the extension of the file with a dot" do
attachment = mock
attachment.expects(:original_filename).returns("one.jpg")
attachment.expects(:styles).returns({})
assert_equal ".jpg", Paperclip::Interpolations.dotextension(attachment, :style)
end
it "returns the extension of the file without a dot if the extension is empty" do
attachment = mock
attachment.expects(:original_filename).returns("one")
attachment.expects(:styles).returns({})
assert_equal "", Paperclip::Interpolations.dotextension(attachment, :style)
end
it "returns the latter half of the content type of the extension if no match found" do
attachment = mock
attachment.expects(:content_type).at_least_once().returns('not/found')
......
......@@ -18,8 +18,8 @@ describe Paperclip::DataUriAdapter do
@subject = Paperclip.io_adapters.for(@contents)
end
it "returns a file name based on the content type" do
assert_equal "data.png", @subject.original_filename
it "returns a nondescript file name" do
assert_equal "data", @subject.original_filename
end
it "returns a content type" do
......
......@@ -9,7 +9,7 @@ describe Paperclip::StringioAdapter do
end
it "returns a file name" do
assert_equal "data.txt", @subject.original_filename
assert_equal "data", @subject.original_filename
end
it "returns a content type" do
......
......@@ -21,6 +21,11 @@ describe Paperclip::MediaTypeSpoofDetector do
assert ! Paperclip::MediaTypeSpoofDetector.using(file, "").spoofed?
end
it 'does not reject a file that does have an extension' do
file = File.open(fixture_file("empty.html"))
assert ! Paperclip::MediaTypeSpoofDetector.using(file, "data").spoofed?
end
it 'does not reject when the supplied file is an IOAdapter' do
adapter = Paperclip.io_adapters.for(File.new(fixture_file("5k.png")))
assert ! Paperclip::MediaTypeSpoofDetector.using(adapter, adapter.original_filename).spoofed?
......
......@@ -130,7 +130,7 @@ describe Paperclip::Storage::Fog do
fog_credentials: @credentials,
fog_host: nil,
fog_file: {cache_control: 1234},
path: ":attachment/:basename.:extension",
path: ":attachment/:basename:dotextension",
storage: :fog
}
......@@ -227,7 +227,7 @@ describe Paperclip::Storage::Fog do
end
it "provides a public url" do
assert @dummy.avatar.url =~ /^http:\/\/example\.com\/avatars\/data\.txt\?\d*$/
expect(@dummy.avatar.url).to match(/^http:\/\/example\.com\/avatars\/data\?\d*$/)
end
end
......@@ -237,7 +237,7 @@ describe Paperclip::Storage::Fog do
fog_directory: @fog_directory,
fog_credentials: @credentials,
fog_host: 'http://img%d.example.com',
path: ":attachment/:basename.:extension",
path: ":attachment/:basename:dotextension",
storage: :fog
)
@dummy = Dummy.new
......@@ -246,7 +246,7 @@ describe Paperclip::Storage::Fog do
end
it "provides a public url" do
assert @dummy.avatar.url =~ /^http:\/\/img[0123]\.example\.com\/avatars\/data\.txt\?\d*$/
expect(@dummy.avatar.url).to match(/^http:\/\/img[0123]\.example\.com\/avatars\/data\?\d*$/)
end
end
......
......@@ -101,14 +101,14 @@ describe Paperclip::Storage::S3 do
rebuild_model storage: :s3,
s3_credentials: {},
bucket: "bucket",
path: ":attachment/:basename.:extension",
path: ":attachment/:basename:dotextension",
url: ":s3_path_url"
@dummy = Dummy.new
@dummy.avatar = stringy_file
end
it "returns a url based on an S3 path" do
assert_match %r{^http://s3.amazonaws.com/bucket/avatars/data.txt}, @dummy.avatar.url
assert_match %r{^http://s3.amazonaws.com/bucket/avatars/data[^\.]}, @dummy.avatar.url
end
it "uses the correct bucket" do
......@@ -116,7 +116,7 @@ describe Paperclip::Storage::S3 do
end
it "uses the correct key" do
assert_equal "avatars/data.txt", @dummy.avatar.s3_object.key
assert_equal "avatars/data", @dummy.avatar.s3_object.key
end
end
......@@ -142,13 +142,13 @@ describe Paperclip::Storage::S3 do
s3_credentials: {},
s3_protocol: 'https',
bucket: "bucket",
path: ":attachment/:basename.:extension"
path: ":attachment/:basename:dotextension"
@dummy = Dummy.new
@dummy.avatar = stringy_file
end
it "returns a url based on an S3 path" do
assert_match %r{^https://s3.amazonaws.com/bucket/avatars/data.txt}, @dummy.avatar.url
assert_match %r{^https://s3.amazonaws.com/bucket/avatars/data[^\.]}, @dummy.avatar.url
end
end
......@@ -158,13 +158,13 @@ describe Paperclip::Storage::S3 do
s3_credentials: {},
s3_protocol: :https,
bucket: "bucket",
path: ":attachment/:basename.:extension"
path: ":attachment/:basename:dotextension"
@dummy = Dummy.new
@dummy.avatar = stringy_file
end
it "returns a url based on an S3 path" do
assert_match %r{^https://s3.amazonaws.com/bucket/avatars/data.txt}, @dummy.avatar.url
assert_match %r{^https://s3.amazonaws.com/bucket/avatars/data[^\.]}, @dummy.avatar.url
end
end
......@@ -174,13 +174,13 @@ describe Paperclip::Storage::S3 do
s3_credentials: {},
s3_protocol: '',
bucket: "bucket",
path: ":attachment/:basename.:extension"
path: ":attachment/:basename:dotextension"
@dummy = Dummy.new
@dummy.avatar = stringy_file
end
it "returns a url based on an S3 path" do
assert_match %r{^//s3.amazonaws.com/bucket/avatars/data.txt}, @dummy.avatar.url
assert_match %r{^//s3.amazonaws.com/bucket/avatars/data[^\.]}, @dummy.avatar.url
end
end
......@@ -188,7 +188,7 @@ describe Paperclip::Storage::S3 do
before do
rebuild_model storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
styles: {
thumb: "80x80>"
},
......@@ -203,11 +203,11 @@ describe Paperclip::Storage::S3 do
end
it "uses an S3 object based on the correct path for the default style" do
assert_equal("avatars/original/data.txt", @dummy.avatar.s3_object.key)
assert_equal("avatars/original/data", @dummy.avatar.s3_object.key)
end
it "uses an S3 object based on the correct path for the custom style" do
assert_equal("avatars/thumb/data.txt", @dummy.avatar.s3_object(:thumb).key)
assert_equal("avatars/thumb/data", @dummy.avatar.s3_object(:thumb).key)
end
end
......@@ -216,14 +216,14 @@ describe Paperclip::Storage::S3 do
rebuild_model storage: :s3,
s3_credentials: {},
bucket: "bucket",
path: ":attachment/:basename.:extension",
path: ":attachment/:basename:dotextension",
s3_host_name: "s3-ap-northeast-1.amazonaws.com"
@dummy = Dummy.new
@dummy.avatar = stringy_file
end
it "returns a url based on an :s3_host_name path" do
assert_match %r{^http://s3-ap-northeast-1.amazonaws.com/bucket/avatars/data.txt}, @dummy.avatar.url
assert_match %r{^http://s3-ap-northeast-1.amazonaws.com/bucket/avatars/data[^\.]}, @dummy.avatar.url
end
it "uses the S3 bucket with the correct host name" do
......@@ -236,7 +236,7 @@ describe Paperclip::Storage::S3 do
rebuild_model storage: :s3,
s3_credentials: {},
bucket: "bucket",
path: ":attachment/:basename.:extension",
path: ":attachment/:basename:dotextension",
s3_host_name: lambda {|a| a.instance.value }
@dummy = Dummy.new
class << @dummy
......@@ -247,7 +247,7 @@ describe Paperclip::Storage::S3 do
it "uses s3_host_name as a proc if available" do
@dummy.value = "s3.something.com"
assert_equal "http://s3.something.com/bucket/avatars/data.txt", @dummy.avatar.url(:original, timestamp: false)
assert_equal "http://s3.something.com/bucket/avatars/data", @dummy.avatar.url(:original, timestamp: false)
end
end
......@@ -256,7 +256,7 @@ describe Paperclip::Storage::S3 do
rebuild_model styles: { large: ['500x500#', :jpg] },
storage: :s3,
bucket: "bucket",
path: ":attachment/:basename.:extension",
path: ":attachment/:basename:dotextension",
s3_credentials: {
'access_key_id' => "12345",
'secret_access_key' => "54321"
......@@ -290,7 +290,7 @@ describe Paperclip::Storage::S3 do
rebuild_model styles: lambda { |attachment| attachment.instance.counter; {thumbnail: { geometry: "50x50#", s3_headers: {'Cache-Control' => 'max-age=31557600'}} }},
storage: :s3,
bucket: "bucket",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
s3_credentials: {
'access_key_id' => "12345",
'secret_access_key' => "54321"
......@@ -385,14 +385,14 @@ describe Paperclip::Storage::S3 do
rebuild_model storage: :s3,
s3_credentials: {},
bucket: "bucket",
path: ":attachment/:basename.:extension",
path: ":attachment/:basename:dotextension",
url: ":s3_domain_url"
@dummy = Dummy.new
@dummy.avatar = stringy_file
end
it "returns a url based on an S3 subdomain" do
assert_match %r{^http://bucket.s3.amazonaws.com/avatars/data.txt}, @dummy.avatar.url
assert_match %r{^http://bucket.s3.amazonaws.com/avatars/data[^\.]}, @dummy.avatar.url
end
end
......@@ -404,14 +404,14 @@ describe Paperclip::Storage::S3 do
development: { bucket: "dev_bucket" }
},
s3_host_alias: "something.something.com",
path: ":attachment/:basename.:extension",
path: ":attachment/:basename:dotextension",
url: ":s3_alias_url"
@dummy = Dummy.new
@dummy.avatar = stringy_file
end
it "returns a url based on the host_alias" do
assert_match %r{^http://something.something.com/avatars/data.txt}, @dummy.avatar.url
assert_match %r{^http://something.something.com/avatars/data[^\.]}, @dummy.avatar.url
end
end
......@@ -420,7 +420,7 @@ describe Paperclip::Storage::S3 do
rebuild_model storage: :s3,
s3_credentials: { bucket: "prod_bucket" },
s3_host_alias: Proc.new{|atch| "cdn#{atch.instance.counter % 4}.example.com"},
path: ":attachment/:basename.:extension",
path: ":attachment/:basename:dotextension",
url: ":s3_alias_url"
Dummy.class_eval do
def counter
......@@ -434,8 +434,8 @@ describe Paperclip::Storage::S3 do
end
it "returns a url based on the host_alias" do
assert_match %r{^http://cdn1.example.com/avatars/data.txt}, @dummy.avatar.url
assert_match %r{^http://cdn2.example.com/avatars/data.txt}, @dummy.avatar.url
assert_match %r{^http://cdn1.example.com/avatars/data[^\.]}, @dummy.avatar.url
assert_match %r{^http://cdn2.example.com/avatars/data[^\.]}, @dummy.avatar.url
end
it "still returns the bucket name" do
......@@ -449,14 +449,14 @@ describe Paperclip::Storage::S3 do
rebuild_model storage: :s3,
s3_credentials: {},
bucket: "bucket",
path: ":attachment/:basename.:extension",
path: ":attachment/:basename:dotextension",
url: ":asset_host"
@dummy = Dummy.new
@dummy.avatar = stringy_file
end
it "returns a relative URL for Rails to calculate assets host" do
assert_match %r{^avatars/data\.txt}, @dummy.avatar.url
assert_match %r{^avatars/data[^\.]}, @dummy.avatar.url
end
end
......@@ -472,7 +472,7 @@ describe Paperclip::Storage::S3 do
},
s3_host_alias: "something.something.com",
s3_permissions: "private",
path: ":attachment/:basename.:extension",
path: ":attachment/:basename:dotextension",
url: ":s3_alias_url"
}
......@@ -562,7 +562,7 @@ describe Paperclip::Storage::S3 do
},
s3_permissions: :private,
s3_host_alias: "something.something.com",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
url: ":s3_alias_url"
rails_env("production") do
......@@ -648,7 +648,7 @@ describe Paperclip::Storage::S3 do
before do
rebuild_model storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
s3_credentials: {
aws_access_key_id: "12345",
aws_secret_access_key: "54321"
......@@ -825,7 +825,7 @@ describe Paperclip::Storage::S3 do
before do
rebuild_model storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
s3_credentials: {
'access_key_id' => "12345",
'secret_access_key' => "54321"
......@@ -864,7 +864,7 @@ describe Paperclip::Storage::S3 do
before do
rebuild_model storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
s3_credentials: {
'access_key_id' => "12345",
'secret_access_key' => "54321"
......@@ -903,7 +903,7 @@ describe Paperclip::Storage::S3 do
before do
rebuild_model storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
s3_credentials: {
'access_key_id' => "12345",
'secret_access_key' => "54321"
......@@ -942,7 +942,7 @@ describe Paperclip::Storage::S3 do
before do
rebuild_model storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
s3_credentials: {
'access_key_id' => "12345",
'secret_access_key' => "54321"
......@@ -983,7 +983,7 @@ describe Paperclip::Storage::S3 do
rebuild_model(
storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
s3_credentials: {
'access_key_id' => "12345",
'secret_access_key' => "54321"},
......@@ -1021,7 +1021,7 @@ describe Paperclip::Storage::S3 do
before do
rebuild_model storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
s3_credentials: {
'access_key_id' => "12345",
'secret_access_key' => "54321"
......@@ -1060,7 +1060,7 @@ describe Paperclip::Storage::S3 do
before do
rebuild_model storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
s3_credentials: {
'access_key_id' => "12345",
'secret_access_key' => "54321"
......@@ -1145,7 +1145,7 @@ describe Paperclip::Storage::S3 do
before do
rebuild_model storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
s3_credentials: {
'access_key_id' => "12345",
'secret_access_key' => "54321"
......@@ -1182,7 +1182,7 @@ describe Paperclip::Storage::S3 do
before do
rebuild_model storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
s3_credentials: {
'access_key_id' => "12345",
'secret_access_key' => "54321"
......@@ -1220,7 +1220,7 @@ describe Paperclip::Storage::S3 do
before do
rebuild_model storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
styles: {
thumb: "80x80>"
},
......@@ -1267,7 +1267,7 @@ describe Paperclip::Storage::S3 do
rebuild_model(
storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
styles: {
thumb: "80x80>"
},
......@@ -1311,7 +1311,7 @@ describe Paperclip::Storage::S3 do
rebuild_model(
storage: :s3,
bucket: "testing",
path: ":attachment/:style/:basename.:extension",
path: ":attachment/:style/:basename:dotextension",
styles: {
thumb: "80x80>"
},
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment