This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new 06092ae Improved: Fix some bugs Spotbugs reports (OFBIZ-12386)
06092ae is described below
commit 06092ae0afd37c6855e6b13f2c9ea48c0d2e7251
Author: Jacques Le Roux <[email protected]>
AuthorDate: Sun Dec 26 14:50:32 2021 +0100
Improved: Fix some bugs Spotbugs reports (OFBIZ-12386)
As advised by SpotBugs:
<<This code creates a java.util.Random object, uses it to generate one
random
number, and then discards the Random object. This produces mediocre quality
random numbers and is inefficient. If possible, rewrite the code so that the
Random object is created once and saved, and each time a new random number
is
required invoke a method on the existing Random object to obtain it.
If it is important that the generated Random numbers not be guessable, you
must
not create a new Random for each random number; the values are too easily
guessable. You should strongly consider using a java.security.SecureRandom
instead (and avoid allocating a new SecureRandom for each random number
needed).
Rank: Troubling (14), confidence: High
Pattern: DMI_RANDOM_USED_ONLY_ONCE
Type: DMI, Category: BAD_PRACTICE (Bad practice)>>
uses <<private static final SecureRandom SECURE_RANDOM = new
SecureRandom();>>
in classes:
PaymentGatewayServices
ValueLinkApi
ContactMechServices
ProductStoreWorker
DesCrypt
HashCrypt
EntityCrypto
ContextFilter.java
Despite doing so SpotButs still reports the same error for those classes.
As it's obviously an error I prefer to not excludes those cases that will
maybe
not be reported in a future version of SpotButs in Eclipse. For instance
they
are not reported at all in previous version (4.5.1) but are in 4.5.2
---
.../accounting/payment/PaymentGatewayServices.java | 5 +++--
.../thirdparty/valuelink/ValueLinkApi.java | 20 +++++++-------------
.../ofbiz/party/contact/ContactMechServices.java | 6 +++---
.../ofbiz/product/store/ProductStoreWorker.java | 4 ++--
.../java/org/apache/ofbiz/base/crypto/DesCrypt.java | 5 +++--
.../java/org/apache/ofbiz/base/crypto/HashCrypt.java | 4 +++-
.../org/apache/ofbiz/entity/util/EntityCrypto.java | 8 ++++----
.../apache/ofbiz/webapp/control/ContextFilter.java | 4 +++-
8 files changed, 28 insertions(+), 28 deletions(-)
diff --git
a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/PaymentGatewayServices.java
b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/PaymentGatewayServices.java
index 382d691..a059ccf 100644
---
a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/PaymentGatewayServices.java
+++
b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/PaymentGatewayServices.java
@@ -92,6 +92,8 @@ public class PaymentGatewayServices {
private static final RoundingMode ROUNDING =
UtilNumber.getRoundingMode("order.rounding");
private static final BigDecimal ZERO = BigDecimal.ZERO.setScale(DECIMALS,
ROUNDING);
+ private static final SecureRandom SECURE_RANDOM = new SecureRandom();
+
/**
* Authorizes a single order preference with an option to specify an
amount. The result map has the Booleans
* "errors" and "finished" which notify the user if there were any errors
and if the authorization was finished.
@@ -3441,8 +3443,7 @@ public class PaymentGatewayServices {
Locale locale = (Locale) context.get("locale");
Map<String, Object> result = ServiceUtil.returnSuccess();
String refNum = UtilDateTime.nowAsString();
- SecureRandom secureRandom = new SecureRandom();
- int i = secureRandom.nextInt(9);
+ int i = SECURE_RANDOM.nextInt(9);
if (i < 5 || i % 2 == 0) {
result.put("authResult", Boolean.TRUE);
result.put("authFlag", "A");
diff --git
a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/valuelink/ValueLinkApi.java
b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/valuelink/ValueLinkApi.java
index a07d9a3..d9fcf32 100644
---
a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/valuelink/ValueLinkApi.java
+++
b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/valuelink/ValueLinkApi.java
@@ -41,7 +41,6 @@ import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Properties;
-import java.util.Random;
import javax.crypto.BadPaddingException;
import javax.crypto.Cipher;
@@ -92,6 +91,8 @@ public class ValueLinkApi {
private Long mwkIndex = null;
private boolean debug = false;
+ private static final SecureRandom SECURE_RANDOM = new SecureRandom();
+
protected ValueLinkApi() { }
protected ValueLinkApi(Delegator delegator, Properties props) {
String mId = (String) props.get("payment.valuelink.merchantId");
@@ -158,8 +159,7 @@ public class ValueLinkApi {
*/
public String encryptPin(String pin) {
byte[] rawIv = new byte[8];
- SecureRandom random = new SecureRandom();
- random.nextBytes(rawIv);
+ SECURE_RANDOM.nextBytes(rawIv);
IvParameterSpec iv = new IvParameterSpec(rawIv);
// get the Cipher
Cipher mwkCipher = null;
@@ -352,8 +352,7 @@ public class ValueLinkApi {
// test the KEK
byte[] rawIv = new byte[8];
- SecureRandom secRandom = new SecureRandom();
- secRandom.nextBytes(rawIv);
+ SECURE_RANDOM.nextBytes(rawIv);
IvParameterSpec iv = new IvParameterSpec(rawIv);
Cipher cipher = null;
@@ -600,12 +599,9 @@ public class ValueLinkApi {
// 8 bytes random data
byte[] random = new byte[8];
- Random ran = new SecureRandom();
- ran.nextBytes(random);
byte[] rawIv = new byte[8];
- SecureRandom secRandom = new SecureRandom();
- secRandom.nextBytes(rawIv);
+ SECURE_RANDOM.nextBytes(rawIv);
IvParameterSpec iv = new IvParameterSpec(rawIv);
// open a cipher using the new mwk
@@ -821,8 +817,7 @@ public class ValueLinkApi {
protected byte[] cryptoViaKek(byte[] content, int mode) throws
GeneralException {
// open a cipher using the kek for transport
byte[] rawIv = new byte[8];
- SecureRandom random = new SecureRandom();
- random.nextBytes(rawIv);
+ SECURE_RANDOM.nextBytes(rawIv);
IvParameterSpec iv = new IvParameterSpec(rawIv);
// Create the Cipher - DESede/CBC/PKCS5Padding
@@ -880,9 +875,8 @@ public class ValueLinkApi {
* @return the byte [ ]
*/
protected byte[] getRandomBytes(int length) {
- Random rand = new SecureRandom();
byte[] randomBytes = new byte[length];
- rand.nextBytes(randomBytes);
+ SECURE_RANDOM.nextBytes(randomBytes);
return randomBytes;
}
diff --git
a/applications/party/src/main/java/org/apache/ofbiz/party/contact/ContactMechServices.java
b/applications/party/src/main/java/org/apache/ofbiz/party/contact/ContactMechServices.java
index 1803e67..836ed25 100644
---
a/applications/party/src/main/java/org/apache/ofbiz/party/contact/ContactMechServices.java
+++
b/applications/party/src/main/java/org/apache/ofbiz/party/contact/ContactMechServices.java
@@ -61,6 +61,8 @@ public class ContactMechServices {
private static final String RESOURCE = "PartyUiLabels";
private static final String RES_ERROR = "PartyErrorUiLabels";
+ private static final SecureRandom SECURE_RANDOM = new SecureRandom();
+
/**
* Creates a ContactMech
* <b>security check</b>: userLogin partyId must equal partyId, or must
have PARTYMGR_CREATE permission
@@ -976,11 +978,9 @@ public class ContactMechServices {
Date date = calendar.getTime();
Timestamp expireDate = UtilDateTime.toTimestamp(date);
- SecureRandom secureRandom = new SecureRandom();
-
synchronized (ContactMechServices.class) {
while (true) {
- Long random = secureRandom.nextLong();
+ Long random = SECURE_RANDOM.nextLong();
verifyHash = HashCrypt.digestHash("MD5",
Long.toString(random).getBytes(StandardCharsets.UTF_8));
List<GenericValue> emailAddVerifications = null;
try {
diff --git
a/applications/product/src/main/java/org/apache/ofbiz/product/store/ProductStoreWorker.java
b/applications/product/src/main/java/org/apache/ofbiz/product/store/ProductStoreWorker.java
index 7fbe820..76eec0a 100644
---
a/applications/product/src/main/java/org/apache/ofbiz/product/store/ProductStoreWorker.java
+++
b/applications/product/src/main/java/org/apache/ofbiz/product/store/ProductStoreWorker.java
@@ -55,6 +55,7 @@ import org.apache.ofbiz.webapp.website.WebSiteWorker;
public final class ProductStoreWorker {
private static final String MODULE = ProductStoreWorker.class.getName();
+ private static final SecureRandom SECURE_RANDOM = new SecureRandom();
private ProductStoreWorker() { }
@@ -453,8 +454,7 @@ public final class ProductStoreWorker {
partyId, Map<String, Object> passThruFields) {
List<GenericValue> randomSurveys = getSurveys(delegator,
productStoreId, groupName, null, "RANDOM_POLL", null);
if (UtilValidate.isNotEmpty(randomSurveys)) {
- SecureRandom rand = new SecureRandom();
- int index = rand.nextInt(randomSurveys.size());
+ int index = SECURE_RANDOM.nextInt(randomSurveys.size());
GenericValue appl = randomSurveys.get(index);
return new ProductStoreSurveyWrapper(appl, partyId,
passThruFields);
} else {
diff --git
a/framework/base/src/main/java/org/apache/ofbiz/base/crypto/DesCrypt.java
b/framework/base/src/main/java/org/apache/ofbiz/base/crypto/DesCrypt.java
index 924b4b6..02ddb9b 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/crypto/DesCrypt.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/crypto/DesCrypt.java
@@ -43,6 +43,8 @@ import org.apache.ofbiz.base.util.GeneralException;
*/
public class DesCrypt {
+ private static final SecureRandom SECURE_RANDOM = new SecureRandom();
+
public static Key generateKey() throws NoSuchAlgorithmException {
KeyGenerator keyGen = KeyGenerator.getInstance("DESede");
@@ -52,8 +54,7 @@ public class DesCrypt {
public static byte[] encrypt(Key key, byte[] bytes) throws
GeneralException {
byte[] rawIv = new byte[8];
- SecureRandom random = new SecureRandom();
- random.nextBytes(rawIv);
+ SECURE_RANDOM.nextBytes(rawIv);
IvParameterSpec iv = new IvParameterSpec(rawIv);
// Create the Cipher - DESede/CBC/PKCS5Padding
diff --git
a/framework/base/src/main/java/org/apache/ofbiz/base/crypto/HashCrypt.java
b/framework/base/src/main/java/org/apache/ofbiz/base/crypto/HashCrypt.java
index 51914c1..03c7e8f 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/crypto/HashCrypt.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/crypto/HashCrypt.java
@@ -54,6 +54,8 @@ public class HashCrypt {
private static final int PBKDF2_ITERATIONS =
UtilProperties.getPropertyAsInteger("security.properties",
"password.encrypt.pbkdf2.iterations", 10000);
+ private static final SecureRandom SECURE_RANDOM = new SecureRandom();
+
public static MessageDigest getMessageDigest(String type) {
try {
return MessageDigest.getInstance(type);
@@ -145,7 +147,7 @@ public class HashCrypt {
hashType = "SHA";
}
if (salt == null) {
- salt = RandomStringUtils.random(new SecureRandom().nextInt(15) +
1, CRYPT_CHAR_SET);
+ salt = RandomStringUtils.random(SECURE_RANDOM.nextInt(15) + 1,
CRYPT_CHAR_SET);
}
StringBuilder sb = new StringBuilder();
sb.append("$").append(hashType).append("$").append(salt).append("$");
diff --git
a/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java
b/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java
index 84e2528..e58b933 100644
---
a/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java
+++
b/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java
@@ -23,7 +23,6 @@ import java.nio.charset.StandardCharsets;
import java.security.Key;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
-import java.util.Random;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
@@ -56,6 +55,8 @@ public final class EntityCrypto {
private final ConcurrentMap<String, byte[]> keyMap = new
ConcurrentHashMap<>();
private final StorageHandler[] handlers;
+ private static final SecureRandom SECURE_RANDOM = new SecureRandom();
+
public EntityCrypto(Delegator delegator, String kekText) throws
EntityCryptoException {
this.delegator = delegator;
byte[] kek;
@@ -418,10 +419,9 @@ public final class EntityCrypto {
byte[] saltBytes;
switch (encryptMethod) {
case SALT:
- Random random = new SecureRandom();
// random length 5-16
- saltBytes = new byte[5 + random.nextInt(11)];
- random.nextBytes(saltBytes);
+ saltBytes = new byte[5 + SECURE_RANDOM.nextInt(11)];
+ SECURE_RANDOM.nextBytes(saltBytes);
break;
default:
saltBytes = new byte[0];
diff --git
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
index 69f89b1..b23f2c5 100644
---
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
+++
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
@@ -19,6 +19,7 @@
package org.apache.ofbiz.webapp.control;
import java.io.IOException;
+import java.security.SecureRandom;
import java.util.Enumeration;
import javax.servlet.Filter;
@@ -51,6 +52,7 @@ import org.apache.ofbiz.webapp.website.WebSiteWorker;
public class ContextFilter implements Filter {
private static final String MODULE = ContextFilter.class.getName();
+ private static final SecureRandom SECURE_RANDOM = new SecureRandom();
private FilterConfig config = null;
@@ -82,7 +84,7 @@ public class ContextFilter implements Filter {
isMultitenant = EntityUtil.isMultiTenantEnabled();
// this will speed up the initial sessionId generation
- new java.security.SecureRandom().nextLong();
+ SECURE_RANDOM.nextLong();
}
@Override
