Add latest changes from gitlab-org/security/gitlab@14-6-stable-ee

16 files changed, 67 insertions, 177 deletions

diff --git a/app/assets/javascripts/create_item_dropdown.js b/app/assets/javascripts/create_item_dropdown.js
index 1472adf458b..b39720c6094 100644
--- a/app/assets/javascripts/create_item_dropdown.js
+++ b/app/assets/javascripts/create_item_dropdown.js
@@ -1,4 +1,3 @@
-import { escape } from 'lodash';
 import initDeprecatedJQueryDropdown from '~/deprecated_jquery_dropdown';
 
 export default class CreateItemDropdown {
@@ -37,14 +36,14 @@ export default class CreateItemDropdown {
       },
       selectable: true,
       toggleLabel(selected) {
-        return selected && 'id' in selected ? escape(selected.title) : this.defaultToggleLabel;
+        return selected && 'id' in selected ? selected.title : this.defaultToggleLabel;
       },
       fieldName: this.fieldName,
       text(item) {
-        return escape(item.text);
+        return item.text;
       },
       id(item) {
-        return escape(item.id);
+        return item.id;
       },
       onFilter: this.toggleCreateNewButton.bind(this),
       clicked: (options) => {
diff --git a/app/services/concerns/protected_ref_name_sanitizer.rb b/app/services/concerns/protected_ref_name_sanitizer.rb
deleted file mode 100644
index 3966c410fec..00000000000
--- a/app/services/concerns/protected_ref_name_sanitizer.rb
+++ /dev/null
@@ -1,12 +0,0 @@
-# frozen_string_literal: true
-
-module ProtectedRefNameSanitizer
-  def sanitize_name(name)
-    name = CGI.unescapeHTML(name)
-    name = Sanitize.fragment(name)
-
-    # Sanitize.fragment escapes HTML chars, so unescape again to allow names
-    # like `feature->master`
-    CGI.unescapeHTML(name)
-  end
-end
diff --git a/app/services/protected_branches/base_service.rb b/app/services/protected_branches/base_service.rb
index 1ab3ccfcaae..f48e02ab4b5 100644
--- a/app/services/protected_branches/base_service.rb
+++ b/app/services/protected_branches/base_service.rb
@@ -2,8 +2,6 @@
 
 module ProtectedBranches
   class BaseService < ::BaseService
-    include ProtectedRefNameSanitizer
-
     # current_user - The user that performs the action
     # params - A hash of parameters
     def initialize(project, current_user = nil, params = {})
@@ -15,14 +13,5 @@ module ProtectedBranches
     def after_execute(*)
       # overridden in EE::ProtectedBranches module
     end
-
-    private
-
-    def filtered_params
-      return unless params
-
-      params[:name] = sanitize_name(params[:name]) if params[:name].present?
-      params
-    end
   end
 end
diff --git a/app/services/protected_branches/create_service.rb b/app/services/protected_branches/create_service.rb
index ea494dd4426..dada449989a 100644
--- a/app/services/protected_branches/create_service.rb
+++ b/app/services/protected_branches/create_service.rb
@@ -21,7 +21,7 @@ module ProtectedBranches
     end
 
     def protected_branch
-      @protected_branch ||= project.protected_branches.new(filtered_params)
+      @protected_branch ||= project.protected_branches.new(params)
     end
   end
 end
diff --git a/app/services/protected_branches/update_service.rb b/app/services/protected_branches/update_service.rb
index 40e9a286af9..1e70f2d9793 100644
--- a/app/services/protected_branches/update_service.rb
+++ b/app/services/protected_branches/update_service.rb
@@ -8,7 +8,7 @@ module ProtectedBranches
       old_merge_access_levels = protected_branch.merge_access_levels.map(&:clone)
       old_push_access_levels = protected_branch.push_access_levels.map(&:clone)
 
-      if protected_branch.update(filtered_params)
+      if protected_branch.update(params)
         after_execute(protected_branch: protected_branch, old_merge_access_levels: old_merge_access_levels, old_push_access_levels: old_push_access_levels)
       end
 
diff --git a/app/services/protected_tags/base_service.rb b/app/services/protected_tags/base_service.rb
deleted file mode 100644
index e0181815f0f..00000000000
--- a/app/services/protected_tags/base_service.rb
+++ /dev/null
@@ -1,16 +0,0 @@
-# frozen_string_literal: true
-
-module ProtectedTags
-  class BaseService < ::BaseService
-    include ProtectedRefNameSanitizer
-
-    private
-
-    def filtered_params
-      return unless params
-
-      params[:name] = sanitize_name(params[:name]) if params[:name].present?
-      params
-    end
-  end
-end
diff --git a/app/services/protected_tags/create_service.rb b/app/services/protected_tags/create_service.rb
index 7d2b583a295..65303f21a4a 100644
--- a/app/services/protected_tags/create_service.rb
+++ b/app/services/protected_tags/create_service.rb
@@ -1,13 +1,13 @@
 # frozen_string_literal: true
 
 module ProtectedTags
-  class CreateService < ProtectedTags::BaseService
+  class CreateService < ::BaseService
     attr_reader :protected_tag
 
     def execute
       raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project)
 
-      project.protected_tags.create(filtered_params)
+      project.protected_tags.create(params)
     end
   end
 end
diff --git a/app/services/protected_tags/update_service.rb b/app/services/protected_tags/update_service.rb
index e337ec39898..283aa8882c5 100644
--- a/app/services/protected_tags/update_service.rb
+++ b/app/services/protected_tags/update_service.rb
@@ -1,11 +1,11 @@
 # frozen_string_literal: true
 
 module ProtectedTags
-  class UpdateService < ProtectedTags::BaseService
+  class UpdateService < ::BaseService
     def execute(protected_tag)
       raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project)
 
