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

7 files changed, 74 insertions, 4 deletions

diff --git a/app/controllers/concerns/page_limiter.rb b/app/controllers/concerns/page_limiter.rb
index 362b02e5856..3e75c428fdd 100644
--- a/app/controllers/concerns/page_limiter.rb
+++ b/app/controllers/concerns/page_limiter.rb
@@ -57,7 +57,7 @@ module PageLimiter
 
   # Record the page limit being hit in Prometheus
   def record_page_limit_interception
-    dd = DeviceDetector.new(request.user_agent)
+    dd = Gitlab::SafeDeviceDetector.new(request.user_agent)
 
     Gitlab::Metrics.counter(:gitlab_page_out_of_bounds,
       controller: params[:controller],
diff --git a/app/models/active_session.rb b/app/models/active_session.rb
index b16c4a2b353..2d1dec1977d 100644
--- a/app/models/active_session.rb
+++ b/app/models/active_session.rb
@@ -67,7 +67,7 @@ class ActiveSession
   def self.set(user, request)
     Gitlab::Redis::Sessions.with do |redis|
       session_private_id = request.session.id.private_id
-      client = DeviceDetector.new(request.user_agent)
+      client = Gitlab::SafeDeviceDetector.new(request.user_agent)
       timestamp = Time.current
       expiry = Settings.gitlab['session_expire_delay'] * 60
 
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index 946cdda2e75..639fe138a46 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -41,6 +41,7 @@ class WebHook < ApplicationRecord
   after_initialize :initialize_url_variables
 
   before_validation :reset_token
+  before_validation :reset_url_variables, unless: ->(hook) { hook.is_a?(ServiceHook) }
   before_validation :set_branch_filter_nil, \
     if: -> { branch_filter_strategy_all_branches? && enhanced_webhook_support_regex? }
   validates :push_events_branch_filter, \
@@ -224,6 +225,10 @@ class WebHook < ApplicationRecord
     self.token = nil if url_changed? && !encrypted_token_changed?
   end
 
+  def reset_url_variables
+    self.url_variables = {} if url_changed? && !encrypted_url_variables_changed?
+  end
+
   def next_failure_count
     recent_failures.succ.clamp(1, MAX_FAILURES)
   end
diff --git a/lib/gitlab/safe_device_detector.rb b/lib/gitlab/safe_device_detector.rb
new file mode 100644
index 00000000000..ba1010ae5e4
--- /dev/null
+++ b/lib/gitlab/safe_device_detector.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+# rubocop:disable Gitlab/NamespacedClass
+require 'device_detector'
+
+module Gitlab
+  class SafeDeviceDetector < ::DeviceDetector
+    USER_AGENT_MAX_SIZE = 1024
+
+    def initialize(user_agent)
+      super(user_agent)
+      @user_agent = user_agent && user_agent[0..USER_AGENT_MAX_SIZE]
+    end
+  end
+end
+
+# rubocop:enable Gitlab/NamespacedClass
diff --git a/spec/lib/gitlab/safe_device_detector_spec.rb b/spec/lib/gitlab/safe_device_detector_spec.rb
new file mode 100644
index 00000000000..c37dc1e1c7e
--- /dev/null
+++ b/spec/lib/gitlab/safe_device_detector_spec.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'device_detector'
+require_relative '../../../lib/gitlab/safe_device_detector'
+
+RSpec.describe Gitlab::SafeDeviceDetector, feature_category: :authentication_and_authorization do
+  it 'retains the behavior for normal user agents' do
+    chrome_user_agent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 \
+    (KHTML, like Gecko) Chrome/108.0.0.0 Safari/537.36"
+
+    expect(described_class.new(chrome_user_agent).user_agent).to be_eql(chrome_user_agent)
+    expect(described_class.new(chrome_user_agent).name).to be_eql('Chrome')
+  end
+
+  it 'truncates big user agents' do
+    big_user_agent = "chrome #{'abc' * 1024}"
+    expect(described_class.new(big_user_agent).user_agent).not_to be_eql(big_user_agent)
+  end
+end
diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb
index 9b55db15f3b..54d81208860 100644
--- a/spec/models/hooks/web_hook_spec.rb
+++ b/spec/models/hooks/web_hook_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe WebHook do
+RSpec.describe WebHook, feature_category: :integrations do
   include AfterNextHelpers
 
   let_it_be(:project) { create(:project) }
@@ -225,6 +225,32 @@ RSpec.describe WebHook do
       end
     end
 
+    describe 'before_validation :reset_url_variables' do
+      subject(:hook) { build_stubbed(:project_hook, :url_variables, project: project, url: 'http://example.com/{abc}') }
+
+      it 'resets url variables if url changed' do
+        hook.url = 'http://example.com/new-hook'
+
+        expect(hook).to be_valid
+        expect(hook.url_variables).to eq({})
+      end
+
+      it 'resets url variables if url is changed but url variables stayed the same' do
+        hook.url = 'http://test.example.com/{abc}'
+
+        expect(hook).not_to be_valid
+        expect(hook.url_variables).to eq({})
+      end
+
+      it 'does not reset url variables if both url and url variables are changed' do
+        hook.url = 'http://example.com/{one}/{two}'
+        hook.url_variables = { 'one' => 'foo', 'two' => 'bar' }
+
+        expect(hook).to be_valid
+        expect(hook.url_variables).to eq({ 'one' => 'foo', 'two' => 'bar' })
+      end
+    end
+
     it "only consider these branch filter strategies are valid" do
       expected_valid_types = %w[all_branches regex wildcard]
       expect(described_class.branch_filter_strategies.keys).to contain_exactly(*expected_valid_types)
diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb
index c081b20d95f..4b925a058e7 100644
--- a/spec/services/web_hook_service_spec.rb
+++ b/spec/services/web_hook_service_spec.rb
@@ -129,7 +129,10 @@ RSpec.describe WebHookService, :request_store, :clean_gitlab_redis_shared_state
 
       context 'there is userinfo' do
         before do
-          project_hook.update!(url: 'http://{one}:{two}@example.com')
+          project_hook.update!(
+            url: 'http://{one}:{two}@example.com',
+            url_variables: { 'one' => 'a', 'two' => 'b' }
+          )
           stub_full_request('http://example.com', method: :post)
         end