From 5cd4dc99827eb10c7a8f17b3c8c142fe7fb6f8e4 Mon Sep 17 00:00:00 2001 From: Lukas Jezek Date: Mon, 20 Apr 2026 08:32:15 +0000 Subject: [PATCH 1/3] Fixes #39478 - CVE-2026-5136 --- app/models/usergroup.rb | 10 +++++++ test/models/usergroup_test.rb | 56 ++++++++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/app/models/usergroup.rb b/app/models/usergroup.rb index ff4c738078..2b6e047835 100644 --- a/app/models/usergroup.rb +++ b/app/models/usergroup.rb @@ -41,6 +41,7 @@ class Usergroup < ApplicationRecord scoped_search :relation => :roles, :on => :id, :rename => :role_id, :complete_enabled => false, :only_explicit => true, :validator => ScopedSearch::Validators::INTEGER validate :ensure_uniq_name, :ensure_last_admin_remains_admin validate :admin_can_be_set, :if => ->(o) { o.changed.include?('admin') } + validate :ensure_roles_not_escalated accepts_nested_attributes_for :external_usergroups, :reject_if => ->(a) { a[:name].blank? }, :allow_destroy => true @@ -131,4 +132,13 @@ def other_admins def admin_can_be_set errors.add :admin, _('admin flag can only be modified by admins') unless User.current.admin? end + + def ensure_roles_not_escalated + return if User.current.nil? + + roles_check = new_record? ? role_ids.present? : role_ids_changed? + if roles_check && !User.current.can_assign?(role_ids) + errors.add :role_ids, _("you can't assign some of roles you selected") + end + end end diff --git a/test/models/usergroup_test.rb b/test/models/usergroup_test.rb index 1f198c63bd..4c627908d9 100644 --- a/test/models/usergroup_test.rb +++ b/test/models/usergroup_test.rb @@ -209,7 +209,61 @@ def populate_usergroups assert_equal recipients, [users[0]] end - # TODO test who can modify usergroup roles and who can assign users!!! possible privileges escalation + context 'role assignment (privilege escalation)' do + setup do + @owned_role = Role.where(:name => "usergroup_test_owned_role").first_or_create + @foreign_role = Role.where(:name => "usergroup_test_foreign_role").first_or_create + end + + test "non-admin with create_usergroups cannot assign roles they do not have" do + setup_user "create", "usergroups" + usergroup = Usergroup.new(:name => "escalation-blocked-create", :role_ids => [@foreign_role.id]) + refute usergroup.valid? + assert_includes usergroup.errors.attribute_names, :role_ids + end + + test "non-admin with create_usergroups can assign roles they already hold" do + setup_user "create", "usergroups" do |user| + user.roles << @owned_role + end + usergroup = Usergroup.new(:name => "delegated-roles-ok", :role_ids => [@owned_role.id]) + assert_valid usergroup + assert usergroup.save + end + + test "non-admin with edit_usergroups cannot add roles they do not hold" do + usergroup = as_admin { FactoryBot.create(:usergroup, :name => "escalation-update-target", :role_ids => [@owned_role.id]) } + setup_user "edit", "usergroups" do |user| + user.roles << @owned_role + end + usergroup.role_ids = usergroup.role_ids + [@foreign_role.id] + refute usergroup.valid? + assert_includes usergroup.errors.attribute_names, :role_ids + end + + test "admin can assign arbitrary roles to usergroup" do + as_admin do + usergroup = Usergroup.new(:name => "admin-assigns-roles", :role_ids => [@foreign_role.id]) + assert_valid usergroup + assert usergroup.save + end + end + + test "user with escalate_roles permission can assign roles they do not hold to usergroup" do + user = FactoryBot.create(:user, :admin => false) + escalate_role = FactoryBot.create(:role) + FactoryBot.create(:user_role, :owner => user, :role => escalate_role) + FactoryBot.create(:filter, :role => escalate_role, :permissions => [permissions(:escalate_roles)]) + usergroup_role = FactoryBot.create(:role) + FactoryBot.create(:user_role, :owner => user, :role => usergroup_role) + FactoryBot.create(:filter, :role => usergroup_role, :permissions => [permissions(:create_usergroups)]) + as_user user do + usergroup = Usergroup.new(:name => "escalate-roles-ok", :role_ids => [@foreign_role.id]) + assert_valid usergroup + assert usergroup.save + end + end + end context 'external usergroups' do setup do From de1b609f4fa6db790b24d8a63df6c5f2a7792bae Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Thu, 28 May 2026 13:07:15 +0200 Subject: [PATCH 2/3] Refs #39478 - Add test for usergroup role escalation A user who is a member of a usergroup can add a role granting escalate_roles to that usergroup, bypassing the privilege escalation check. The role assignment is cached before the validation runs, so can_escalate? sees the newly granted permission and allows it. Co-Authored-By: Claude Opus 4.6 --- test/models/usergroup_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/models/usergroup_test.rb b/test/models/usergroup_test.rb index 4c627908d9..138607fb74 100644 --- a/test/models/usergroup_test.rb +++ b/test/models/usergroup_test.rb @@ -241,6 +241,21 @@ def populate_usergroups assert_includes usergroup.errors.attribute_names, :role_ids end + test "non-admin member of usergroup cannot add a role granting escalate_roles" do + usergroup = as_admin { FactoryBot.create(:usergroup, :name => "escalate-via-membership") } + escalate_role = as_admin do + role = FactoryBot.create(:role, :name => "role_with_escalate") + FactoryBot.create(:filter, :role => role, :permissions => [permissions(:escalate_roles)]) + role + end + setup_user "edit", "usergroups" do |user| + usergroup.users << user + end + usergroup.role_ids = [escalate_role.id] + refute usergroup.valid? + assert_includes usergroup.errors.attribute_names, :role_ids + end + test "admin can assign arbitrary roles to usergroup" do as_admin do usergroup = Usergroup.new(:name => "admin-assigns-roles", :role_ids => [@foreign_role.id]) From 2baf3879521ac92da4d5a49db160bed06f27da84 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Fri, 29 May 2026 11:35:50 +0200 Subject: [PATCH 3/3] Refs #39478 - Fix usergroup role assignment validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user is a member of a usergroup and edits its roles, the role_ids= setter immediately persists UserRole records and populates the CachedUserRole cache. By the time the ensure_roles_not_escalated validation runs, can_escalate? sees the freshly cached permission and allows the assignment — letting the user grant themselves escalate_roles through the group. Fix by excluding CachedUserRole entries for the newly added roles on the edited usergroup when checking escalation capability. Pre-existing roles on the same usergroup (including a legitimate escalate_roles grant) are still honored. Co-Authored-By: Claude Opus 4.6 --- app/models/user.rb | 14 ++++++++++++ app/models/usergroup.rb | 7 ++++-- test/models/usergroup_test.rb | 43 +++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index bbf2c60dd3..ea80ef8132 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -457,6 +457,10 @@ def can_escalate? admin? || can?(:escalate_roles) end + def can_assign_for_usergroup?(role_ids, usergroup, new_role_ids) + can_escalate_excluding_roles?(usergroup, new_role_ids) || role_ids.all? { |r| role_ids_was.include?(r) } + end + # only admin can change admin flag def can_change_admin_flag? admin? @@ -610,6 +614,16 @@ def authenticate_by_personal_access_token(token) private + def can_escalate_excluding_roles?(usergroup, role_ids) + return true if admin? + tainted_user_role_ids = UserRole.where(owner: usergroup, role_id: role_ids).pluck(:id) + cached_user_roles + .where.not(user_role_id: tainted_user_role_ids) + .joins(role: { filters: :permissions }) + .where(permissions: { name: :escalate_roles }) + .exists? + end + def prepare_password if password.present? self.password_salt = salt_password diff --git a/app/models/usergroup.rb b/app/models/usergroup.rb index 2b6e047835..3575b2b103 100644 --- a/app/models/usergroup.rb +++ b/app/models/usergroup.rb @@ -137,8 +137,11 @@ def ensure_roles_not_escalated return if User.current.nil? roles_check = new_record? ? role_ids.present? : role_ids_changed? - if roles_check && !User.current.can_assign?(role_ids) - errors.add :role_ids, _("you can't assign some of roles you selected") + if roles_check + new_role_ids = new_record? ? role_ids : role_ids - role_ids_was + unless User.current.can_assign_for_usergroup?(role_ids, self, new_role_ids) + errors.add :role_ids, _("you can't assign some of roles you selected") + end end end end diff --git a/test/models/usergroup_test.rb b/test/models/usergroup_test.rb index 138607fb74..0ab0e87e0d 100644 --- a/test/models/usergroup_test.rb +++ b/test/models/usergroup_test.rb @@ -256,6 +256,49 @@ def populate_usergroups assert_includes usergroup.errors.attribute_names, :role_ids end + test "member of usergroup with escalate_roles can add other roles to that usergroup" do + escalate_role = as_admin do + role = FactoryBot.create(:role, :name => "role_with_escalate_for_member") + FactoryBot.create(:filter, :role => role, :permissions => [permissions(:escalate_roles)]) + role + end + usergroup = as_admin { FactoryBot.create(:usergroup, :name => "escalate-member-adds-role", :role_ids => [escalate_role.id]) } + setup_user "edit", "usergroups" do |user| + usergroup.users << user + end + usergroup.role_ids = usergroup.role_ids + [@foreign_role.id] + assert_valid usergroup + end + + test "transitive member of usergroup cannot add a role granting escalate_roles" do + parent_usergroup = as_admin { FactoryBot.create(:usergroup, :name => "escalate-transitive-parent") } + child_usergroup = as_admin { FactoryBot.create(:usergroup, :name => "escalate-transitive-child") } + as_admin { parent_usergroup.usergroups << child_usergroup } + escalate_role = as_admin do + role = FactoryBot.create(:role, :name => "role_with_escalate_transitive") + FactoryBot.create(:filter, :role => role, :permissions => [permissions(:escalate_roles)]) + role + end + setup_user "edit", "usergroups" do |user| + child_usergroup.users << user + end + parent_usergroup.role_ids = [escalate_role.id] + refute parent_usergroup.valid? + assert_includes parent_usergroup.errors.attribute_names, :role_ids + end + + test "non-admin cannot create usergroup with self as member and a role granting escalate_roles" do + escalate_role = as_admin do + role = FactoryBot.create(:role, :name => "role_with_escalate_create_self") + FactoryBot.create(:filter, :role => role, :permissions => [permissions(:escalate_roles)]) + role + end + setup_user "create", "usergroups" + usergroup = Usergroup.new(:name => "escalate-create-self", :role_ids => [escalate_role.id], :user_ids => [User.current.id]) + refute usergroup.valid? + assert_includes usergroup.errors.attribute_names, :role_ids + end + test "admin can assign arbitrary roles to usergroup" do as_admin do usergroup = Usergroup.new(:name => "admin-assigns-roles", :role_ids => [@foreign_role.id])