From ebf9d0c4e33ea1c7058c0d9b9121e6a8d03f034f Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 7 Nov 2017 08:30:38 +0000 Subject: Merge branch 'ssrf-protections-round-2' into 'security-10-1' Replace SSRF resolver with Addrinfo.getaddrinfo to include alternative localhost versions See merge request gitlab/gitlabhq!2219 (cherry picked from commit 4a1e73783d5480aa514db7b53e10c075f95580b5) 1bffa0c3 Replace SSRF resolver with Addrinfo.getaddrinfo to include alternative localhost versions --- lib/gitlab/url_blocker.rb | 4 +++- spec/lib/gitlab/url_blocker_spec.rb | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb index fee1a127fd7..13150ddab67 100644 --- a/lib/gitlab/url_blocker.rb +++ b/lib/gitlab/url_blocker.rb @@ -22,10 +22,12 @@ module Gitlab return true if blocked_user_or_hostname?(uri.user) return true if blocked_user_or_hostname?(uri.hostname) - server_ips = Resolv.getaddresses(uri.hostname) + server_ips = Addrinfo.getaddrinfo(uri.hostname, 80, nil, :STREAM).map(&:ip_address) return true if (blocked_ips & server_ips).any? rescue Addressable::URI::InvalidURIError return true + rescue SocketError + return false end false diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index f18823b61ef..d9b3c2350b1 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -20,6 +20,22 @@ describe Gitlab::UrlBlocker do expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git')).to be true end + it 'returns true for alternative version of 127.0.0.1 (0177.1)' do + expect(described_class.blocked_url?('https://0177.1:65535/foo/foo.git')).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (0x7f.1)' do + expect(described_class.blocked_url?('https://0x7f.1:65535/foo/foo.git')).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (2130706433)' do + expect(described_class.blocked_url?('https://2130706433:65535/foo/foo.git')).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (127.000.000.001)' do + expect(described_class.blocked_url?('https://127.000.000.001:65535/foo/foo.git')).to be true + end + it 'returns true for a non-alphanumeric hostname' do stub_resolv -- cgit v1.2.1 From 7521d0cb07f70a1193146b7c7c7556d4f25b35aa Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 1 Nov 2017 09:25:49 +0000 Subject: Merge branch '36099-api-responses-missing-x-content-type-options-header' into '10-1-stable' Include X-Content-Type-Options (XCTO) header into API responses See merge request gitlab/gitlabhq!2211 (cherry picked from commit 6c818e77f2abeef2dd7b17a269611b018701fa79) e087e075 Include X-Content-Type-Options (XCTO) header into API responses --- lib/api/api.rb | 5 ++++- spec/requests/api/projects_spec.rb | 6 ++++++ spec/support/matchers/security_header_matcher.rb | 5 +++++ 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 spec/support/matchers/security_header_matcher.rb diff --git a/lib/api/api.rb b/lib/api/api.rb index 79e55a2f4f7..1664197689d 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -57,7 +57,10 @@ module API mount ::API::V3::Variables end - before { header['X-Frame-Options'] = 'SAMEORIGIN' } + before do + header['X-Frame-Options'] = 'SAMEORIGIN' + header['X-Content-Type-Options'] = 'nosniff' + end # The locale is set to the current user's locale when `current_user` is loaded after { Gitlab::I18n.use_default_locale } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 5964244f8c5..2e3416cb74d 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -50,6 +50,12 @@ describe API::Projects do expect(json_response).to be_an Array expect(json_response.map { |p| p['id'] }).to contain_exactly(*projects.map(&:id)) end + + it 'returns the proper security headers' do + get api('/projects', current_user), filter + + expect(response).to include_security_headers + end end shared_examples_for 'projects response without N + 1 queries' do diff --git a/spec/support/matchers/security_header_matcher.rb b/spec/support/matchers/security_header_matcher.rb new file mode 100644 index 00000000000..f8518d13ebb --- /dev/null +++ b/spec/support/matchers/security_header_matcher.rb @@ -0,0 +1,5 @@ +RSpec::Matchers.define :include_security_headers do |expected| + match do |actual| + expect(actual.headers).to include('X-Content-Type-Options') + end +end -- cgit v1.2.1 From 1bc43e657bf116bbeec2e0e2e8664cdf6261a4c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 7 Nov 2017 14:56:53 +0000 Subject: Merge branch 'fix-mysql-grant-check' into 'master' Fix TRIGGER checks for MySQL Closes #38372 See merge request gitlab-org/gitlab-ce!15226 (cherry picked from commit d45fef88f7f0aa249893f9f151185eac5b9bb870) --- changelogs/unreleased/fix-mysql-grant-check.yml | 5 +++++ lib/gitlab/database/grant.rb | 30 ++++++++++++++++--------- spec/lib/gitlab/database/grant_spec.rb | 22 +++++------------- 3 files changed, 29 insertions(+), 28 deletions(-) create mode 100644 changelogs/unreleased/fix-mysql-grant-check.yml diff --git a/changelogs/unreleased/fix-mysql-grant-check.yml b/changelogs/unreleased/fix-mysql-grant-check.yml new file mode 100644 index 00000000000..a1c1aa67d79 --- /dev/null +++ b/changelogs/unreleased/fix-mysql-grant-check.yml @@ -0,0 +1,5 @@ +--- +title: Fix TRIGGER checks for MySQL +merge_request: +author: +type: fixed diff --git a/lib/gitlab/database/grant.rb b/lib/gitlab/database/grant.rb index aee3981e79a..9f76967fc77 100644 --- a/lib/gitlab/database/grant.rb +++ b/lib/gitlab/database/grant.rb @@ -6,28 +6,36 @@ module Gitlab if Database.postgresql? 'information_schema.role_table_grants' else - 'mysql.user' + 'information_schema.schema_privileges' end - def self.scope_to_current_user - if Database.postgresql? - where('grantee = user') - else - where("CONCAT(User, '@', Host) = current_user()") - end - end - # Returns true if the current user can create and execute triggers on the # given table. def self.create_and_execute_trigger?(table) priv = if Database.postgresql? where(privilege_type: 'TRIGGER', table_name: table) + .where('grantee = user') else - where(Trigger_priv: 'Y') + queries = [ + Grant.select(1) + .from('information_schema.user_privileges') + .where("PRIVILEGE_TYPE = 'SUPER'") + .where("GRANTEE = CONCAT('\\'', REPLACE(CURRENT_USER(), '@', '\\'@\\''), '\\'')"), + + Grant.select(1) + .from('information_schema.schema_privileges') + .where("PRIVILEGE_TYPE = 'TRIGGER'") + .where('TABLE_SCHEMA = ?', Gitlab::Database.database_name) + .where("GRANTEE = CONCAT('\\'', REPLACE(CURRENT_USER(), '@', '\\'@\\''), '\\'')") + ] + + union = SQL::Union.new(queries).to_sql + + Grant.from("(#{union}) privs") end - priv.scope_to_current_user.any? + priv.any? end end end diff --git a/spec/lib/gitlab/database/grant_spec.rb b/spec/lib/gitlab/database/grant_spec.rb index 651da3e8476..5ebf3f399b6 100644 --- a/spec/lib/gitlab/database/grant_spec.rb +++ b/spec/lib/gitlab/database/grant_spec.rb @@ -1,16 +1,6 @@ require 'spec_helper' describe Gitlab::Database::Grant do - describe '.scope_to_current_user' do - it 'scopes the relation to the current user' do - user = Gitlab::Database.username - column = Gitlab::Database.postgresql? ? :grantee : :User - names = described_class.scope_to_current_user.pluck(column).uniq - - expect(names).to eq([user]) - end - end - describe '.create_and_execute_trigger' do it 'returns true when the user can create and execute a trigger' do # We assume the DB/user is set up correctly so that triggers can be @@ -18,13 +8,11 @@ describe Gitlab::Database::Grant do expect(described_class.create_and_execute_trigger?('users')).to eq(true) end - it 'returns false when the user can not create and/or execute a trigger' do - allow(described_class).to receive(:scope_to_current_user) - .and_return(described_class.none) - - result = described_class.create_and_execute_trigger?('kittens') - - expect(result).to eq(false) + it 'returns false when the user can not create and/or execute a trigger', :postgresql do + # In case of MySQL the user may have SUPER permissions, making it + # impossible to have `false` returned when running tests; hence we only + # run these tests on PostgreSQL. + expect(described_class.create_and_execute_trigger?('foo')).to eq(false) end end end -- cgit v1.2.1 From 567d8a636ce93698f171e97d1b73219bd45deb4c Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 8 Nov 2017 17:49:24 +0800 Subject: Update CHANGELOG.md for 10.1.2 [ci skip] --- CHANGELOG.md | 4 ++++ changelogs/unreleased/fix-mysql-grant-check.yml | 5 ----- 2 files changed, 4 insertions(+), 5 deletions(-) delete mode 100644 changelogs/unreleased/fix-mysql-grant-check.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e9515c1da9..30ab05164c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 10.1.2 (2017-11-08) + +- [FIXED] Fix TRIGGER checks for MySQL. + ## 10.1.1 (2017-10-31) - [FIXED] Auto Devops kubernetes default namespace is now correctly built out of gitlab project group-name. !14642 (Mircea Danila Dumitrescu) diff --git a/changelogs/unreleased/fix-mysql-grant-check.yml b/changelogs/unreleased/fix-mysql-grant-check.yml deleted file mode 100644 index a1c1aa67d79..00000000000 --- a/changelogs/unreleased/fix-mysql-grant-check.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix TRIGGER checks for MySQL -merge_request: -author: -type: fixed -- cgit v1.2.1 From af60a6ccb7196df571aed3bf6c1d09a072bad157 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 8 Nov 2017 17:49:53 +0800 Subject: Update VERSION to 10.1.2 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 23127993ac0..b6132546fce 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -10.1.1 +10.1.2 -- cgit v1.2.1