-      protected_tag.update(filtered_params)
+      protected_tag.update(params)
       protected_tag
     end
   end
diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb
index f092e03046a..48228ede684 100644
--- a/lib/gitlab/url_blocker.rb
+++ b/lib/gitlab/url_blocker.rb
@@ -148,9 +148,17 @@ module Gitlab
         unless allow_local_network
           validate_local_network(address_info)
           validate_link_local(address_info)
+          validate_shared_address(address_info)
         end
       end
 
+      def validate_shared_address(addrs_info)
+        netmask = IPAddr.new('100.64.0.0/10')
+        return unless addrs_info.any? { |addr| netmask.include?(addr.ip_address) }
+
+        raise BlockedUrlError, "Requests to the shared address space are not allowed"
+      end
+
       def get_port(uri)
         uri.port || uri.default_port
       end
diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb
index 15ec11c256f..05070e3afdf 100644
--- a/spec/features/protected_branches_spec.rb
+++ b/spec/features/protected_branches_spec.rb
@@ -53,6 +53,17 @@ RSpec.describe 'Protected Branches', :js do
       sign_in(user)
     end
 
+    it 'allows to create a protected branch with name containing HTML tags' do
+      visit project_protected_branches_path(project)
+      set_defaults
+      set_protected_branch_name('foo<b>bar<\b>')
+      click_on "Protect"
+
+      within(".protected-branches-list") { expect(page).to have_content('foo<b>bar<\b>') }
+      expect(ProtectedBranch.count).to eq(1)
+      expect(ProtectedBranch.last.name).to eq('foo<b>bar<\b>')
+    end
+
     describe 'Delete protected branch' do
       before do
         create(:protected_branch, project: project, name: 'fix')
diff --git a/spec/frontend/create_item_dropdown_spec.js b/spec/frontend/create_item_dropdown_spec.js
index 56c09cd731e..143ccb9b930 100644
--- a/spec/frontend/create_item_dropdown_spec.js
+++ b/spec/frontend/create_item_dropdown_spec.js
@@ -17,6 +17,11 @@ const DROPDOWN_ITEM_DATA = [
     id: 'three',
     text: 'three',
   },
+  {
+    title: '<b>four</b>title',
+    id: '<b>four</b>id',
+    text: '<b>four</b>text',
+  },
 ];
 
 describe('CreateItemDropdown', () => {
@@ -63,6 +68,10 @@ describe('CreateItemDropdown', () => {
       const $itemEls = $wrapperEl.find('.js-dropdown-content a');
 
       expect($itemEls.length).toEqual(DROPDOWN_ITEM_DATA.length);
+
+      DROPDOWN_ITEM_DATA.forEach((dataItem, i) => {
+        expect($($itemEls[i]).text()).toEqual(dataItem.text);
+      });
     });
   });
 
@@ -177,7 +186,7 @@ describe('CreateItemDropdown', () => {
       const $itemEls = $wrapperEl.find('.js-dropdown-content a');
 
       expect($itemEls.length).toEqual(1 + DROPDOWN_ITEM_DATA.length);
-      expect($($itemEls[3]).text()).toEqual('new-item-text');
+      expect($($itemEls[DROPDOWN_ITEM_DATA.length]).text()).toEqual('new-item-text');
       expect($wrapperEl.find('.dropdown-toggle-text').text()).toEqual('new-item-title');
     });
   });
diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb
index 0713475d59b..5b77290ce2e 100644
--- a/spec/lib/gitlab/url_blocker_spec.rb
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -279,6 +279,8 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do
     end
 
     context 'when allow_local_network is' do
+      let(:shared_address_space_ips) { ['100.64.0.0', '100.64.127.127', '100.64.255.255'] }
+
       let(:local_ips) do
         [
           '192.168.1.2',
@@ -292,7 +294,8 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do
           '[::ffff:ac10:20]',
           '[feef::1]',
           '[fee2::]',
-          '[fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa]'
+          '[fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa]',
+          *shared_address_space_ips
         ]
       end
 
@@ -385,18 +388,7 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do
                 '127.0.0.1',
                 '127.0.0.2',
                 '192.168.1.1',
-                '192.168.1.2',
-                '0:0:0:0:0:ffff:192.168.1.2',
-                '::ffff:c0a8:102',
-                '10.0.0.2',
-                '0:0:0:0:0:ffff:10.0.0.2',
-                '::ffff:a00:2',
-                '172.16.0.2',
-                '0:0:0:0:0:ffff:172.16.0.2',
-                '::ffff:ac10:20',
-                'feef::1',
-                'fee2::',
-                'fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa',
+                *local_ips,
                 '0:0:0:0:0:ffff:169.254.169.254',
                 '::ffff:a9fe:a9fe',
                 '::ffff:169.254.168.100',
diff --git a/spec/services/protected_branches/create_service_spec.rb b/spec/services/protected_branches/create_service_spec.rb
index 756c775be9b..707e87d2477 100644
--- a/spec/services/protected_branches/create_service_spec.rb
+++ b/spec/services/protected_branches/create_service_spec.rb
@@ -24,38 +24,14 @@ RSpec.describe ProtectedBranches::CreateService do
       expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER])
     end
 
-    context 'when name has escaped HTML' do
-      let(:name) { 'feature-&gt;test' }
+    context 'when protecting a branch with a name that contains HTML tags' do
+      let(:name) { 'foo<b>bar<\b>' }
 
-      it 'creates the new protected branch matching the unescaped version' do
-        expect { service.execute }.to change(ProtectedBranch, :count).by(1)
-        expect(project.protected_branches.last.name).to eq('feature->test')
-      end
-
-      context 'and name contains HTML tags' do
-        let(:name) { '&lt;b&gt;master&lt;/b&gt;' }
-
-        it 'creates the new protected branch with sanitized name' do
-          expect { service.execute }.to change(ProtectedBranch, :count).by(1)
-          expect(project.protected_branches.last.name).to eq('master')
-        end
-
-        context 'and contains unsafe HTML' do
-          let(:name) { '&lt;script&gt;alert(&#39;foo&#39;);&lt;/script&gt;' }
+      subject(:service) { described_class.new(project, user, params) }
 
-          it 'does not create the new protected branch' do
-            expect { service.execute }.not_to change(ProtectedBranch, :count)
-          end
-        end
-      end
-
-      context 'when name contains unescaped HTML tags' do
-        let(:name) { '<b>master</b>' }
-
-        it 'creates the new protected branch with sanitized name' do
-          expect { service.execute }.to change(ProtectedBranch, :count).by(1)
-          expect(project.protected_branches.last.name).to eq('master')
-        end
+      it 'creates a new protected branch' do
+        expect { service.execute }.to change(ProtectedBranch, :count).by(1)
+        expect(project.protected_branches.last.name).to eq(name)
       end
     end
 
diff --git a/spec/services/protected_branches/update_service_spec.rb b/spec/services/protected_branches/update_service_spec.rb
index b5cf1a54aff..441eea9d7dd 100644
--- a/spec/services/protected_branches/update_service_spec.rb
+++ b/spec/services/protected_branches/update_service_spec.rb
@@ -18,35 +18,14 @@ RSpec.describe ProtectedBranches::UpdateService do
       expect(result.reload.name).to eq(params[:name])
     end
 
-    context 'when name has escaped HTML' do
-      let(:new_name) { 'feature-&gt;test' }
+    context 'when updating name of a protected branch to one that contains HTML tags' do
+      let(:new_name) { 'foo<b>bar<\b>' }
+      let(:result) { service.execute(protected_branch) }
 
