fix(security): validate user names on API save path — stored-XSS hardening (chain PR 8)

[mbiuki]

Security fix — XSS→RCE chain, server-side validation layer (tracking: dotCMS/private-issues#651)

Class: Stored XSS input gap (CWE-79) · Severity: Critical (chain) · Routing: Team : Maintenance

Problem

PUT /api/v1/users/current → UserResourceHelper → UserAPIImpl.save() only ran validateMaximumLength. The </>-blocking UserName.regexp.pattern was enforced only in the legacy UserLocalManagerImpl.validate(), which this path never calls — so givenName/surname were stored verbatim and later rendered unescaped in the admin Users panel.

Fix

• New UserHelper.validateName(firstName, lastName), driven by the existing configurable UserName.regexp.pattern allow-list.
• Called from the central UserAPIImpl.save(user, user, respectFrontEndRoles) choke point, so all save paths (REST + legacy) are covered.

Defense-in-depth note

This is the input-validation layer. The canonical fix is output encoding at render (separate PR for view_users.jsp) — filters alone are bypassable (the original report used a field-split bypass). Both layers should land.

Verification

• ./mvnw compile -pl :dotcms-core -DskipTests ✅

⚠️ Draft. Publicly references an unpatched critical chain per maintainer instruction — coordinate with the security-advisory/release process before merge.

[github-actions]

❌ Linked Issue Needs Team Label

This PR is linked to issue #651, but that issue has no Team : * label. Every linked issue must be owned by a team for tracking and triage.

How to fix this:

Apply a Team : * label to the linked issue (e.g., Team : Scout, Team : Platform, Team : Falcon, Team : Maintenance). Then push a new commit or edit the PR description to re-run this check.

---

This comment was automatically generated by the issue linking workflow

[claude]

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

---

Rollback Safety Analysis

• Gather context and PR diff
• Read rollback-unsafe categories reference
• Analyze diff against unsafe categories
• Apply appropriate label

---

Result: ✅ Safe To Rollback

The changes in this PR consist entirely of pure Java business-logic additions — no unsafe category is triggered:

Category	Check	Result
C-1 Structural data model change	No runonce tasks, no schema DDL	✅ Not applicable
C-2 Elasticsearch mapping change	No ES mapping code touched	✅ Not applicable
C-3 Content JSON model version bump	No CURRENT_MODEL_VERSION or ImmutableContentlet change	✅ Not applicable
C-4 DROP TABLE / DROP COLUMN	No DDL of any kind	✅ Not applicable
H-1 One-way data migration/backfill	No UPDATE … SET across rows, no data transform	✅ Not applicable
H-2 RENAME TABLE / RENAME COLUMN	No rename DDL	✅ Not applicable
H-3 PK / unique constraint restructuring	No constraint changes	✅ Not applicable
H-4 New ContentType field type	No new Field subclass or FieldTypeAPI registration	✅ Not applicable
H-5 Binary storage provider change	No FileStorageAPIImpl / StoragePersistenceProvider change	✅ Not applicable
H-6 DROP PROCEDURE / DROP FUNCTION	No stored procedure changes	✅ Not applicable
H-7 NOT NULL column without default	No schema changes at all	✅ Not applicable
H-8 VTL viewtool contract change	No viewtool classes modified	✅ Not applicable
M-1 Non-broadening column type change	No column type changes	✅ Not applicable
M-2 Push publishing bundle format change	No bundler/handler changes	✅ Not applicable
M-3 REST/GraphQL API contract change	No response shape changes; the endpoint now rejects previously-accepted malicious input — this is a tightening of input validation, not a contract break for well-formed requests	✅ Not applicable
M-4 OSGi public interface change	UserHelper.validateName is a new static method added — no existing signature modified	✅ Not applicable

Summary: Rolling back to the previous release removes the added validateName() call and the method itself. N-1 simply reverts to the prior behavior (no name-format validation on the REST save path). No database, index, or persistent state is altered — rollback is fully reversible.

Label applied: AI: Safe To Rollback

[github-actions]

🤖 dotBot Review (Bedrock)

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

Confirmed findings

• 🟠 High dotCMS/src/main/java/com/dotmarketing/business/UserHelper.java:201 — Incorrect configuration property retrieval
  The code uses SystemProperties.get("UserName.regexp.pattern") instead of Config.getStringProperty(...). In dotCMS, Config handles configuration from multiple sources (properties file, env vars), while SystemProperties only reads system properties. This could result in the regex pattern not being correctly loaded, leading to improper validation. Grep results show other instances in the codebase (e.g., UserLocalManagerImpl.java) use Config.getStringProperty("UserName.regexp.pattern"), indicating this is a deviation from standard practice.
• 🟠 High dotCMS/src/main/java/com/dotmarketing/business/UserAPIImpl.java:446 — Potential NullPointerException in user name validation
  UserAPIImpl.java line 446 passes userToSave.getFirstName() and getLastName() directly to UserHelper.validateName(). If either returns null, String.matches() will throw NullPointerException. The validation helper does not appear to perform null checks before regex matching in the current PR changes.

---

_{us.deepseek.r1-v1:0 · Run: #28387045872 · tokens: in: 11573 · out: 3549 · total: 15122 · calls: 6 · est. ~$0.035}

[github-actions]

🟠 [High] Incorrect configuration property retrieval

The code uses SystemProperties.get("UserName.regexp.pattern") instead of Config.getStringProperty(...). In dotCMS, Config handles configuration from multiple sources (properties file, env vars), while SystemProperties only reads system properties. This could result in the regex pattern not being correctly loaded, leading to improper validation. Grep results show other instances in the codebase (e.g., UserLocalManagerImpl.java) use Config.getStringProperty("UserName.regexp.pattern"), indicating this is a deviation from standard practice.

[github-actions]

🟠 [High] Potential NullPointerException in user name validation

UserAPIImpl.java line 446 passes userToSave.getFirstName() and getLastName() directly to UserHelper.validateName(). If either returns null, String.matches() will throw NullPointerException. The validation helper does not appear to perform null checks before regex matching in the current PR changes.