Merge branch 'security-mass-assignment-on-project-update' into 'master'

Disallow changing namespace of a project in update method

See merge request gitlab/gitlabhq!3028

3 files changed, 30 insertions, 5 deletions

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 33c6608d321..f76e6663995 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -47,7 +47,7 @@ class ProjectsController < Projects::ApplicationController
   end
 
   def create
-    @project = ::Projects::CreateService.new(current_user, project_params).execute
+    @project = ::Projects::CreateService.new(current_user, project_params(attributes: project_params_create_attributes)).execute
 
     if @project.saved?
       cookies[:issue_board_welcome_hidden] = { path: project_path(@project), value: nil, expires: Time.at(0) }
@@ -328,9 +328,9 @@ class ProjectsController < Projects::ApplicationController
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
-  def project_params
+  def project_params(attributes: [])
     params.require(:project)
-      .permit(project_params_attributes)
+      .permit(project_params_attributes + attributes)
   end
 
   def project_params_attributes
@@ -349,11 +349,10 @@ class ProjectsController < Projects::ApplicationController
       :last_activity_at,
       :lfs_enabled,
       :name,
-      :namespace_id,
       :only_allow_merge_if_all_discussions_are_resolved,
       :only_allow_merge_if_pipeline_succeeds,
-      :printing_merge_request_link_enabled,
       :path,
+      :printing_merge_request_link_enabled,
       :public_builds,
       :request_access_enabled,
       :runners_token,
@@ -375,6 +374,10 @@ class ProjectsController < Projects::ApplicationController
     ]
   end
 
+  def project_params_create_attributes
+    [:namespace_id]
+  end
+
   def custom_import_params
     {}
   end
diff --git a/changelogs/unreleased/security-mass-assignment-on-project-update.yml b/changelogs/unreleased/security-mass-assignment-on-project-update.yml
new file mode 100644
index 00000000000..93561cd91b3
--- /dev/null
+++ b/changelogs/unreleased/security-mass-assignment-on-project-update.yml
@@ -0,0 +1,5 @@
+---
+title: Disallow updating namespace when updating a project
+merge_request:
+author:
+type: security
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index a1662658ade..356d606d5c5 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -369,6 +369,23 @@ describe ProjectsController do
         end
       end
 
+      it 'does not update namespace' do
+        controller.instance_variable_set(:@project, project)
+
+        params = {
+          namespace_id: 'test'
+        }
+
+        expect do
+          put :update,
+            params: {
+              namespace_id: project.namespace,
+              id: project.id,
+              project: params
+            }
+        end.not_to change { project.namespace.reload }
+      end
+
       def update_project(**parameters)
         put :update,
             params: {