From 90cce51e418e7fa8636033a4d6b96cc3bb49641b Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Thu, 30 Jul 2026 13:48:20 +0000
Subject: [PATCH] Fix CodeQL warning-severity alerts: weak salt PRNG, unsafe DCL, thread-unsafe date formats (#789)
---
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/RootMonitoringPanel.java | 7 ++-
opendj-server-legacy/src/main/java/org/opends/server/loggers/AsynchronousTextWriter.java | 31 +++++++--------
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/ServerDescriptor.java | 7 ++-
opendj-server-legacy/src/main/java/org/opends/server/extensions/GSSAPISASLMechanismHandler.java | 8 +++-
opendj-server-legacy/src/main/java/org/opends/server/loggers/CommonAudit.java | 3 +
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/BaseDNTableModel.java | 7 ++-
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/util/ConfigFromConnection.java | 28 ++++++++++++--
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/util/Utilities.java | 7 ++-
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/renderer/BaseDNCellRenderer.java | 7 ++-
opendj-server-legacy/src/main/java/org/opends/server/extensions/Sha2Crypt.java | 11 ++++-
10 files changed, 77 insertions(+), 39 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/BaseDNTableModel.java b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/BaseDNTableModel.java
index 420ffd5..e99555c 100644
--- a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/BaseDNTableModel.java
+++ b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/BaseDNTableModel.java
@@ -13,6 +13,7 @@
*
* Copyright 2008 Sun Microsystems, Inc.
* Portions Copyright 2013-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.guitools.controlpanel.datamodel;
@@ -45,14 +46,14 @@
private boolean displayReplicationInformation;
/** Key value to identify the case of a value not available because the server is down. */
- public static String NOT_AVAILABLE_SERVER_DOWN = "NOT_AVAILABLE_SERVER_DOWN";
+ public static final String NOT_AVAILABLE_SERVER_DOWN = "NOT_AVAILABLE_SERVER_DOWN";
/** Key value to identify the case of a value not available because authentication is required. */
- public static String NOT_AVAILABLE_AUTHENTICATION_REQUIRED =
+ public static final String NOT_AVAILABLE_AUTHENTICATION_REQUIRED =
"NOT_AVAILABLE_AUTHENTICATION_REQUIRED";
/** Key value to identify the case of a value not available. */
- public static String NOT_AVAILABLE = "NOT_AVAILABLE";
+ public static final String NOT_AVAILABLE = "NOT_AVAILABLE";
/**
* Constructor for this table model.
diff --git a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/ServerDescriptor.java b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/ServerDescriptor.java
index 154f863..e66b6e4 100644
--- a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/ServerDescriptor.java
+++ b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/ServerDescriptor.java
@@ -13,6 +13,7 @@
*
* Copyright 2008-2010 Sun Microsystems, Inc.
* Portions Copyright 2014-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.guitools.controlpanel.datamodel;
@@ -22,6 +23,7 @@
import java.io.File;
import java.io.IOException;
+import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@@ -695,8 +697,9 @@
{
String start = firstValueAsString(rootMonitor, START_DATE.getAttributeName());
String current = firstValueAsString(rootMonitor, CURRENT_DATE.getAttributeName());
- Date startTime = ConfigFromConnection.utcParser.parse(start);
- Date currentTime = ConfigFromConnection.utcParser.parse(current);
+ SimpleDateFormat utcParser = ConfigFromConnection.newUtcParser();
+ Date startTime = utcParser.parse(start);
+ Date currentTime = utcParser.parse(current);
return currentTime.getTime() - startTime.getTime();
}
catch (Throwable t)
diff --git a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/RootMonitoringPanel.java b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/RootMonitoringPanel.java
index 13012a2..a129f74 100644
--- a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/RootMonitoringPanel.java
+++ b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/RootMonitoringPanel.java
@@ -13,11 +13,13 @@
*
* Copyright 2009 Sun Microsystems, Inc.
* Portions Copyright 2014-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.guitools.controlpanel.ui;
import java.awt.Component;
import java.awt.GridBagConstraints;
+import java.text.SimpleDateFormat;
import java.util.Date;
import javax.swing.Box;
@@ -157,8 +159,9 @@
{
String start = firstValueAsString(sr, START_DATE.getAttributeName());
String current = firstValueAsString(sr, CURRENT_DATE.getAttributeName());
- Date startTime = ConfigFromConnection.utcParser.parse(start);
- Date currentTime = ConfigFromConnection.utcParser.parse(current);
+ SimpleDateFormat utcParser = ConfigFromConnection.newUtcParser();
+ Date startTime = utcParser.parse(start);
+ Date currentTime = utcParser.parse(current);
long upSeconds = (currentTime.getTime() - startTime.getTime()) / 1000;
long upDays = upSeconds / 86400;
diff --git a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/renderer/BaseDNCellRenderer.java b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/renderer/BaseDNCellRenderer.java
index 2faa66f..51cc3b3 100644
--- a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/renderer/BaseDNCellRenderer.java
+++ b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/renderer/BaseDNCellRenderer.java
@@ -13,6 +13,7 @@
*
* Copyright 2008 Sun Microsystems, Inc.
* Portions Copyright 2015-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.guitools.controlpanel.ui.renderer;
@@ -38,15 +39,15 @@
public Component getTableCellRendererComponent(JTable table, Object value,
boolean isSelected, boolean hasFocus, int row, int column) {
String text = (String)value;
- if (text == BaseDNTableModel.NOT_AVAILABLE)
+ if (BaseDNTableModel.NOT_AVAILABLE.equals(text))
{
Utilities.setNotAvailable(this);
}
- else if (text == BaseDNTableModel.NOT_AVAILABLE_AUTHENTICATION_REQUIRED)
+ else if (BaseDNTableModel.NOT_AVAILABLE_AUTHENTICATION_REQUIRED.equals(text))
{
Utilities.setNotAvailableBecauseAuthenticationIsRequired(this);
}
- else if (text == BaseDNTableModel.NOT_AVAILABLE_SERVER_DOWN)
+ else if (BaseDNTableModel.NOT_AVAILABLE_SERVER_DOWN.equals(text))
{
Utilities.setNotAvailableBecauseServerIsDown(this);
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/util/ConfigFromConnection.java b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/util/ConfigFromConnection.java
index 7c49b13..edaafa1 100644
--- a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/util/ConfigFromConnection.java
+++ b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/util/ConfigFromConnection.java
@@ -13,6 +13,7 @@
*
* Copyright 2008-2011 Sun Microsystems, Inc.
* Portions Copyright 2013-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.guitools.controlpanel.util;
@@ -135,14 +136,33 @@
}
}
- /** The date formatter to be used to parse GMT dates. */
- public static final SimpleDateFormat utcParser = new SimpleDateFormat(ServerConstants.DATE_FORMAT_GMT_TIME);
+ /**
+ * Returns a date formatter to be used to parse GMT dates.
+ * <p>
+ * A new instance is returned by each call because {@link SimpleDateFormat} is not thread-safe and
+ * this method is called from several threads.
+ *
+ * @return a date formatter to be used to parse GMT dates
+ */
+ public static SimpleDateFormat newUtcParser()
{
+ SimpleDateFormat utcParser = new SimpleDateFormat(ServerConstants.DATE_FORMAT_GMT_TIME);
utcParser.setTimeZone(TimeZone.getTimeZone("UTC"));
+ return utcParser;
}
- /** The date formatter to be used to format dates. */
- public static final DateFormat formatter = DateFormat.getDateTimeInstance();
+ /**
+ * Returns a date formatter to be used to format dates.
+ * <p>
+ * A new instance is returned by each call because {@link DateFormat} is not thread-safe and this
+ * method is called from several threads.
+ *
+ * @return a date formatter to be used to format dates
+ */
+ public static DateFormat newDateFormatter()
+ {
+ return DateFormat.getDateTimeInstance();
+ }
/**
* Returns the monitoring entry for the entry caches.
diff --git a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/util/Utilities.java b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/util/Utilities.java
index 68fb0d6..86dc38a 100644
--- a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/util/Utilities.java
+++ b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/util/Utilities.java
@@ -13,6 +13,7 @@
*
* Copyright 2008-2010 Sun Microsystems, Inc.
* Portions Copyright 2011-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.guitools.controlpanel.util;
@@ -2413,7 +2414,7 @@
}
Long l = Long.parseLong(monitoringValue);
Date date = new Date(l);
- return ConfigFromConnection.formatter.format(date);
+ return ConfigFromConnection.newDateFormatter().format(date);
}
else if (attr.isTime())
{
@@ -2427,8 +2428,8 @@
{
try
{
- Date date = ConfigFromConnection.utcParser.parse(monitoringValue);
- return ConfigFromConnection.formatter.format(date);
+ Date date = ConfigFromConnection.newUtcParser().parse(monitoringValue);
+ return ConfigFromConnection.newDateFormatter().format(date);
}
catch (Throwable t)
{
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/extensions/GSSAPISASLMechanismHandler.java b/opendj-server-legacy/src/main/java/org/opends/server/extensions/GSSAPISASLMechanismHandler.java
index d325a9f..1bc5579 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/extensions/GSSAPISASLMechanismHandler.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/extensions/GSSAPISASLMechanismHandler.java
@@ -13,6 +13,7 @@
*
* Copyright 2006-2009 Sun Microsystems, Inc.
* Portions Copyright 2011-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.extensions;
@@ -207,9 +208,12 @@
{
if (loginContext == null)
{
- loginContext = new LoginContext(
+ // Only publish the login context once the login has completed, otherwise another thread
+ // could obtain it through the unsynchronized check above before it is usable.
+ final LoginContext newLoginContext = new LoginContext(
GSSAPISASLMechanismHandler.class.getName(), this);
- loginContext.login();
+ newLoginContext.login();
+ loginContext = newLoginContext;
}
}
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/extensions/Sha2Crypt.java b/opendj-server-legacy/src/main/java/org/opends/server/extensions/Sha2Crypt.java
index 9bd20cf..99c59fc 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/extensions/Sha2Crypt.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/extensions/Sha2Crypt.java
@@ -34,8 +34,8 @@
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
import java.util.Arrays;
-import java.util.Random;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -82,6 +82,12 @@
"./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
/**
+ * Source of randomness used to generate salts. A single shared instance is used because
+ * {@code SecureRandom} is thread-safe and reseeding it for every value would weaken it.
+ */
+ private static final SecureRandom SALT_RANDOM = new SecureRandom();
+
+ /**
* Base64 like conversion of bytes to ASCII chars.
*
* @param b2
@@ -120,8 +126,7 @@
static String getRandomSalt(int num) {
StringBuilder saltString = new StringBuilder();
for (int i = 1; i <= num; i++) {
- saltString.append(B64T.charAt(new Random().
- nextInt(B64T.length())));
+ saltString.append(B64T.charAt(SALT_RANDOM.nextInt(B64T.length())));
}
return saltString.toString();
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/loggers/AsynchronousTextWriter.java b/opendj-server-legacy/src/main/java/org/opends/server/loggers/AsynchronousTextWriter.java
index cb97035..fe62576 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/loggers/AsynchronousTextWriter.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/loggers/AsynchronousTextWriter.java
@@ -13,6 +13,7 @@
*
* Copyright 2006-2008 Sun Microsystems, Inc.
* Portions Copyright 2013-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.loggers;
@@ -22,6 +23,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import org.forgerock.i18n.LocalizableMessage;
+import org.forgerock.util.Reject;
import org.opends.server.api.DirectoryThread;
import org.opends.server.api.ServerShutdownListener;
import org.opends.server.core.DirectoryServer;
@@ -59,7 +61,7 @@
{
this.name = name;
this.autoFlush = autoFlush;
- this.writer = writer;
+ this.writer = Reject.checkNotNull(writer);
this.queue = new LinkedBlockingQueue<>(capacity);
this.capacity = capacity;
@@ -145,21 +147,18 @@
@Override
public void writeRecord(String record)
{
- // No writer? Off to the bit bucket.
- if (writer != null) {
- while (!stopRequested.get())
+ while (!stopRequested.get())
+ {
+ // Put request on queue for writer
+ try
{
- // Put request on queue for writer
- try
- {
- queue.put(record);
- break;
- }
- catch(InterruptedException e)
- {
- // We expect this to happen. Just ignore it and hopefully
- // drop out in the next try.
- }
+ queue.put(record);
+ break;
+ }
+ catch(InterruptedException e)
+ {
+ // We expect this to happen. Just ignore it and hopefully
+ // drop out in the next try.
}
}
}
@@ -237,7 +236,7 @@
}
// Shutdown the wrapped writer.
- if (shutdownWrapped && writer != null)
+ if (shutdownWrapped)
{
writer.shutdown();
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/loggers/CommonAudit.java b/opendj-server-legacy/src/main/java/org/opends/server/loggers/CommonAudit.java
index 59240a4..c64a4bb 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/loggers/CommonAudit.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/loggers/CommonAudit.java
@@ -12,6 +12,7 @@
* information: "Portions Copyright [year] [name of copyright owner]".
*
* Copyright 2015-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.loggers;
@@ -255,7 +256,7 @@
*/
public void removePublisher(LogPublisherCfg config) throws ConfigException
{
- logger.trace(String.format("Shutting down common audit for configuration entry:", config.dn()));
+ logger.trace(String.format("Shutting down common audit for configuration entry: %s", config.dn()));
String normalizedName = getConfigNormalizedName(config);
try
{
--
Gitblit v1.10.0