From 36c8a31d573bdd2edd4c87be63eb8dde20a79761 Mon Sep 17 00:00:00 2001
From: GitLab Bot <gitlab-bot@gitlab.com>
Date: Wed, 28 Sep 2022 22:00:24 +0000
Subject: Add latest changes from gitlab-org/security/gitlab@15-4-stable-ee

---
 app/services/base_project_service.rb              |  4 +++-
 lib/gitlab/json_cache.rb                          |  4 +---
 spec/controllers/boards/issues_controller_spec.rb | 20 ++++++++++++++++++--
 spec/services/issues/create_service_spec.rb       | 13 +++++++++++++
 spec/services/issues/update_service_spec.rb       | 17 +++++++++++++++++
 5 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/app/services/base_project_service.rb b/app/services/base_project_service.rb
index 1bf4a235a79..8cb6b632a9e 100644
--- a/app/services/base_project_service.rb
+++ b/app/services/base_project_service.rb
@@ -7,7 +7,9 @@ class BaseProjectService < ::BaseContainerService
   attr_accessor :project
 
   def initialize(project:, current_user: nil, params: {})
-    super(container: project, current_user: current_user, params: params)
+    # we need to exclude project params since they may come from external requests. project should always
+    # be passed as part of the service's initializer
+    super(container: project, current_user: current_user, params: params.except(:project, :project_id))
 
     @project = project
   end
diff --git a/lib/gitlab/json_cache.rb b/lib/gitlab/json_cache.rb
index d5c018cfc68..d2916a01809 100644
--- a/lib/gitlab/json_cache.rb
+++ b/lib/gitlab/json_cache.rb
@@ -43,9 +43,7 @@ module Gitlab
     end
 
     def write(key, value, options = nil)
-      # As we use json as the serialization format, return everything from
-      # ActiveModel objects included encrypted values.
-      backend.write(cache_key(key), value.to_json(unsafe_serialization_hash: true), options)
+      backend.write(cache_key(key), value.to_json, options)
     end
 
     def fetch(key, options = {}, &block)
diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb
index 1fd249eba69..3e1cdfccc61 100644
--- a/spec/controllers/boards/issues_controller_spec.rb
+++ b/spec/controllers/boards/issues_controller_spec.rb
@@ -427,6 +427,22 @@ RSpec.describe Boards::IssuesController do
   end
 
   describe 'POST create' do
+    context 'when trying to create issue on an unauthorized project' do
+      let(:unauthorized_project) { create(:project, :private) }
+      let(:issue_params) { { project_id: unauthorized_project.id } }
+
+      it 'creates the issue on the board\'s project' do
+        expect do
+          create_issue user: user, board: board, list: list1, title: 'New issue', additional_issue_params: issue_params
+        end.to change(Issue, :count).by(1)
+
+        created_issue = Issue.last
+
+        expect(created_issue.project).to eq(project)
+        expect(unauthorized_project.reload.issues.count).to eq(0)
+      end
+    end
+
     context 'with valid params' do
       before do
         create_issue user: user, board: board, list: list1, title: 'New issue'
@@ -500,13 +516,13 @@ RSpec.describe Boards::IssuesController do
       end
     end
 
-    def create_issue(user:, board:, list:, title:)
+    def create_issue(user:, board:, list:, title:, additional_issue_params: {})
       sign_in(user)
 
       post :create, params: {
                       board_id: board.to_param,
                       list_id: list.to_param,
-                      issue: { title: title, project_id: project.id }
+                      issue: { title: title, project_id: project.id }.merge(additional_issue_params)
                     },
                     format: :json
     end
diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb
index 4a84862b9d5..3d52dc07c4f 100644
--- a/spec/services/issues/create_service_spec.rb
+++ b/spec/services/issues/create_service_spec.rb
@@ -47,6 +47,19 @@ RSpec.describe Issues::CreateService do
           due_date: Date.tomorrow }
       end
 
+      context 'when an unauthorized project_id is provided' do
+        let(:unauthorized_project) { create(:project) }
+
+        before do
+          opts[:project_id] = unauthorized_project.id
+        end
+
+        it 'ignores the project_id param and creates issue in the given project' do
+          expect(issue.project).to eq(project)
+          expect(unauthorized_project.reload.issues.count).to eq(0)
+        end
+      end
+
       it 'works if base work item types were not created yet' do
         WorkItems::Type.delete_all
 
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 8a2e9ed74f7..634a4206d48 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -69,6 +69,23 @@ RSpec.describe Issues::UpdateService, :mailer do
         }
       end
 
+      context 'when an unauthorized project_id is provided' do
+        let(:unauthorized_project) { create(:project) }
+
+        before do
+          opts[:project_id] = unauthorized_project.id
+        end
+
+        it 'ignores the project_id param and does not update the issue\'s project' do
+          expect do
+            update_issue(opts)
+            unauthorized_project.reload
+          end.to not_change { unauthorized_project.issues.count }
+
+          expect(issue.project).to eq(project)
+        end
+      end
+
       it 'updates the issue with the given params' do
         expect(TodosDestroyer::ConfidentialIssueWorker).not_to receive(:perform_in)
 
-- 
cgit v1.2.3