Skip to content

Commit

Permalink
Fix Content-Type allowlist bypass vulnerability remained
Browse files Browse the repository at this point in the history
  • Loading branch information
mshibuya committed Mar 20, 2024
1 parent e148cd8 commit 25b1c80
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 17 deletions.
2 changes: 1 addition & 1 deletion lib/carrierwave/sanitized_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ def sanitize(name)
def declared_content_type
@declared_content_type ||
if @file.respond_to?(:content_type) && @file.content_type
@file.content_type.to_s.chomp
Marcel::MimeType.for(declared_type: @file.content_type.to_s.chomp)
end
end

Expand Down
27 changes: 27 additions & 0 deletions spec/sanitized_file_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,33 @@

expect { sanitized_file.content_type }.not_to raise_error
end

it "uses the first one when multiple mime types are given using a semicolon" do
file = File.open(file_path("bork.txt"))
allow(file).to receive(:content_type) { 'image/png; text/html' }

sanitized_file = CarrierWave::SanitizedFile.new(file)

expect(sanitized_file.content_type).to eq("image/png")
end

it "uses the first one when multiple mime types are given using a comma" do
file = File.open(file_path("bork.txt"))
allow(file).to receive(:content_type) { 'image/png, text/html' }

sanitized_file = CarrierWave::SanitizedFile.new(file)

expect(sanitized_file.content_type).to eq("image/png")
end

it "drops content type parameters" do
file = File.open(file_path("bork.txt"))
allow(file).to receive(:content_type) { 'text/html; charset=utf-8' }

sanitized_file = CarrierWave::SanitizedFile.new(file)

expect(sanitized_file.content_type).to eq("text/html")
end
end

describe "#content_type=" do
Expand Down
16 changes: 0 additions & 16 deletions spec/uploader/content_type_allowlist_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,22 +87,6 @@
expect { uploader.cache!(bork_file) }.to raise_error(CarrierWave::IntegrityError)
end
end

context "when the allowlist contains charset" do
before do
allow(uploader).to receive(:content_type_allowlist).and_return(%r{text/plain;\s*charset=utf-8})
end

it "accepts the content with allowed charset" do
allow(bork_file).to receive(:content_type).and_return('text/plain; charset=utf-8')
expect { uploader.cache!(bork_file) }.not_to raise_error
end

it "rejects the content without charset" do
allow(bork_file).to receive(:content_type).and_return('text/plain')
expect { uploader.cache!(bork_file) }.to raise_error(CarrierWave::IntegrityError)
end
end
end

context "when there is a whitelist" do
Expand Down

0 comments on commit 25b1c80

Please sign in to comment.