sec: enforce admin check on layout assignment and role grant endpoints

[mbiuki]

Summary

Fixes a privilege-escalation chain (reported via responsible disclosure) that allowed any authenticated backend user to reach CMS Administrator and subsequently achieve remote code execution via OSGi bundle upload.

Attack chain closed by this PR:

1. PUT /api/v1/toolgroups/{id}/_addtouser — any backend user could self-assign the admin Settings layout (containing the roles portlet) with no privilege check
2. POST /dwr/call/plaincall/RoleAjax.addUserToRole.dwr — with the roles portlet now accessible, the user could grant themselves the CMS Administrator role; the only gate was portlet-access, which step 1 satisfied
3. As CMS Administrator, attacker uploads a malicious OSGi bundle → arbitrary OS command execution

Changes

ToolGroupResource.java

• _addtouser: require loggedInUser.isAdmin() for all layouts except gettingstarted (the onboarding layout — intentionally self-assignable)
• _removefromuser: same admin guard applied symmetrically

RoleAjax.java

• addUserToRole: add caller.isAdmin() check after the existing portlet-access check; log unauthorized attempts via SecurityLogger
• removeUsersFromRole: same admin guard applied symmetrically

Test plan

• Verify a low-privilege backend user receives 403 on PUT /api/v1/toolgroups/{layout_id}/_addtouser (non-gettingStarted layout)
• Verify gettingStarted layout self-assignment still works for non-admin users (onboarding flow must not break)
• Verify a low-privilege backend user with roles-portlet access receives DotSecurityException on RoleAjax.addUserToRole via DWR
• Verify a CMS Administrator can still assign layouts and manage roles normally
• Run the researcher's PoC script — it must fail at step 1 and never reach role escalation

References

• Internal tracking: dotCMS/private-issues#642
• CWE-269: Improper Privilege Management

🤖 Generated with Claude Code

[github-actions]

❌ Issue Linking Required

This PR could not be linked to an issue. All PRs must be linked to an issue for tracking purposes.

How to fix this:

Option 1: Add keyword to PR body (Recommended - auto-removes this comment)
Edit this PR description and add one of these lines:

• This PR fixes dotCMS/private-issues#123 or Fixes: dotCMS/private-issues#123

• This PR closes dotCMS/private-issues#123 or Closes: dotCMS/private-issues#123

• This PR resolves dotCMS/private-issues#123 or Resolves: dotCMS/private-issues#123

• Other supported keywords: fix, fixed, close, closed, resolve, resolved
  Option 2: Link via GitHub UI (Note: won't clear the failed check)

1. Go to the PR → Development section (right sidebar)

2. Click "Link issue" and select an existing issue

3. Push a new commit or re-run the workflow to clear the failed check
   Option 3: Use branch naming
   Create a new branch with one of these patterns:

• 123-feature-description (number at start)

• issue-123-feature-description (issue-number at start)

• feature-issue-123 (issue-number anywhere)

Why is this required?

Issue linking ensures proper tracking, documentation, and helps maintain project history. It connects your code changes to the problem they solve.---

This comment was automatically generated by the issue linking workflow

[claude]

Claude finished @mbiuki's task in 1m 42s —— View job

---

Rollback Safety Analysis

• Read rollback-unsafe categories reference
• Get full PR diff
• Analyze diff against unsafe categories
• Apply appropriate label

---

Result: ✅ Safe To Rollback

Label applied: AI: Safe To Rollback

Analysis:

The two changed files contain pure Java authorization logic only:

File	Change
ToolGroupResource.java	Added loggedInUser.isAdmin() guard in _removefromuser and _addtouser methods
RoleAjax.java	Added caller.isAdmin() guard in removeUsersFromRole and addUserToRole methods

Checked against all unsafe categories — none apply:

Category	Verdict
C-1 Structural Data Model Change	✅ No DB schema changes
C-2 Elasticsearch Mapping Change	✅ No ES mapping changes
C-3 Content JSON Model Version Bump	✅ No contentlet_as_json or model version changes
C-4 DROP TABLE / DROP COLUMN	✅ No DDL statements
H-1 One-Way Data Migration	✅ No data transformations
H-2 RENAME TABLE / RENAME COLUMN	✅ No renames
H-3 PK / Unique Constraint Restructuring	✅ No constraint changes
H-4 New ContentType Field Type	✅ No new field types
H-5 Binary Storage Provider Change	✅ No storage changes
H-6 DROP PROCEDURE / DROP FUNCTION	✅ No stored procedures modified
H-7 NOT NULL Column Without Default	✅ No DDL changes
H-8 VTL Viewtool Contract Change	✅ No viewtool methods changed
M-1 Column Type Change	✅ No DDL changes
M-2 Push Publishing Bundle Format Change	✅ No bundle format changes
M-3 REST/GraphQL API Contract Change	✅ No response shape changes — only added DotSecurityException (HTTP 403) on previously unguarded paths
M-4 OSGi Plugin API Breakage	✅ No public interface changes

Rollback scenario: Rolling back to N-1 simply removes the new admin guard — previously-unprotected endpoints become unprotected again, which is the known pre-patch state. No data, schema, or index state is affected by this change in either direction.

[github-actions]

🤖 dotBot Review (Bedrock)

Reviewed 2 file(s); 0 candidate(s) → 0 confirmed, 0 uncertain (unverified, kept for review).

✅ No issues found after verification.

---

_{us.deepseek.r1-v1:0 · Run: #28331363735 · tokens: in: 4318 · out: 1404 · total: 5722 · calls: 2 · est. ~$0.013}

[mbiuki]

This PR fixes https://github.com/dotCMS/private-issues/issues/642

[github-actions]

PRs linked to this issue

• sec: harden portlet-only gated methods in RoleAjax by @mbiuki

[mbiuki]

Versions Affected

Exploitable range: v21.02 → v26.06.22-03 (current latest)

Gap	Introduced	First vulnerable release
RoleAjax.addUserToRole (portlet-only gate)	March 2012	v3.0
ToolGroupResource._addtouser (enabling step)	December 2020	v21.02

No patched release has been cut yet — this PR is the fix.

[mbiuki]

Vulnerability Introduction Timeline

ToolGroupResource.addToolGroupToUser — admin check was never present

Date	Commit	Author	Event
2020-12-16	8aa87f7b87	Will Ezell	Method created without any admin check (PR #19680, issue #19581) — only requiredBackendUser(true) enforced
2021-03-22	890feffa94	Nollymar Longa	?userid= parameter added — widened exploit surface from self-assignment to assigning layouts to any user
2025-11-04	3babaf0d17	—	Partial portlet-level mitigation added (private-issues#482) — insufficient alone
PR #36344	—	—	Full fix: admin-only gate added — not yet in any release

Affected Versions

• First vulnerable release: 21.02 (December 2020 commit shipped in the Feb 2021 release cut)
• Widened attack surface (other-user assignment): ~21.03–21.04 (March 2021 commit)
• All affected: 21.02 → v26.06.22-03 (current latest), including all LTS lines:
  • 23.10 LTS ✗
  • 24.04 LTS ✗
  • 24.12 LTS ✗
  • 25.07 LTS ✗
• Partial mitigation from: v25.11.07-1 (still exploitable via chain with sec: enforce admin check on layout assignment and role grant endpoints #36344)
• Fully fixed in: not yet released