diff --git a/CHANGELOG.md b/CHANGELOG.md index 629a092977..0ac3dec21c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 13.9.6 (2021-04-13) + +### Security (2 changes) + +- Clean only legitimate JPG and TIFF files. +- Update ruby-saml and rexml gems. + + ## 13.9.5 (2021-03-31) ### Security (6 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 62f8ce01fb..c2273bba24 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -13.9.5 \ No newline at end of file +13.9.6 \ No newline at end of file diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index 48309c07a5..36fd88335d 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -8.63.2 +8.63.3 diff --git a/Gemfile b/Gemfile index 8d39ce9414..c70e1755e1 100644 --- a/Gemfile +++ b/Gemfile @@ -29,6 +29,8 @@ gem 'devise', '~> 4.7.2' gem 'bcrypt', '3.1.12' 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-oauth2', '~> 0.0.9' diff --git a/Gemfile.lock b/Gemfile.lock index 538a43a6eb..9d3fb1eee4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -998,7 +998,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) @@ -1072,8 +1072,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) @@ -1498,6 +1499,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 @@ -1509,6 +1511,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.0.1) diff --git a/VERSION b/VERSION index 62f8ce01fb..c2273bba24 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -13.9.5 \ No newline at end of file +13.9.6 \ No newline at end of file diff --git a/debian/changelog b/debian/changelog index 00cfd16d40..bfe30b9ff2 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +gitlab (13.9.6+ds1-1) experimental; urgency=medium + + * New upstream security release 13.9.6+ds1 + * Update minimum version of ruby-rexml to 3.2.5 + * Add ruby-saml as a dependency + * Refresh patches + + -- Pirate Praveen Thu, 15 Apr 2021 22:42:45 +0530 + gitlab (13.9.5+ds1-1~fto10+1) buster-fasttrack; urgency=medium * Rebuild for buster-fasttrack. diff --git a/debian/control b/debian/control index 6889b57c23..51597ad37d 100644 --- a/debian/control +++ b/debian/control @@ -56,6 +56,8 @@ Depends: ${shlibs:Depends}, ${misc:Depends}, ruby-bcrypt (>= 3.1.14~), ruby-doorkeeper (>= 5.5~), ruby-doorkeeper-openid-connect (>= 1.7.5~), + ruby-rexml (>= 3.2.5~), + ruby-saml (>= 1.12.1~), ruby-omniauth (>= 1.8~), ruby-omniauth-auth0 (>= 2.0~), ruby-omniauth-azure-oauth2 (>= 0.0.10~), diff --git a/debian/patches/0050-relax-stable-libs.patch b/debian/patches/0050-relax-stable-libs.patch index b83624f087..73b4616a71 100644 --- a/debian/patches/0050-relax-stable-libs.patch +++ b/debian/patches/0050-relax-stable-libs.patch @@ -3,7 +3,7 @@ gitlab Gemfile --- a/Gemfile +++ b/Gemfile -@@ -2,53 +2,53 @@ +@@ -2,55 +2,55 @@ source 'https://rubygems.org' @@ -42,9 +42,13 @@ gitlab Gemfile -gem 'bcrypt', '3.1.12' -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 'bcrypt', '~> 3.1', '>= 3.1.12' +gem 'doorkeeper', '~> 5.5' +gem 'doorkeeper-openid_connect', '~> 1.7', '>= 1.7.5' ++gem 'rexml', '~> 3.2', '>= 3.2.5' ++gem 'ruby-saml', '~> 1.12', '>= 1.12.1' gem 'omniauth', '~> 1.8' -gem 'omniauth-auth0', '~> 2.0.0' +gem 'omniauth-auth0', '~> 2.0' @@ -77,7 +81,7 @@ gitlab Gemfile # Kerberos authentication. EE-only gem 'gssapi', group: :kerberos -@@ -56,17 +56,17 @@ +@@ -58,17 +58,17 @@ # Spam and anti-bot protection gem 'recaptcha', '~> 4.11', require: 'recaptcha/rails' gem 'akismet', '~> 3.0' @@ -100,7 +104,7 @@ gitlab Gemfile # GitLab Pages letsencrypt support gem 'acme-client', '~> 2.0', '>= 2.0.6' -@@ -74,27 +74,27 @@ +@@ -76,27 +76,27 @@ gem 'browser', '~> 4.2' # GPG @@ -136,7 +140,7 @@ gitlab Gemfile gem 'graphlient', '~> 0.4.0' # Used by BulkImport feature (group::import) gem 'hashie' -@@ -105,11 +105,11 @@ +@@ -107,11 +107,11 @@ gem 'kaminari', '~> 1.0' # HAML @@ -150,7 +154,7 @@ gitlab Gemfile # for backups gem 'fog-aws', '~> 3.8' -@@ -130,37 +130,37 @@ +@@ -132,37 +132,37 @@ gem 'unf', '~> 0.1.4' # Seed data @@ -201,7 +205,7 @@ gitlab Gemfile gem 'escape_utils', '~> 1.1' # Calendar rendering -@@ -171,7 +171,7 @@ +@@ -173,7 +173,7 @@ gem 'diff_match_patch', '~> 0.1.0' # Application server @@ -210,7 +214,7 @@ gitlab Gemfile # https://github.com/sharpstone/rack-timeout/blob/master/README.md#rails-apps-manually gem 'rack-timeout', '~> 0.5.1', require: 'rack/timeout/base' -@@ -181,7 +181,7 @@ +@@ -183,7 +183,7 @@ end group :puma do @@ -219,7 +223,7 @@ gitlab Gemfile gem 'puma_worker_killer', '~> 0.3.1', require: false end -@@ -192,13 +192,13 @@ +@@ -194,13 +194,13 @@ gem 'acts-as-taggable-on', '~> 7.0' # Background jobs @@ -236,7 +240,7 @@ gitlab Gemfile # HTTP requests gem 'httparty', '~> 0.16.4' -@@ -210,14 +210,14 @@ +@@ -212,14 +212,14 @@ gem 'ruby-progressbar', '~> 1.10' # GitLab settings @@ -254,7 +258,7 @@ gitlab Gemfile # Export Ruby Regex to Javascript gem 'js_regex', '~> 3.4' -@@ -230,20 +230,20 @@ +@@ -232,20 +232,20 @@ gem 'connection_pool', '~> 2.0' # Redis session store @@ -278,7 +282,7 @@ gitlab Gemfile # Hangouts Chat integration gem 'hangouts-chat', '~> 0.0.5' -@@ -255,11 +255,11 @@ +@@ -257,11 +257,11 @@ gem 'ruby-fogbugz', '~> 0.2.1' # Kubernetes integration @@ -293,7 +297,7 @@ gitlab Gemfile # Sanitizes SVG input gem 'loofah', '~> 2.2' -@@ -285,9 +285,9 @@ +@@ -287,9 +287,9 @@ gem 'rack-proxy', '~> 0.6.0' @@ -306,7 +310,7 @@ gitlab Gemfile gem 'addressable', '~> 2.7' gem 'gemojione', '~> 3.3' -@@ -298,18 +298,18 @@ +@@ -300,18 +300,18 @@ gem "gitlab-license", "~> 1.3" # Protect against bruteforcing @@ -329,7 +333,7 @@ gitlab Gemfile # Thrift is a dependency of gitlab-labkit, we want a version higher than 0.14.0 # because of https://gitlab.com/gitlab-org/gitlab/-/issues/321900 gem 'thrift', '>= 0.14.0' -@@ -317,11 +317,11 @@ +@@ -319,11 +319,11 @@ # I18n gem 'ruby_parser', '~> 3.15', require: false gem 'rails-i18n', '~> 6.0' @@ -343,7 +347,7 @@ gitlab Gemfile # Perf bar gem 'peek', '~> 1.1' -@@ -354,39 +354,39 @@ +@@ -356,39 +356,39 @@ end group :development, :test do @@ -394,7 +398,7 @@ gitlab Gemfile gem 'timecop', '~> 0.9.1' -@@ -408,18 +408,18 @@ +@@ -410,18 +410,18 @@ end group :test do @@ -419,7 +423,7 @@ gitlab Gemfile gem 'rails-controller-testing' gem 'concurrent-ruby', '~> 1.1' gem 'test-prof', '~> 0.12.0' -@@ -438,7 +438,7 @@ +@@ -440,7 +440,7 @@ gem 'email_reply_trimmer', '~> 0.1' gem 'html2text' @@ -428,7 +432,7 @@ gitlab Gemfile gem 'stackprof', '~> 0.2.15', require: false gem 'rbtrace', '~> 0.4', require: false gem 'memory_profiler', '~> 0.9', require: false -@@ -452,8 +452,8 @@ +@@ -454,8 +454,8 @@ gem 'health_check', '~> 3.0' # System information @@ -439,7 +443,7 @@ gitlab Gemfile # NTP client gem 'net-ntp' -@@ -469,13 +469,13 @@ +@@ -471,13 +471,13 @@ end # Gitaly GRPC protocol definitions @@ -456,7 +460,7 @@ gitlab Gemfile # Feature toggles gem 'flipper', '~> 0.17.1' -@@ -494,12 +494,12 @@ +@@ -496,12 +496,12 @@ # Countries list gem 'countries', '~> 3.0' @@ -471,7 +475,7 @@ gitlab Gemfile # Locked as long as quoted-printable encoding issues are not resolved # Monkey-patched in `config/initializers/mail_encoding_patch.rb` -@@ -513,12 +513,12 @@ +@@ -515,12 +515,12 @@ gem 'valid_email', '~> 0.1' # JSON diff --git a/debian/patches/0100-remove-development-test.patch b/debian/patches/0100-remove-development-test.patch index 8483594d51..d760415f7b 100644 --- a/debian/patches/0100-remove-development-test.patch +++ b/debian/patches/0100-remove-development-test.patch @@ -2,7 +2,7 @@ Bundler will fail when it can't find these locally --- a/Gemfile +++ b/Gemfile -@@ -94,7 +94,6 @@ +@@ -96,7 +96,6 @@ # https://gitlab.com/gitlab-org/gitlab/issues/31747 gem 'graphiql-rails', '~> 1.4', '>= 1.4.10' gem 'apollo_upload_server', '~> 2.0', '>= 2.0.2' @@ -10,7 +10,7 @@ Bundler will fail when it can't find these locally gem 'graphlient', '~> 0.4.0' # Used by BulkImport feature (group::import) gem 'hashie' -@@ -319,7 +318,6 @@ +@@ -321,7 +320,6 @@ gem 'rails-i18n', '~> 6.0' gem 'gettext_i18n_rails', '~> 1.8' gem 'gettext_i18n_rails_js', '~> 1.3' @@ -18,7 +18,7 @@ Bundler will fail when it can't find these locally gem 'batch-loader', '~> 1.4' -@@ -339,20 +337,6 @@ +@@ -341,20 +339,6 @@ gem 'raindrops', '~> 0.18' end @@ -39,7 +39,7 @@ Bundler will fail when it can't find these locally group :development, :test do gem 'deprecation_toolkit', '~> 1.5', '>= 1.5.1', require: false gem 'bullet', '~> 6.1', '>= 6.1.3' -@@ -375,12 +359,6 @@ +@@ -377,12 +361,6 @@ gem 'spring', '~> 2.1' gem 'spring-commands-rspec', '~> 1.0', '>= 1.0.4' @@ -52,7 +52,7 @@ Bundler will fail when it can't find these locally gem 'benchmark-ips', '~> 2.3', require: false gem 'knapsack', '~> 1.17' -@@ -397,16 +375,6 @@ +@@ -399,16 +377,6 @@ gem 'rblineprof', '~> 0.3.6', platform: :mri, require: false end diff --git a/debian/patches/0110-make-test-dependencies-conditional.patch b/debian/patches/0110-make-test-dependencies-conditional.patch index 67da2f0901..4c8d99fdaf 100644 --- a/debian/patches/0110-make-test-dependencies-conditional.patch +++ b/debian/patches/0110-make-test-dependencies-conditional.patch @@ -2,7 +2,7 @@ Make test dependencies conditional so we can enable them when running autopkgtes --- a/Gemfile +++ b/Gemfile -@@ -337,7 +337,7 @@ +@@ -339,7 +339,7 @@ gem 'raindrops', '~> 0.18' end @@ -11,7 +11,7 @@ Make test dependencies conditional so we can enable them when running autopkgtes gem 'deprecation_toolkit', '~> 1.5', '>= 1.5.1', require: false gem 'bullet', '~> 6.1', '>= 6.1.3' gem 'gitlab-pry-byebug', platform: :mri, require: ['pry-byebug', 'pry-byebug/pry_remote_ext'] -@@ -373,9 +373,7 @@ +@@ -375,9 +375,7 @@ gem 'parallel', '~> 1.19', require: false gem 'rblineprof', '~> 0.3.6', platform: :mri, require: false diff --git a/debian/patches/0340-relax-httparty.patch b/debian/patches/0340-relax-httparty.patch index 49c6866613..20e139f36e 100644 --- a/debian/patches/0340-relax-httparty.patch +++ b/debian/patches/0340-relax-httparty.patch @@ -2,7 +2,7 @@ Allow newer versions of httparty to satisfy dependency --- a/Gemfile +++ b/Gemfile -@@ -200,7 +200,7 @@ +@@ -202,7 +202,7 @@ gem 'fugit', '~> 1.2', '>= 1.2.1' # HTTP requests diff --git a/debian/patches/0350-relax-rdoc.patch b/debian/patches/0350-relax-rdoc.patch index c03769a423..3bf3a90267 100644 --- a/debian/patches/0350-relax-rdoc.patch +++ b/debian/patches/0350-relax-rdoc.patch @@ -2,7 +2,7 @@ Allow rdoc from ruby 2.5 to match requirement --- a/Gemfile +++ b/Gemfile -@@ -148,7 +148,7 @@ +@@ -150,7 +150,7 @@ gem 'commonmarker', '~> 0.21' gem 'kramdown', '~> 2.3' gem 'RedCloth', '~> 4.3', '>= 4.3.2' diff --git a/debian/patches/0430-remove-gitlab-markup.patch b/debian/patches/0430-remove-gitlab-markup.patch index 1d609164f1..e8c5e7fb19 100644 --- a/debian/patches/0430-remove-gitlab-markup.patch +++ b/debian/patches/0430-remove-gitlab-markup.patch @@ -4,7 +4,7 @@ maintaining two almost same packages. --- a/Gemfile +++ b/Gemfile -@@ -143,7 +143,6 @@ +@@ -145,7 +145,6 @@ # Markdown and HTML processing gem 'html-pipeline', '~> 2.13', '>= 2.13.2' gem 'deckar01-task_list', '~> 2.3', '>= 2.3.1' diff --git a/debian/patches/0440-remove-unicorn.patch b/debian/patches/0440-remove-unicorn.patch index a1d84ae203..71a6e8468b 100644 --- a/debian/patches/0440-remove-unicorn.patch +++ b/debian/patches/0440-remove-unicorn.patch @@ -3,7 +3,7 @@ gitlab-puma changes is included in puma package. --- a/Gemfile +++ b/Gemfile -@@ -173,11 +173,6 @@ +@@ -175,11 +175,6 @@ # https://github.com/sharpstone/rack-timeout/blob/master/README.md#rails-apps-manually gem 'rack-timeout', '~> 0.5.1', require: 'rack/timeout/base' diff --git a/debian/patches/0480-embed-elasticsearch-model.patch b/debian/patches/0480-embed-elasticsearch-model.patch index df60823cd1..022f904245 100644 --- a/debian/patches/0480-embed-elasticsearch-model.patch +++ b/debian/patches/0480-embed-elasticsearch-model.patch @@ -2,7 +2,7 @@ Embed this gem until gitlab moved to 7.x version --- a/Gemfile +++ b/Gemfile -@@ -132,7 +132,7 @@ +@@ -134,7 +134,7 @@ gem 'seed-fu', '~> 2.3', '>= 2.3.7' # Search diff --git a/debian/patches/0480-embed-elasticsearch-rails.patch b/debian/patches/0480-embed-elasticsearch-rails.patch index dbc076d535..df7f034611 100644 --- a/debian/patches/0480-embed-elasticsearch-rails.patch +++ b/debian/patches/0480-embed-elasticsearch-rails.patch @@ -2,7 +2,7 @@ Embed this gem until gitlab moved to 7.x version --- a/Gemfile +++ b/Gemfile -@@ -133,7 +133,7 @@ +@@ -135,7 +135,7 @@ # Search gem 'elasticsearch-model', '~> 6.1', path: 'vendor/gems/elasticsearch-model' diff --git a/debian/patches/0482-remove-ee-only-gems.patch b/debian/patches/0482-remove-ee-only-gems.patch index 2bd5e763ba..455802c04d 100644 --- a/debian/patches/0482-remove-ee-only-gems.patch +++ b/debian/patches/0482-remove-ee-only-gems.patch @@ -2,7 +2,7 @@ This gem is used only in gitlab Enterprise Edition --- a/Gemfile +++ b/Gemfile -@@ -50,9 +50,6 @@ +@@ -52,9 +52,6 @@ gem 'rack-oauth2', '~> 1.16' gem 'jwt', '~> 2.1' @@ -12,7 +12,7 @@ This gem is used only in gitlab Enterprise Edition # Spam and anti-bot protection gem 'recaptcha', '~> 4.11', require: 'recaptcha/rails' gem 'akismet', '~> 3.0' -@@ -288,8 +285,6 @@ +@@ -290,8 +287,6 @@ gem 'request_store', '~> 1.5' gem 'base32', '~> 0.3.0' diff --git a/debian/patches/0484-relax-grape-entity.patch b/debian/patches/0484-relax-grape-entity.patch index 431a140f2e..6656494b49 100644 --- a/debian/patches/0484-relax-grape-entity.patch +++ b/debian/patches/0484-relax-grape-entity.patch @@ -2,7 +2,7 @@ Debian already has 0.8 --- a/Gemfile +++ b/Gemfile -@@ -81,7 +81,7 @@ +@@ -83,7 +83,7 @@ # API gem 'grape', '~> 1.5', '>= 1.5.2' diff --git a/debian/patches/0485-relax-gitlab-sidekiq-fetcher.patch b/debian/patches/0485-relax-gitlab-sidekiq-fetcher.patch index a784a1be34..12285fbe62 100644 --- a/debian/patches/0485-relax-gitlab-sidekiq-fetcher.patch +++ b/debian/patches/0485-relax-gitlab-sidekiq-fetcher.patch @@ -2,7 +2,7 @@ Allow newer version in the archive to satisfy the requirement --- a/Gemfile +++ b/Gemfile -@@ -185,7 +185,7 @@ +@@ -187,7 +187,7 @@ gem 'sidekiq', '~> 5.2', '>= 5.2.7' gem 'sidekiq-cron', '~> 1.0' gem 'redis-namespace', '~> 1.7' diff --git a/debian/patches/0486-relax-sidekiq.patch b/debian/patches/0486-relax-sidekiq.patch index fd570b8a52..273e11f542 100644 --- a/debian/patches/0486-relax-sidekiq.patch +++ b/debian/patches/0486-relax-sidekiq.patch @@ -2,7 +2,7 @@ ruby-sidekiq 6 is in unstable --- a/Gemfile +++ b/Gemfile -@@ -182,7 +182,7 @@ +@@ -184,7 +184,7 @@ gem 'acts-as-taggable-on', '~> 7.0' # Background jobs diff --git a/debian/patches/0499-10-relax-capybara.patch b/debian/patches/0499-10-relax-capybara.patch index 3f60fbdd01..2218e799e9 100644 --- a/debian/patches/0499-10-relax-capybara.patch +++ b/debian/patches/0499-10-relax-capybara.patch @@ -1,6 +1,6 @@ --- a/Gemfile +++ b/Gemfile -@@ -368,7 +368,7 @@ +@@ -370,7 +370,7 @@ gem 'rspec_profiling', '~> 0.0.6' gem 'rspec-parameterized', require: false diff --git a/debian/patches/0499-20-remove-capybara-screenshot.patch b/debian/patches/0499-20-remove-capybara-screenshot.patch index 1fbe0e0e13..d16035b488 100644 --- a/debian/patches/0499-20-remove-capybara-screenshot.patch +++ b/debian/patches/0499-20-remove-capybara-screenshot.patch @@ -1,6 +1,6 @@ --- a/Gemfile +++ b/Gemfile -@@ -369,7 +369,6 @@ +@@ -371,7 +371,6 @@ gem 'rspec-parameterized', require: false gem 'capybara', '~> 3.12' diff --git a/debian/patches/0499-30-remove-guard-rspec.patch b/debian/patches/0499-30-remove-guard-rspec.patch index c374836b65..16bbda0a29 100644 --- a/debian/patches/0499-30-remove-guard-rspec.patch +++ b/debian/patches/0499-30-remove-guard-rspec.patch @@ -1,6 +1,6 @@ --- a/Gemfile +++ b/Gemfile -@@ -378,7 +378,6 @@ +@@ -380,7 +380,6 @@ gem 'concurrent-ruby', '~> 1.1' gem 'test-prof', '~> 0.12.0' gem 'rspec_junit_formatter' diff --git a/debian/patches/0499-40-relax-rouge.patch b/debian/patches/0499-40-relax-rouge.patch index e10b372555..cc60fe576e 100644 --- a/debian/patches/0499-40-relax-rouge.patch +++ b/debian/patches/0499-40-relax-rouge.patch @@ -3,7 +3,7 @@ rouge update is blocked by --- a/Gemfile +++ b/Gemfile -@@ -152,7 +152,7 @@ +@@ -154,7 +154,7 @@ gem 'asciidoctor-include-ext', '~> 0.3.1', require: false gem 'asciidoctor-plantuml', '~> 0.0.12' gem 'asciidoctor-kroki', '~> 0.4.0', require: false diff --git a/debian/patches/0499-70-relax-graphlient.patch b/debian/patches/0499-70-relax-graphlient.patch index f55c9d5837..ea843073ab 100644 --- a/debian/patches/0499-70-relax-graphlient.patch +++ b/debian/patches/0499-70-relax-graphlient.patch @@ -2,7 +2,7 @@ newer version is in the archive --- a/Gemfile +++ b/Gemfile -@@ -91,7 +91,7 @@ +@@ -93,7 +93,7 @@ # https://gitlab.com/gitlab-org/gitlab/issues/31747 gem 'graphiql-rails', '~> 1.4', '>= 1.4.10' gem 'apollo_upload_server', '~> 2.0', '>= 2.0.2' diff --git a/debian/patches/0499-90-relax-webrick.patch b/debian/patches/0499-90-relax-webrick.patch index b23ce6b5b5..eb25c5da8b 100644 --- a/debian/patches/0499-90-relax-webrick.patch +++ b/debian/patches/0499-90-relax-webrick.patch @@ -1,6 +1,6 @@ --- a/Gemfile +++ b/Gemfile -@@ -319,7 +319,7 @@ +@@ -321,7 +321,7 @@ # Metrics group :metrics do gem 'method_source', '~> 1.0', require: false diff --git a/debian/patches/0499-91-relax-gitlab-labkit.patch b/debian/patches/0499-91-relax-gitlab-labkit.patch index 0c8c3ec22b..70c32fdf8c 100644 --- a/debian/patches/0499-91-relax-gitlab-labkit.patch +++ b/debian/patches/0499-91-relax-gitlab-labkit.patch @@ -2,7 +2,7 @@ gitaly needs gitlab-labkit ~> 0.15.0 --- a/Gemfile +++ b/Gemfile -@@ -297,7 +297,7 @@ +@@ -299,7 +299,7 @@ gem 'premailer-rails', '~> 1.10', '>= 1.10.3' # LabKit: Tracing and Correlation diff --git a/lib/gitlab/sanitizers/exif.rb b/lib/gitlab/sanitizers/exif.rb index ed3e32f3e7..eec50deb61 100644 --- a/lib/gitlab/sanitizers/exif.rb +++ b/lib/gitlab/sanitizers/exif.rb @@ -45,6 +45,7 @@ module Gitlab 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 @@ module Gitlab 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 @@ module Gitlab # 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 diff --git a/spec/lib/gitlab/sanitizers/exif_spec.rb b/spec/lib/gitlab/sanitizers/exif_spec.rb index 88ef3ce6aa..63b2f3fc69 100644 --- a/spec/lib/gitlab/sanitizers/exif_spec.rb +++ b/spec/lib/gitlab/sanitizers/exif_spec.rb @@ -4,6 +4,11 @@ require 'spec_helper' 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 @@ RSpec.describe Gitlab::Sanitizers::Exif do 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 @@ RSpec.describe Gitlab::Sanitizers::Exif do 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 @@ RSpec.describe Gitlab::Sanitizers::Exif do [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 @@ RSpec.describe Gitlab::Sanitizers::Exif do 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 @@ RSpec.describe Gitlab::Sanitizers::Exif do 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 @@ RSpec.describe Gitlab::Sanitizers::Exif do 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 diff --git a/workhorse/CHANGELOG b/workhorse/CHANGELOG index 0d29061cca..4193568c0b 100644 --- a/workhorse/CHANGELOG +++ b/workhorse/CHANGELOG @@ -1,5 +1,11 @@ # Changelog for gitlab-workhorse +## v8.63.3 + +### Security +- Check image content type before running exiftool in workhorse + https://gitlab.com/gitlab-org/gitlab-workhorse/-/merge_requests/ + ## v8.63.2 ### Security diff --git a/workhorse/VERSION b/workhorse/VERSION index 48309c07a5..36fd88335d 100644 --- a/workhorse/VERSION +++ b/workhorse/VERSION @@ -1 +1 @@ -8.63.2 +8.63.3 diff --git a/workhorse/go.mod b/workhorse/go.mod index 20344f0081..047ca37d15 100644 --- a/workhorse/go.mod +++ b/workhorse/go.mod @@ -29,6 +29,7 @@ require ( gitlab.com/gitlab-org/gitaly v1.74.0 gitlab.com/gitlab-org/labkit v1.0.0 gocloud.dev v0.21.1-0.20201223184910-5094f54ed8bb + golang.org/x/image v0.0.0-20191009234506-e7c1f5e7dbb8 golang.org/x/lint v0.0.0-20200302205851-738671d3881b golang.org/x/net v0.0.0-20201224014010-6772e930b67b golang.org/x/sys v0.0.0-20210110051926-789bb1bd4061 // indirect diff --git a/workhorse/internal/upload/exif/exif.go b/workhorse/internal/upload/exif/exif.go index a9307b1ca9..2f8218c3bc 100644 --- a/workhorse/internal/upload/exif/exif.go +++ b/workhorse/internal/upload/exif/exif.go @@ -22,6 +22,14 @@ type cleaner struct { eof bool } +type FileType int + +const ( + TypeUnknown FileType = iota + TypeJPEG + TypeTIFF +) + func NewCleaner(ctx context.Context, stdin io.Reader) (io.ReadCloser, error) { c := &cleaner{ctx: ctx} @@ -100,8 +108,16 @@ func (c *cleaner) startProcessing(stdin io.Reader) error { return nil } -func IsExifFile(filename string) bool { - filenameMatch := regexp.MustCompile(`(?i)\.(jpg|jpeg|tiff)$`) +func FileTypeFromSuffix(filename string) FileType { + jpegMatch := regexp.MustCompile(`(?i)^[^\n]*\.(jpg|jpeg)$`) + if jpegMatch.MatchString(filename) { + return TypeJPEG + } - return filenameMatch.MatchString(filename) + tiffMatch := regexp.MustCompile(`(?i)^[^\n]*\.tiff$`) + if tiffMatch.MatchString(filename) { + return TypeTIFF + } + + return TypeUnknown } diff --git a/workhorse/internal/upload/exif/exif_test.go b/workhorse/internal/upload/exif/exif_test.go index 373d97f7fc..ee5883d9e0 100644 --- a/workhorse/internal/upload/exif/exif_test.go +++ b/workhorse/internal/upload/exif/exif_test.go @@ -11,39 +11,57 @@ import ( "github.com/stretchr/testify/require" ) -func TestIsExifFile(t *testing.T) { +func TestFileTypeFromSuffix(t *testing.T) { tests := []struct { name string - expected bool + expected FileType }{ { name: "/full/path.jpg", - expected: true, + expected: TypeJPEG, }, { name: "path.jpeg", - expected: true, + expected: TypeJPEG, }, { name: "path.tiff", - expected: true, + expected: TypeTIFF, }, { name: "path.JPG", - expected: true, + expected: TypeJPEG, }, { name: "path.tar", - expected: false, + expected: TypeUnknown, }, { name: "path", - expected: false, + expected: TypeUnknown, + }, + { + name: "something.jpg.py", + expected: TypeUnknown, + }, + { + name: "something.py.jpg", + expected: TypeJPEG, + }, + { + name: `something.jpg + .py`, + expected: TypeUnknown, + }, + { + name: `something.something + .jpg`, + expected: TypeUnknown, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - require.Equal(t, test.expected, IsExifFile(test.name)) + require.Equal(t, test.expected, FileTypeFromSuffix(test.name)) }) } } diff --git a/workhorse/internal/upload/exif/testdata/sample_exif.tiff b/workhorse/internal/upload/exif/testdata/sample_exif.tiff new file mode 100644 index 0000000000..6671d818ed Binary files /dev/null and b/workhorse/internal/upload/exif/testdata/sample_exif.tiff differ diff --git a/workhorse/internal/upload/exif/testdata/sample_exif_corrupted.jpg b/workhorse/internal/upload/exif/testdata/sample_exif_corrupted.jpg new file mode 100644 index 0000000000..3b5c692de5 Binary files /dev/null and b/workhorse/internal/upload/exif/testdata/sample_exif_corrupted.jpg differ diff --git a/workhorse/internal/upload/exif/testdata/sample_exif_invalid.jpg b/workhorse/internal/upload/exif/testdata/sample_exif_invalid.jpg new file mode 100644 index 0000000000..9f8a284c64 --- /dev/null +++ b/workhorse/internal/upload/exif/testdata/sample_exif_invalid.jpg @@ -0,0 +1 @@ +invalid data diff --git a/workhorse/internal/upload/rewrite.go b/workhorse/internal/upload/rewrite.go index e51604c6ed..ba6bd0e501 100644 --- a/workhorse/internal/upload/rewrite.go +++ b/workhorse/internal/upload/rewrite.go @@ -8,12 +8,15 @@ import ( "io/ioutil" "mime/multipart" "net/http" + "os" "strings" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "gitlab.com/gitlab-org/labkit/log" + "golang.org/x/image/tiff" + "gitlab.com/gitlab-org/gitlab-workhorse/internal/api" "gitlab.com/gitlab-org/gitlab-workhorse/internal/filestore" "gitlab.com/gitlab-org/gitlab-workhorse/internal/lsif_transformer/parser" @@ -122,9 +125,11 @@ func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipa var inputReader io.ReadCloser var err error + + imageType := exif.FileTypeFromSuffix(filename) switch { - case exif.IsExifFile(filename): - inputReader, err = handleExifUpload(ctx, p, filename) + case imageType != exif.TypeUnknown: + inputReader, err = handleExifUpload(ctx, p, filename, imageType) if err != nil { return err } @@ -164,12 +169,48 @@ func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipa return rew.filter.ProcessFile(ctx, name, fh, rew.writer) } -func handleExifUpload(ctx context.Context, r io.Reader, filename string) (io.ReadCloser, error) { +func handleExifUpload(ctx context.Context, r io.Reader, filename string, imageType exif.FileType) (io.ReadCloser, error) { + tmpfile, err := ioutil.TempFile("", "exifremove") + if err != nil { + return nil, err + } + go func() { + <-ctx.Done() + tmpfile.Close() + }() + if err := os.Remove(tmpfile.Name()); err != nil { + return nil, err + } + + _, err = io.Copy(tmpfile, r) + if err != nil { + return nil, err + } + + tmpfile.Seek(0, io.SeekStart) + isValidType := false + switch imageType { + case exif.TypeJPEG: + isValidType = isJPEG(tmpfile) + case exif.TypeTIFF: + isValidType = isTIFF(tmpfile) + } + + tmpfile.Seek(0, io.SeekStart) + if !isValidType { + log.WithContextFields(ctx, log.Fields{ + "filename": filename, + "imageType": imageType, + }).Print("invalid content type, not running exiftool") + + return tmpfile, nil + } + log.WithContextFields(ctx, log.Fields{ "filename": filename, }).Print("running exiftool to remove any metadata") - cleaner, err := exif.NewCleaner(ctx, r) + cleaner, err := exif.NewCleaner(ctx, tmpfile) if err != nil { return nil, err } @@ -177,6 +218,29 @@ func handleExifUpload(ctx context.Context, r io.Reader, filename string) (io.Rea return cleaner, nil } +func isTIFF(r io.Reader) bool { + _, err := tiff.Decode(r) + if err == nil { + return true + } + + if _, unsupported := err.(tiff.UnsupportedError); unsupported { + return true + } + + return false +} + +func isJPEG(r io.Reader) bool { + // Only the first 512 bytes are used to sniff the content type. + buf, err := ioutil.ReadAll(io.LimitReader(r, 512)) + if err != nil { + return false + } + + return http.DetectContentType(buf) == "image/jpeg" +} + func handleLsifUpload(ctx context.Context, reader io.Reader, tempPath, filename string, preauth *api.Response) (io.ReadCloser, error) { parserConfig := parser.Config{ TempPath: tempPath, diff --git a/workhorse/internal/upload/rewrite_test.go b/workhorse/internal/upload/rewrite_test.go new file mode 100644 index 0000000000..6fc41c3fef --- /dev/null +++ b/workhorse/internal/upload/rewrite_test.go @@ -0,0 +1,43 @@ +package upload + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestImageTypeRecongition(t *testing.T) { + tests := []struct { + filename string + isJPEG bool + isTIFF bool + }{ + { + filename: "exif/testdata/sample_exif.jpg", + isJPEG: true, + isTIFF: false, + }, { + filename: "exif/testdata/sample_exif.tiff", + isJPEG: false, + isTIFF: true, + }, { + filename: "exif/testdata/sample_exif_corrupted.jpg", + isJPEG: true, + isTIFF: false, + }, { + filename: "exif/testdata/sample_exif_invalid.jpg", + isJPEG: false, + isTIFF: false, + }, + } + + for _, test := range tests { + t.Run(test.filename, func(t *testing.T) { + input, err := os.Open(test.filename) + require.NoError(t, err) + require.Equal(t, test.isJPEG, isJPEG(input)) + require.Equal(t, test.isTIFF, isTIFF(input)) + }) + } +} diff --git a/workhorse/internal/upload/uploads_test.go b/workhorse/internal/upload/uploads_test.go index fc1a1ac57e..0885f31d5a 100644 --- a/workhorse/internal/upload/uploads_test.go +++ b/workhorse/internal/upload/uploads_test.go @@ -358,26 +358,10 @@ func TestInvalidFileNames(t *testing.T) { } func TestUploadHandlerRemovingExif(t *testing.T) { - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) - defer os.RemoveAll(tempPath) - - var buffer bytes.Buffer - content, err := ioutil.ReadFile("exif/testdata/sample_exif.jpg") require.NoError(t, err) - writer := multipart.NewWriter(&buffer) - file, err := writer.CreateFormFile("file", "test.jpg") - require.NoError(t, err) - - _, err = file.Write(content) - require.NoError(t, err) - - err = writer.Close() - require.NoError(t, err) - - ts := testhelper.TestServerWithHandler(regexp.MustCompile(`/url/path\z`), func(w http.ResponseWriter, r *http.Request) { + runUploadTest(t, content, "sample_exif.jpg", 200, func(w http.ResponseWriter, r *http.Request) { err := r.ParseMultipartForm(100000) require.NoError(t, err) @@ -389,30 +373,54 @@ func TestUploadHandlerRemovingExif(t *testing.T) { w.WriteHeader(200) fmt.Fprint(w, "RESPONSE") }) - defer ts.Close() - - httpRequest, err := http.NewRequest("POST", ts.URL+"/url/path", &buffer) - require.NoError(t, err) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - httpRequest = httpRequest.WithContext(ctx) - httpRequest.ContentLength = int64(buffer.Len()) - httpRequest.Header.Set("Content-Type", writer.FormDataContentType()) - response := httptest.NewRecorder() - - handler := newProxy(ts.URL) - apiResponse := &api.Response{TempPath: tempPath} - preparer := &DefaultPreparer{} - opts, _, err := preparer.Prepare(apiResponse) - require.NoError(t, err) - - HandleFileUploads(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts) - require.Equal(t, 200, response.Code) } -func TestUploadHandlerRemovingInvalidExif(t *testing.T) { +func TestUploadHandlerRemovingExifTiff(t *testing.T) { + content, err := ioutil.ReadFile("exif/testdata/sample_exif.tiff") + require.NoError(t, err) + + runUploadTest(t, content, "sample_exif.tiff", 200, func(w http.ResponseWriter, r *http.Request) { + err := r.ParseMultipartForm(100000) + require.NoError(t, err) + + size, err := strconv.Atoi(r.FormValue("file.size")) + require.NoError(t, err) + require.True(t, size < len(content), "Expected the file to be smaller after removal of exif") + require.True(t, size > 0, "Expected to receive not empty file") + + w.WriteHeader(200) + fmt.Fprint(w, "RESPONSE") + }) +} + +func TestUploadHandlerRemovingExifInvalidContentType(t *testing.T) { + content, err := ioutil.ReadFile("exif/testdata/sample_exif_invalid.jpg") + require.NoError(t, err) + + runUploadTest(t, content, "sample_exif_invalid.jpg", 200, func(w http.ResponseWriter, r *http.Request) { + err := r.ParseMultipartForm(100000) + require.NoError(t, err) + + output, err := ioutil.ReadFile(r.FormValue("file.path")) + require.NoError(t, err) + require.Equal(t, content, output, "Expected the file to be same as before") + + w.WriteHeader(200) + fmt.Fprint(w, "RESPONSE") + }) +} + +func TestUploadHandlerRemovingExifCorruptedFile(t *testing.T) { + content, err := ioutil.ReadFile("exif/testdata/sample_exif_corrupted.jpg") + require.NoError(t, err) + + runUploadTest(t, content, "sample_exif_corrupted.jpg", 422, func(w http.ResponseWriter, r *http.Request) { + err := r.ParseMultipartForm(100000) + require.Error(t, err) + }) +} + +func runUploadTest(t *testing.T, image []byte, filename string, httpCode int, tsHandler func(http.ResponseWriter, *http.Request)) { tempPath, err := ioutil.TempDir("", "uploads") require.NoError(t, err) defer os.RemoveAll(tempPath) @@ -420,17 +428,16 @@ func TestUploadHandlerRemovingInvalidExif(t *testing.T) { var buffer bytes.Buffer writer := multipart.NewWriter(&buffer) - file, err := writer.CreateFormFile("file", "test.jpg") + file, err := writer.CreateFormFile("file", filename) + require.NoError(t, err) + + _, err = file.Write(image) require.NoError(t, err) - fmt.Fprint(file, "this is not valid image data") err = writer.Close() require.NoError(t, err) - ts := testhelper.TestServerWithHandler(regexp.MustCompile(`/url/path\z`), func(w http.ResponseWriter, r *http.Request) { - err := r.ParseMultipartForm(100000) - require.Error(t, err) - }) + ts := testhelper.TestServerWithHandler(regexp.MustCompile(`/url/path\z`), tsHandler) defer ts.Close() httpRequest, err := http.NewRequest("POST", ts.URL+"/url/path", &buffer) @@ -451,7 +458,7 @@ func TestUploadHandlerRemovingInvalidExif(t *testing.T) { require.NoError(t, err) HandleFileUploads(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts) - require.Equal(t, 422, response.Code) + require.Equal(t, httpCode, response.Code) } func newProxy(url string) *proxy.Proxy {