Commit 72f71818 authored by GitLab Bot's avatar GitLab Bot
Browse files

Add latest changes from gitlab-org/security/gitlab@13-10-stable-ee

parent 9f1ef4d5460e
......@@ -28,6 +28,8 @@ gem 'devise', '~> 4.7.2'
gem 'bcrypt', '~> 3.1', '>= 3.1.14'
gem 'doorkeeper', '~> 5.5.0.rc2'
gem 'doorkeeper-openid_connect', '~> 1.7.5'
gem 'rexml', '~> 3.2.5'
gem 'ruby-saml', '~> 1.12.1'
gem 'omniauth', '~> 1.8'
gem 'omniauth-auth0', '~> 2.0.0'
gem 'omniauth-azure-activedirectory-v2', '~> 0.1'
......
......@@ -1042,7 +1042,7 @@ GEM
retriable (3.1.2)
reverse_markdown (1.4.0)
nokogiri
rexml (3.2.4)
rexml (3.2.5)
rinku (2.0.0)
rotp (2.1.2)
rouge (3.26.0)
......@@ -1116,8 +1116,9 @@ GEM
ruby-magic-static (0.3.4)
ruby-prof (1.3.1)
ruby-progressbar (1.11.0)
ruby-saml (1.7.2)
nokogiri (>= 1.5.10)
ruby-saml (1.12.1)
nokogiri (>= 1.10.5)
rexml
ruby-statistics (2.1.2)
ruby2_keywords (0.0.2)
ruby_parser (3.15.0)
......@@ -1552,6 +1553,7 @@ DEPENDENCIES
request_store (~> 1.5)
responders (~> 3.0)
retriable (~> 3.1.2)
rexml (~> 3.2.5)
rouge (~> 3.26.0)
rqrcode-rails3 (~> 0.1.7)
rspec-parameterized
......@@ -1563,6 +1565,7 @@ DEPENDENCIES
ruby-magic-static (~> 0.3.4)
ruby-prof (~> 1.3.0)
ruby-progressbar (~> 1.10)
ruby-saml (~> 1.12.1)
ruby_parser (~> 3.15)
rubyzip (~> 2.0.0)
rugged (~> 1.1)
......
---
title: Clean only legitimate JPG and TIFF files
merge_request:
author:
type: security
---
title: Update ruby-saml and rexml gems
merge_request:
author:
type: security
......@@ -45,6 +45,7 @@ class Exif
ALLOWED_TAGS = WHITELISTED_TAGS + IGNORED_TAGS
EXCLUDE_PARAMS = WHITELISTED_TAGS.map { |tag| "-#{tag}" }
ALLOWED_MIME_TYPES = %w(image/jpeg image/tiff).freeze
attr_reader :logger
......@@ -96,12 +97,12 @@ def clean(uploader, dry_run: true)
end
end
private
def extra_tags(path)
exif_tags(path).keys - ALLOWED_TAGS
end
private
def remove_and_store(tmpdir, src_path, uploader)
exec_remove_exif!(src_path)
logger.info "#{upload_ref(uploader.upload)}: exif removed, storing"
......@@ -133,15 +134,26 @@ def fetch_upload_to_file(uploader, dir)
# upload is stored into the file with the original name - this filename
# is used by carrierwave when storing the file back to the storage
filename = File.join(dir, uploader.filename)
contents = uploader.read
check_for_allowed_types(contents)
File.open(filename, 'w') do |file|
file.binmode
file.write uploader.read
file.write contents
end
filename
end
def check_for_allowed_types(contents)
mime_type = Gitlab::Utils::MimeType.from_string(contents)
unless ALLOWED_MIME_TYPES.include?(mime_type)
raise "File type #{mime_type} not supported. Only supports #{ALLOWED_MIME_TYPES.join(", ")}."
end
end
def upload_ref(upload)
"#{upload.id}:#{upload.path}"
end
......
......@@ -4,6 +4,11 @@
RSpec.describe Gitlab::Sanitizers::Exif do
let(:sanitizer) { described_class.new }
let(:mime_type) { 'image/jpeg' }
before do
allow(Gitlab::Utils::MimeType).to receive(:from_string).and_return(mime_type)
end
describe '#batch_clean' do
context 'with image uploads' do
......@@ -43,7 +48,7 @@
end
end
it 'filters only jpg/tiff images' do
it 'filters only jpg/tiff images by filename' do
create(:upload, path: 'filename.jpg')
create(:upload, path: 'filename.jpeg')
create(:upload, path: 'filename.JPG')
......@@ -53,12 +58,16 @@
create(:upload, path: 'filename.txt')
expect(sanitizer).to receive(:clean).exactly(5).times
sanitizer.batch_clean
end
end
describe '#clean' do
let(:uploader) { create(:upload, :with_file, :issuable_upload).retrieve_uploader }
let(:dry_run) { false }
subject { sanitizer.clean(uploader, dry_run: dry_run) }
context "no dry run" do
it "removes exif from the image" do
......@@ -76,7 +85,7 @@
[expected_args, 0]
end
sanitizer.clean(uploader, dry_run: false)
subject
expect(uploader.upload.id).not_to eq(original_upload.id)
expect(uploader.upload.path).to eq(original_upload.path)
......@@ -89,23 +98,35 @@
expect(sanitizer).not_to receive(:exec_remove_exif!)
expect(uploader).not_to receive(:store!)
sanitizer.clean(uploader, dry_run: false)
subject
end
it "raises an error if the exiftool fails with an error" do
expect(Gitlab::Popen).to receive(:popen).and_return(["error", 1])
expect { sanitizer.clean(uploader, dry_run: false) }.to raise_exception(RuntimeError, "failed to get exif tags: error")
expect { subject }.to raise_exception(RuntimeError, "failed to get exif tags: error")
end
context 'for files that do not have the correct MIME type' do
let(:mime_type) { 'text/plain' }
it 'cleans only jpg/tiff images with the correct mime types' do
expect(sanitizer).not_to receive(:extra_tags)
expect { subject }.to raise_error(RuntimeError, /File type text\/plain not supported/)
end
end
end
context "dry run" do
let(:dry_run) { true }
it "doesn't change the image" do
expect(sanitizer).to receive(:extra_tags).and_return({ 'foo' => 'bar' })
expect(sanitizer).not_to receive(:exec_remove_exif!)
expect(uploader).not_to receive(:store!)
sanitizer.clean(uploader, dry_run: true)
subject
end
end
end
......@@ -119,7 +140,7 @@
expect(Gitlab::Popen).to receive(:popen).and_return([tags, 0])
expect(sanitizer.extra_tags('filename')).not_to be_empty
expect(sanitizer.send(:extra_tags, 'filename')).not_to be_empty
end
it "returns an empty list for file with only whitelisted and ignored tags" do
......@@ -130,7 +151,7 @@
expect(Gitlab::Popen).to receive(:popen).and_return([tags, 0])
expect(sanitizer.extra_tags('some file')).to be_empty
expect(sanitizer.send(:extra_tags, 'some file')).to be_empty
end
end
end
File mode changed from 100644 to 100755
File mode changed from 100644 to 100755
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