Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mbiuki wants to merge 1 commit into
base: main
Choose a base branch
from
+28 −0
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions dotCMS/src/main/java/com/dotmarketing/business/UserAPIImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import java.util.Optional;

import static com.dotmarketing.business.UserHelper.validateMaximumLength;
import static com.dotmarketing.business.UserHelper.validateName;
import static com.dotmarketing.util.Constants.DONT_RESPECT_FRONT_END_ROLES;

/**
Expand Down Expand Up @@ -442,6 +443,7 @@ public void save(final User userToSave, final User user, final boolean respectFr
throw new DotSecurityException(
"User doesn't have permission to save the user which is trying to be saved");
}
validateName(userToSave.getFirstName(), userToSave.getLastName());

@github-actions github-actions Bot Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [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.

validateMaximumLength(userToSave.getFirstName(),userToSave.getLastName(),userToSave.getEmailAddress());
userFactory.save(userToSave);
PasswordTrackerLocalManager passwordTracker = PasswordTrackerLocalManagerFactory
Expand Down
26 changes: 26 additions & 0 deletions dotCMS/src/main/java/com/dotmarketing/business/UserHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.exception.DotSecurityException;
import com.dotmarketing.util.Logger;
import com.dotmarketing.util.RegEX;
import com.dotmarketing.util.UtilMethods;
import com.liferay.util.SystemProperties;
import com.liferay.portal.NoSuchUserException;
import com.liferay.portal.*;
import com.liferay.portal.ejb.UserUtil;
Expand Down Expand Up @@ -187,6 +189,30 @@ else if (!PwdToolkitUtil.validate(password1) ||
}
}

/**
* Validates that user-supplied names do not contain HTML/control metacharacters
* (e.g. {@code <}, {@code >}, CR/LF). This mirrors the legacy
* {@code UserLocalManagerImpl} check but is enforced here so it also covers the
* modern REST/API save path (e.g. {@code PUT /api/v1/users/current}), which would
* otherwise persist names verbatim and enable stored XSS in the admin UI.
*
* The allowed character set is driven by the configurable
* {@code UserName.regexp.pattern} system property; validation is skipped only when
* that property is not configured.

@github-actions github-actions Bot Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [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.

*/
public static void validateName(final String firstName, final String lastName) throws DotDataException {
final String pattern = GetterUtil.getString(SystemProperties.get("UserName.regexp.pattern"));
if (!UtilMethods.isSet(pattern)) {
return;
}
if (UtilMethods.isSet(firstName) && !RegEX.contains(firstName, pattern)) {
throw new DotDataException("First Name contains invalid characters");
}
if (UtilMethods.isSet(lastName) && !RegEX.contains(lastName, pattern)) {
throw new DotDataException("Last Name contains invalid characters");
}
}

public static void validateMaximumLength(final String firstName, final String lastName, final String email) throws DotDataException {
if (UtilMethods.exceedsMaxLength(firstName, MAX_FIELD_LENGTH)) {
throw new DotDataException("Length of First Name provided exceeds the maximum limit " + MAX_FIELD_LENGTH);
Expand Down
Loading