Skip to content

Commit

Permalink
Improved: Improve use of RandomStringUtils where it's potentially use…
Browse files Browse the repository at this point in the history
…d in an insecure way (OFBIZ-12854)

This is related to CWE-338 and CVE-2019-16303 that don't concern OFBiz.

Actually the password generated by the passport component is not more insecure
than the ofbiz password used OOTB in many places. But it's somehow hidden
(automated generation) and it's easy to randomise it better, still using only
alphanumeric chars as currently.

There are other uses of RandomStringUtils but they don't relate to passwords
generation and are safely used.

Thanks: Alessandro Albani who reported globally for all ASF projects
  • Loading branch information
JacquesLeRoux committed Sep 12, 2023
1 parent b189871 commit 598ccb6
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public class GitHubEvents {
private static final String SESSION_GITHUB_STATE = "_GITHUB_STATE_";

private static final SecureRandom SECURE_RANDOM = new SecureRandom();
private static final String ALPHANUMERIC = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";

public static final String ENV_PREFIX = UtilProperties.getPropertyValue(GitHubAuthenticator.PROPS, "github.env.prefix", "test");

Expand Down Expand Up @@ -275,8 +276,9 @@ private static String checkLoginGitHubUser(HttpServletRequest request, Map<Strin
String userLoginId = authn.createUser(userInfo);
userLogin = EntityQuery.use(delegator).from("UserLogin").where("userLoginId", userLoginId).queryOne();
}
String autoPassword = RandomStringUtils.randomAlphanumeric(EntityUtilProperties.getPropertyAsInteger("security",
"password.length.min", 5));
String autoPassword = RandomStringUtils.random(
EntityUtilProperties.getPropertyAsInteger("security", "password.length.min", 5), 0, 0, true, true,
ALPHANUMERIC.toCharArray(), SECURE_RANDOM);
boolean useEncryption = "true".equals(UtilProperties.getPropertyValue("security", "password.encrypt"));
userLogin.set("currentPassword", useEncryption ? HashCrypt.digestHash(LoginServices.getHashType(), null, autoPassword) : autoPassword);
userLogin.store();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public class LinkedInEvents {
public static final String ENV_PREFIX = UtilProperties.getPropertyValue(LinkedInAuthenticator.getPROPS(), "linkedin.env.prefix", "test");

private static final SecureRandom SECURE_RANDOM = new SecureRandom();
private static final String ALPHANUMERIC = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";

/**
* Redirect to LinkedIn login page.
Expand Down Expand Up @@ -266,8 +267,9 @@ private static String checkLoginLinkedInUser(HttpServletRequest request, Documen
String userLoginId = authn.createUser(userInfo);
userLogin = EntityQuery.use(delegator).from("UserLogin").where("userLoginId", userLoginId).queryOne();
}
String autoPassword = RandomStringUtils.randomAlphanumeric(EntityUtilProperties.getPropertyAsInteger("security",
"password.length.min", 5));
String autoPassword = RandomStringUtils.random(
EntityUtilProperties.getPropertyAsInteger("security", "password.length.min", 5), 0, 0, true, true,
ALPHANUMERIC.toCharArray(), SECURE_RANDOM);
boolean useEncryption = "true".equals(UtilProperties.getPropertyValue("security", "password.encrypt"));
userLogin.set("currentPassword", useEncryption ? HashCrypt.digestHash(LoginServices.getHashType(), null, autoPassword) : autoPassword);
userLogin.store();
Expand Down

0 comments on commit 598ccb6

Please sign in to comment.