-      it 'updates protected branch name with unescaped HTML' do
-        expect(result.reload.name).to eq('feature->test')
-      end
-
-      context 'and name contains HTML tags' do
-        let(:new_name) { '&lt;b&gt;master&lt;/b&gt;' }
-
-        it 'updates protected branch name with sanitized name' do
-          expect(result.reload.name).to eq('master')
-        end
-
-        context 'and contains unsafe HTML' do
-          let(:new_name) { '&lt;script&gt;alert(&#39;foo&#39;);&lt;/script&gt;' }
-
-          it 'does not update the protected branch' do
-            expect(result.reload.name).to eq(protected_branch.name)
-          end
-        end
-      end
-    end
-
-    context 'when name contains unescaped HTML tags' do
-      let(:new_name) { '<b>master</b>' }
+      subject(:service) { described_class.new(project, user, params) }
 
-      it 'updates protected branch name with sanitized name' do
-        expect(result.reload.name).to eq('master')
+      it 'updates a protected branch' do
+        expect(result.reload.name).to eq(new_name)
       end
     end
 
diff --git a/spec/services/protected_tags/create_service_spec.rb b/spec/services/protected_tags/create_service_spec.rb
index 3d06cc9fb6c..8ac59990a3a 100644
--- a/spec/services/protected_tags/create_service_spec.rb
+++ b/spec/services/protected_tags/create_service_spec.rb
@@ -22,38 +22,14 @@ RSpec.describe ProtectedTags::CreateService do
       expect(project.protected_tags.last.create_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER])
     end
 
-    context 'when name has escaped HTML' do
-      let(:name) { 'tag-&gt;test' }
+    context 'protecting a tag with a name that contains HTML tags' do
+      let(:name) { 'foo<b>bar<\b>' }
 
-      it 'creates the new protected tag matching the unescaped version' do
-        expect { service.execute }.to change(ProtectedTag, :count).by(1)
-        expect(project.protected_tags.last.name).to eq('tag->test')
-      end
-
-      context 'and name contains HTML tags' do
-        let(:name) { '&lt;b&gt;tag&lt;/b&gt;' }
-
-        it 'creates the new protected tag with sanitized name' do
-          expect { service.execute }.to change(ProtectedTag, :count).by(1)
-          expect(project.protected_tags.last.name).to eq('tag')
-        end
-
-        context 'and contains unsafe HTML' do
-          let(:name) { '&lt;script&gt;alert(&#39;foo&#39;);&lt;/script&gt;' }
+      subject(:service) { described_class.new(project, user, params) }
 
-          it 'does not create the new protected tag' do
-            expect { service.execute }.not_to change(ProtectedTag, :count)
-          end
-        end
-      end
-
-      context 'when name contains unescaped HTML tags' do
-        let(:name) { '<b>tag</b>' }
-
-        it 'creates the new protected tag with sanitized name' do
-          expect { service.execute }.to change(ProtectedTag, :count).by(1)
-          expect(project.protected_tags.last.name).to eq('tag')
-        end
+      it 'creates a new protected tag' do
+        expect { service.execute }.to change(ProtectedTag, :count).by(1)
+        expect(project.protected_tags.last.name).to eq(name)
       end
     end
   end
diff --git a/spec/services/protected_tags/update_service_spec.rb b/spec/services/protected_tags/update_service_spec.rb
index 22005bb9b89..d98272ad8bf 100644
--- a/spec/services/protected_tags/update_service_spec.rb
+++ b/spec/services/protected_tags/update_service_spec.rb
@@ -18,35 +18,14 @@ RSpec.describe ProtectedTags::UpdateService do
       expect(result.reload.name).to eq(params[:name])
     end
 
-    context 'when name has escaped HTML' do
-      let(:new_name) { 'tag-&gt;test' }
+    context 'when updating protected tag with a name that contains HTML tags' do
+      let(:new_name) { 'foo<b>bar<\b>' }
+      let(:result) { service.execute(protected_tag) }
 
-      it 'updates protected tag name with unescaped HTML' do
-        expect(result.reload.name).to eq('tag->test')
-      end
-
-      context 'and name contains HTML tags' do
-        let(:new_name) { '&lt;b&gt;tag&lt;/b&gt;' }
-
-        it 'updates protected tag name with sanitized name' do
-          expect(result.reload.name).to eq('tag')
-        end
-
-        context 'and contains unsafe HTML' do
-          let(:new_name) { '&lt;script&gt;alert(&#39;foo&#39;);&lt;/script&gt;' }
-
-          it 'does not update the protected tag' do
-            expect(result.reload.name).to eq(protected_tag.name)
-          end
-        end
-      end
-    end
-
-    context 'when name contains unescaped HTML tags' do
-      let(:new_name) { '<b>tag</b>' }
+      subject(:service) { described_class.new(project, user, params) }
 
-      it 'updates protected tag name with sanitized name' do
-        expect(result.reload.name).to eq('tag')
+      it 'updates a protected tag' do
+        expect(result.reload.name).to eq(new_name)
       end
     end