From 3afb501ed265cf992535dccc8103b204241babc8 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Mon, 3 Aug 2026 14:15:36 +0300 Subject: [PATCH 1/2] Fix CodeQL note-severity alerts: array logging and uncaught NumberFormatException * HostPort and BrowserController logged an InetAddress[]/String[] through string concatenation, which printed "[Ljava.net.InetAddress;@1a2b3c" instead of the addresses (java/print-array). * Two trace messages were missing a space between the concatenated literals (java/missing-space-in-concatenation). * Numbers coming from clients, replication peers, on-disk state and system properties are now parsed with a proper error instead of letting NumberFormatException escape (java/uncaught-number-format-exception): - MemoryBackend rejects a paged results cookie it did not create with a protocol error instead of failing the search with a runtime exception. - Both GSER parsers report an integer which matches the GSER integer pattern but does not fit in an int as a decoding error. - CSN validates the length and the hexadecimal fields of its string representation, and ChangelogBackend reports an invalid replicationCSN assertion value as an attribute syntax error. - ByteArrayScanner reports a malformed number in a replication message as a DataFormatException, like every other parsing error of that scanner. - SubtreeSpecification, SchemaUtils.parseRuleID and GeneralizedTimeSyntax report an out of range number as a syntax error. - ReplicationEnvironment reports a corrupted domains state file, BackupManager a base backup descriptor without a usable last log file size, and the JDBC backend falls back to the default connection TTL instead of failing its class initialization when the TTL system property is not a number. - backendstat reports an invalid entry ID instead of a stack trace. The remaining alerts of this rule in server code are left as they are: the time-of-day values of FixedTimeRotationPolicy are constrained to HHmm by the configuration definition, CtsAccessTokenResolver already converts runtime exceptions into an access token exception, and ProductInformation reads build-generated properties. The alerts in the command line and GUI tools are left for a separate change. --- .../org/forgerock/opendj/ldap/GSERParser.java | 9 +++- .../forgerock/opendj/ldap/MemoryBackend.java | 27 +++++++++++- .../browser/BrowserController.java | 3 +- .../server/backends/ChangelogBackend.java | 16 ++++++- .../backends/jdbc/CachedConnection.java | 28 ++++++++++++- .../backends/pluggable/ID2ChildrenCount.java | 3 +- .../server/backends/pluggable/ID2Entry.java | 26 +++++++++++- .../backends/pluggable/ImportLDIFReader.java | 3 +- .../server/core/PasswordPolicyState.java | 4 +- .../server/protocols/asn1/GSERParser.java | 12 +++++- .../opends/server/replication/common/CSN.java | 25 +++++++---- .../protocol/ByteArrayScanner.java | 21 +++++++++- .../file/ReplicationEnvironment.java | 14 ++++++- .../server/schema/GeneralizedTimeSyntax.java | 6 +-- .../org/opends/server/types/HostPort.java | 7 +++- .../server/types/SubtreeSpecification.java | 10 ++++- .../org/opends/server/util/BackupManager.java | 42 +++++++++++++++++-- .../org/opends/server/util/SchemaUtils.java | 12 +++++- 18 files changed, 233 insertions(+), 35 deletions(-) diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/GSERParser.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/GSERParser.java index 0643b5d67f..60b6bd8878 100644 --- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/GSERParser.java +++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/GSERParser.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2013-2014 Manuel Gaupp + * Portions Copyright 2026 3A Systems, LLC. */ package org.forgerock.opendj.ldap; @@ -321,7 +322,13 @@ public int nextInteger() throws DecodeException { WARN_GSER_NO_VALID_INTEGER.get(gserValue.substring(pos, length)); throw DecodeException.error(msg); } - return Integer.valueOf(next(GSER_INTEGER)).intValue(); + final String integer = next(GSER_INTEGER); + try { + return Integer.parseInt(integer); + } catch (final NumberFormatException e) { + // The value matches the integer pattern but does not fit in an int. + throw DecodeException.error(WARN_GSER_NO_VALID_INTEGER.get(integer), e); + } } /** diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/MemoryBackend.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/MemoryBackend.java index 3060bc113d..6f66414ac6 100644 --- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/MemoryBackend.java +++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/MemoryBackend.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2013-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.forgerock.opendj.ldap; @@ -580,8 +581,7 @@ private void searchWithSubordinates(final RequestContext requestContext, final S } final int pageSize = pagedResults != null ? pagedResults.getSize() : 0; - final int offset = (pagedResults != null && !pagedResults.getCookie().isEmpty()) - ? Integer.valueOf(pagedResults.getCookie().toString()) : 0; + final int offset = decodePagedResultsCookie(pagedResults); int numberOfResults = 0; int position = 0; for (final Entry entry : subtree.values()) { @@ -630,6 +630,29 @@ private void searchWithSubordinates(final RequestContext requestContext, final S resultHandler.handleResult(result); } + /** + * Returns the offset of the first entry to be returned, as encoded by this backend in the cookie + * of the previous page. + * + * @param pagedResults + * The simple paged results control, if present. + * @return The offset of the first entry to be returned. + * @throws LdapException + * If the cookie was not created by this backend. + */ + private static int decodePagedResultsCookie(final SimplePagedResultsControl pagedResults) throws LdapException { + if (pagedResults == null || pagedResults.getCookie().isEmpty()) { + return 0; + } + final String cookie = pagedResults.getCookie().toString(); + try { + return Integer.parseInt(cookie); + } catch (final NumberFormatException e) { + throw newLdapException(newResult(ResultCode.PROTOCOL_ERROR) + .setDiagnosticMessage("Invalid paged results cookie: " + cookie)); + } + } + private R addResultControls(final Request request, final Entry before, final Entry after, final R result) throws LdapException { try { diff --git a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/browser/BrowserController.java b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/browser/BrowserController.java index edfc629eb4..f3c0ab9f01 100644 --- a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/browser/BrowserController.java +++ b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/browser/BrowserController.java @@ -23,6 +23,7 @@ import java.awt.Font; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Enumeration; import java.util.Set; @@ -1838,7 +1839,7 @@ public BrowserNodeInfoImpl(BasicNode node) { sb.append(getURL()); if (getReferral() != null) { sb.append(" -> "); - sb.append(getReferral()); + sb.append(Arrays.toString(getReferral())); } toString = sb.toString(); } diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/ChangelogBackend.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/ChangelogBackend.java index 249f7b0fdb..81f1774f3e 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/ChangelogBackend.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/ChangelogBackend.java @@ -746,7 +746,7 @@ else if (matches(filter, FilterType.EQUALITY, "replicationcsn")) { // == exact CSN // validate provided CSN is correct - new CSN(filter.getAssertionValue().toString()); + decodeCSN(filter.getAssertionValue()); } else if (filter.getFilterType() == FilterType.AND) { @@ -801,6 +801,20 @@ private static long decodeChangeNumber(final ByteString assertionValue) } } + private static CSN decodeCSN(final ByteString assertionValue) + throws DirectoryException + { + try + { + return new CSN(assertionValue.toString()); + } + catch (IllegalArgumentException e) + { + throw new DirectoryException(ResultCode.INVALID_ATTRIBUTE_SYNTAX, + LocalizableMessage.raw("Could not convert value '%s' to a CSN", assertionValue)); + } + } + private boolean matches(SearchFilter filter, FilterType filterType, String primaryName) { return filter.getFilterType() == filterType diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java index 7d36e4e1b5..b0deaf1c35 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java @@ -11,13 +11,15 @@ * Header, with the fields enclosed by brackets [] replaced by your own identifying * information: "Portions Copyright [year] [name of copyright owner]". * - * Copyright 2024-2025 3A Systems, LLC. + * Copyright 2024-2026 3A Systems, LLC. */ package org.opends.server.backends.jdbc; import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.LoadingCache; import com.github.benmanes.caffeine.cache.RemovalCause; +import org.forgerock.i18n.LocalizableMessage; +import org.forgerock.i18n.slf4j.LocalizedLogger; import java.sql.*; import java.time.Duration; @@ -26,10 +28,15 @@ import java.util.concurrent.*; public class CachedConnection implements Connection { + private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass(); + + static final String TTL_PROPERTY = "org.openidentityplatform.opendj.jdbc.ttl"; + static final long DEFAULT_TTL_MS = 15000; + final Connection parent; static LoadingCache> cached = Caffeine.newBuilder() - .expireAfterAccess(Duration.ofMillis(Long.parseLong(System.getProperty("org.openidentityplatform.opendj.jdbc.ttl","15000")))) + .expireAfterAccess(Duration.ofMillis(getCacheTtlMillis())) .removalListener((String key, BlockingQueue value, RemovalCause cause) -> { for (CachedConnection con : value) { try { @@ -43,6 +50,23 @@ public class CachedConnection implements Connection { }) .build(conStr -> new LinkedBlockingQueue<>()); + /** + * Returns the time after which an idle pooled connection is closed, as configured by the + * {@value #TTL_PROPERTY} system property. An invalid value is ignored in favor of the default. + */ + private static long getCacheTtlMillis() { + final String ttl = System.getProperty(TTL_PROPERTY); + if (ttl != null) { + try { + return Long.parseLong(ttl.trim()); + } catch (NumberFormatException e) { + logger.warn(LocalizableMessage.raw("Ignoring invalid value \"%s\" of the %s property, using %d ms", + ttl, TTL_PROPERTY, DEFAULT_TTL_MS)); + } + } + return DEFAULT_TTL_MS; + } + final String connectionString; public CachedConnection(String connectionString, Connection parent) { this.connectionString = connectionString; diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2ChildrenCount.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2ChildrenCount.java index dd57bea879..812300f723 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2ChildrenCount.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2ChildrenCount.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2015 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.backends.pluggable; @@ -129,7 +130,7 @@ public String valueToString(ByteString value) @Override public ByteString generateKey(String data) { - return new EntryID(Long.parseLong(data)).toByteString(); + return new EntryID(ID2Entry.parseEntryID(data)).toByteString(); } /** diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Entry.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Entry.java index be1fd64adf..72bbded077 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Entry.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Entry.java @@ -32,6 +32,8 @@ import java.util.zip.InflaterInputStream; import java.util.zip.InflaterOutputStream; +import org.forgerock.i18n.LocalizableMessage; +import org.forgerock.i18n.LocalizedIllegalArgumentException; import org.forgerock.i18n.slf4j.LocalizedLogger; import org.forgerock.opendj.io.ASN1; import org.forgerock.opendj.io.ASN1Reader; @@ -556,7 +558,27 @@ public String valueToString(ByteString value) @Override public ByteString generateKey(String data) { - EntryID entryID = new EntryID(Long.parseLong(data)); - return entryID.toByteString(); + return new EntryID(parseEntryID(data)).toByteString(); + } + + /** + * Returns the entry ID held by the provided string. + * + * @param data + * The string representation of an entry ID + * @return the parsed entry ID + * @throws LocalizedIllegalArgumentException + * If the provided string does not hold an entry ID + */ + static long parseEntryID(String data) + { + try + { + return Long.parseLong(data); + } + catch (NumberFormatException e) + { + throw new LocalizedIllegalArgumentException(LocalizableMessage.raw("Invalid entry ID: \"%s\"", data)); + } } } diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ImportLDIFReader.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ImportLDIFReader.java index cb553f4db8..71ff855906 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ImportLDIFReader.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ImportLDIFReader.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.backends.pluggable; @@ -211,7 +212,7 @@ private Entry createEntry(List lines, DN entryDN, boolean checkSc { if (logger.isTraceEnabled()) { - logger.trace("Skipping entry %s because reading" + "its attributes failed.", entryDN); + logger.trace("Skipping entry %s because reading its attributes failed.", entryDN); } logToSkipWriter(lines, ERR_LDIF_READ_ATTR_SKIP.get(entryDN, e.getMessage())); return null; diff --git a/opendj-server-legacy/src/main/java/org/opends/server/core/PasswordPolicyState.java b/opendj-server-legacy/src/main/java/org/opends/server/core/PasswordPolicyState.java index 4998dabb2c..8fa24e095e 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/core/PasswordPolicyState.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/core/PasswordPolicyState.java @@ -2700,8 +2700,8 @@ public ByteString generatePassword() { if (logger.isTraceEnabled()) { - logger.trace("Unable to generate a new password for user %s because no password generator has been defined" + - "in the associated password policy.", userDNString); + logger.trace("Unable to generate a new password for user %s because no password generator has been defined " + + "in the associated password policy.", userDNString); } return null; diff --git a/opendj-server-legacy/src/main/java/org/opends/server/protocols/asn1/GSERParser.java b/opendj-server-legacy/src/main/java/org/opends/server/protocols/asn1/GSERParser.java index a43494a66f..92ba2f9d46 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/protocols/asn1/GSERParser.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/protocols/asn1/GSERParser.java @@ -13,6 +13,7 @@ * * Copyright 2013-2014 Manuel Gaupp * Portions Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.protocols.asn1; @@ -382,7 +383,16 @@ public int nextInteger() throws GSERException .substring(pos,length)); throw new GSERException(msg); } - return Integer.valueOf(next(GSER_INTEGER)).intValue(); + final String integer = next(GSER_INTEGER); + try + { + return Integer.parseInt(integer); + } + catch (NumberFormatException e) + { + // The value matches the integer pattern but does not fit in an int. + throw new GSERException(ERR_GSER_NO_VALID_INTEGER.get(integer), e); + } } diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/common/CSN.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/common/CSN.java index 0026223370..74083b4a38 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/replication/common/CSN.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/common/CSN.java @@ -20,6 +20,8 @@ import java.io.Serializable; import java.util.Date; +import org.forgerock.i18n.LocalizableMessage; +import org.forgerock.i18n.LocalizedIllegalArgumentException; import org.forgerock.opendj.ldap.ByteSequence; import org.forgerock.opendj.ldap.ByteSequenceReader; import org.forgerock.opendj.ldap.ByteString; @@ -102,17 +104,26 @@ public static CSN valueOf(ByteSequence bs) * * @param str * the string from which to create a {@link CSN} + * @throws LocalizedIllegalArgumentException + * If the provided string is not a valid {@link #toString()} representation of a CSN */ public CSN(String str) { - String temp = str.substring(0, 16); - timeStamp = Long.parseLong(temp, 16); - - temp = str.substring(16, 20); - serverId = Integer.parseInt(temp, 16); + if (str == null || str.length() < STRING_ENCODING_LENGTH) + { + throw new LocalizedIllegalArgumentException(LocalizableMessage.raw("Invalid CSN: \"%s\"", str)); + } - temp = str.substring(20, 28); - seqnum = Integer.parseInt(temp, 16); + try + { + timeStamp = Long.parseLong(str.substring(0, 16), 16); + serverId = Integer.parseInt(str.substring(16, 20), 16); + seqnum = Integer.parseInt(str.substring(20, STRING_ENCODING_LENGTH), 16); + } + catch (NumberFormatException e) + { + throw new LocalizedIllegalArgumentException(LocalizableMessage.raw("Invalid CSN: \"%s\"", str)); + } } /** diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/ByteArrayScanner.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/ByteArrayScanner.java index 91a39762c3..f247f25fd5 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/ByteArrayScanner.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/ByteArrayScanner.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.replication.protocol; @@ -152,7 +153,15 @@ public long nextLong() throws DataFormatException */ public int nextIntUTF8() throws DataFormatException { - return Integer.valueOf(nextString()); + final String s = nextString(); + try + { + return Integer.parseInt(s); + } + catch (NumberFormatException e) + { + throw new DataFormatException("Expected an int but read \"" + s + "\""); + } } /** @@ -164,7 +173,15 @@ public int nextIntUTF8() throws DataFormatException */ public long nextLongUTF8() throws DataFormatException { - return Long.valueOf(nextString()); + final String s = nextString(); + try + { + return Long.parseLong(s); + } + catch (NumberFormatException e) + { + throw new DataFormatException("Expected a long but read \"" + s + "\""); + } } /** diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/ReplicationEnvironment.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/ReplicationEnvironment.java index d52eb2aef0..870133929f 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/ReplicationEnvironment.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/ReplicationEnvironment.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.replication.server.changelog.file; @@ -731,12 +732,21 @@ private String createDomainId(final DN domainDN) throws ChangelogException } /** Find the next domain id to use. This is the lowest integer that is higher than all existing ids. */ - private String findNextDomainId() + private String findNextDomainId() throws ChangelogException { int nextId = 1; for (final String domainId : domains.values()) { - final Integer id = Integer.valueOf(domainId); + final int id; + try + { + id = Integer.parseInt(domainId); + } + catch (NumberFormatException e) + { + throw new ChangelogException(ERR_CHANGELOG_UNABLE_TO_READ_DOMAIN_STATE_FILE.get( + new File(replicationRootPath, DOMAINS_STATE_FILENAME).getPath()), e); + } if (nextId <= id) { nextId = id + 1; diff --git a/opendj-server-legacy/src/main/java/org/opends/server/schema/GeneralizedTimeSyntax.java b/opendj-server-legacy/src/main/java/org/opends/server/schema/GeneralizedTimeSyntax.java index 075f539fb3..08a9581896 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/schema/GeneralizedTimeSyntax.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/schema/GeneralizedTimeSyntax.java @@ -1200,11 +1200,11 @@ private static long finishDecodingFraction(String value, int startPos, ResultCode.INVALID_ATTRIBUTE_SYNTAX, message); } - double fractionValue = Double.parseDouble(fractionBuffer.toString()); - long additionalMilliseconds = Math.round(fractionValue * multiplier); - try { + double fractionValue = Double.parseDouble(fractionBuffer.toString()); + long additionalMilliseconds = Math.round(fractionValue * multiplier); + GregorianCalendar calendar = new GregorianCalendar(); calendar.setLenient(false); calendar.setTimeZone(timeZone); diff --git a/opendj-server-legacy/src/main/java/org/opends/server/types/HostPort.java b/opendj-server-legacy/src/main/java/org/opends/server/types/HostPort.java index bff0143908..7f3c7469f7 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/types/HostPort.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/types/HostPort.java @@ -24,6 +24,7 @@ import java.net.NetworkInterface; import java.net.SocketException; import java.net.UnknownHostException; +import java.util.Arrays; import java.util.Enumeration; import java.util.HashSet; import java.util.Locale; @@ -497,7 +498,8 @@ public boolean isEquivalentTo(final HostPort other) else if (thisAddresses == null || otherAddresses == null) { if(logger.isTraceEnabled()) { - logger.trace("port and host does not match: " + this + "=" + thisAddresses + "; " + other + "=" + otherAddresses); + logger.trace("port and host does not match: " + this + "=" + Arrays.toString(thisAddresses) + + "; " + other + "=" + Arrays.toString(otherAddresses)); } // One local address and one non-local. return false; @@ -515,7 +517,8 @@ else if (thisAddresses == null || otherAddresses == null) } } if(logger.isTraceEnabled()) { - logger.trace("port and host does not match: " + this + "=" + thisAddresses + "; " + other + "=" + otherAddresses); + logger.trace("port and host does not match: " + this + "=" + Arrays.toString(thisAddresses) + + "; " + other + "=" + Arrays.toString(otherAddresses)); } return false; diff --git a/opendj-server-legacy/src/main/java/org/opends/server/types/SubtreeSpecification.java b/opendj-server-legacy/src/main/java/org/opends/server/types/SubtreeSpecification.java index fc4f60d945..b3acb6f92e 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/types/SubtreeSpecification.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/types/SubtreeSpecification.java @@ -568,7 +568,15 @@ private int nextInt() throws InputMismatchException, NoSuchElementException { final String s = nextValue(INT, INT_TOKEN); - return Integer.parseInt(s); + try + { + return Integer.parseInt(s); + } + catch (NumberFormatException e) + { + // The token matches the integer pattern but is too big to fit in an int. + throw new InputMismatchException(e.getMessage()); + } } /** diff --git a/opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java b/opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java index 98f352228e..2840b9da35 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java @@ -20,6 +20,7 @@ import static java.util.Collections.*; import static org.opends.messages.BackendMessages.*; +import static org.opends.messages.CoreMessages.ERR_BACKUPINFO_CANNOT_DECODE; import static org.opends.messages.UtilityMessages.*; import static org.opends.server.util.ServerConstants.*; import static org.opends.server.util.StaticUtils.*; @@ -515,7 +516,7 @@ private static final class NewBackupArchive { private final NewBackupParams newBackupParams; private final CryptoEngine cryptoEngine; - NewBackupArchive(String backendID, NewBackupParams backupParams, CryptoEngine crypt) + NewBackupArchive(String backendID, NewBackupParams backupParams, CryptoEngine crypt) throws DirectoryException { this.backendID = backendID; this.newBackupParams = backupParams; @@ -525,11 +526,26 @@ private static final class NewBackupArchive { { Map properties = backupParams.baseBackupInfo.getBackupProperties(); latestFileName = properties.get(PROPERTY_LAST_LOGFILE_NAME); - latestFileSize = Long.parseLong(properties.get(PROPERTY_LAST_LOGFILE_SIZE)); + latestFileSize = parseLatestFileSize(backupParams, properties.get(PROPERTY_LAST_LOGFILE_SIZE)); } archiveFilename = BACKUP_BASE_FILENAME + backendID + "-" + backupParams.backupID; } + /** Returns the size recorded by the base backup for the last file it archived. */ + private static long parseLatestFileSize(NewBackupParams backupParams, String size) throws DirectoryException + { + try + { + return Long.parseLong(size); + } + catch (NumberFormatException e) + { + throw new DirectoryException(DirectoryServer.getCoreConfigManager().getServerErrorResultCode(), + ERR_BACKUPINFO_CANNOT_DECODE.get(backupParams.backupDir.getPath(), + PROPERTY_LAST_LOGFILE_SIZE + ": " + size), e); + } + } + String getArchiveFilename() { return archiveFilename; @@ -1561,10 +1577,30 @@ private static int getHighestSuffixNumberForPath(final String basePath) throws I if (pattern.matcher(name).matches()) { String numberAsString = name.substring(baseFile.getPath().length()); - int number = numberAsString.isEmpty() ? 0 : Integer.valueOf(numberAsString); + int number = parseSuffixNumber(numberAsString); highestNumber = number > highestNumber ? number : highestNumber; } } return highestNumber; } + + /** + * Returns the number held by the provided file name suffix, or 0 if the suffix is empty or holds a + * number which is too big to have been generated by this class. + */ + private static int parseSuffixNumber(final String numberAsString) + { + if (numberAsString.isEmpty()) + { + return 0; + } + try + { + return Integer.parseInt(numberAsString); + } + catch (NumberFormatException e) + { + return 0; + } + } } diff --git a/opendj-server-legacy/src/main/java/org/opends/server/util/SchemaUtils.java b/opendj-server-legacy/src/main/java/org/opends/server/util/SchemaUtils.java index 30a95818cb..25ffcdebf5 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/util/SchemaUtils.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/util/SchemaUtils.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.util; @@ -413,7 +414,16 @@ public static String parseDITContentRuleOID(String definition) throws DirectoryE public static int parseRuleID(String definition) throws DirectoryException { // Reuse code of parseOID, even though this is not an OID - return Integer.parseInt(parseOID(definition, ERR_PARSING_DIT_STRUCTURE_RULE_RULEID)); + final String ruleID = parseOID(definition, ERR_PARSING_DIT_STRUCTURE_RULE_RULEID); + try + { + return Integer.parseInt(ruleID); + } + catch (NumberFormatException e) + { + throw new DirectoryException(ResultCode.INVALID_ATTRIBUTE_SYNTAX, + ERR_PARSING_DIT_STRUCTURE_RULE_RULEID.get(definition), e); + } } /** From d05cb2676b2b8e503536ed781035233093be4770 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Mon, 3 Aug 2026 21:07:39 +0300 Subject: [PATCH 2/2] Address review: complete the CSN error conversion, reject a negative JDBC TTL - ByteArrayScanner.nextCSNUTF8() converts LocalizedIllegalArgumentException into the DataFormatException its callers expect (fixes the CI test failure) - CachedConnection falls back to the default TTL on a negative value, which used to reach Caffeine and raise ExceptionInInitializerError - ReplicationEnvironment.readOfflineStateFile and FileReplicaDB .decodeKeyFromString report a corrupted on-disk CSN as ChangelogException - MemoryBackend reports a forged paged results cookie in hexadecimal form and keeps the cause, following the EntryContainer precedent - ChangelogBackend validates the replicationCSN assertion with void validateCSN and keeps the cause - BackupManager traces an ignored backup file suffix and quotes the base path before building the suffix regex - CSN.valueOf(String) documents the thrown exception; SubtreeSpecification uses the no-arg InputMismatchException like the rest of the class - New tests: invalid CSN strings, a non-hexadecimal CSN in a replication message, GSER integer overflow, a forged paged results cookie --- .../forgerock/opendj/ldap/MemoryBackend.java | 4 +++- .../opendj/ldap/GSERParserTestCase.java | 5 +++- .../opendj/ldap/MemoryBackendTestCase.java | 17 ++++++++++++++ .../server/backends/ChangelogBackend.java | 8 +++---- .../backends/jdbc/CachedConnection.java | 11 +++++---- .../opends/server/replication/common/CSN.java | 2 ++ .../protocol/ByteArrayScanner.java | 2 +- .../server/changelog/file/FileReplicaDB.java | 12 +++++++++- .../file/ReplicationEnvironment.java | 5 ++++ .../server/types/SubtreeSpecification.java | 2 +- .../org/opends/server/util/BackupManager.java | 4 +++- .../server/replication/common/CSNTest.java | 23 +++++++++++++++++++ .../replication/protocol/ByteArrayTest.java | 8 +++++++ 13 files changed, 89 insertions(+), 14 deletions(-) diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/MemoryBackend.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/MemoryBackend.java index 6f66414ac6..b5605c6f13 100644 --- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/MemoryBackend.java +++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/MemoryBackend.java @@ -649,7 +649,9 @@ private static int decodePagedResultsCookie(final SimplePagedResultsControl page return Integer.parseInt(cookie); } catch (final NumberFormatException e) { throw newLdapException(newResult(ResultCode.PROTOCOL_ERROR) - .setDiagnosticMessage("Invalid paged results cookie: " + cookie)); + .setDiagnosticMessage( + "Invalid paged results cookie: " + pagedResults.getCookie().toHexString()) + .setCause(e)); } } diff --git a/opendj-core/src/test/java/org/forgerock/opendj/ldap/GSERParserTestCase.java b/opendj-core/src/test/java/org/forgerock/opendj/ldap/GSERParserTestCase.java index 5b63103157..67e59a1c78 100644 --- a/opendj-core/src/test/java/org/forgerock/opendj/ldap/GSERParserTestCase.java +++ b/opendj-core/src/test/java/org/forgerock/opendj/ldap/GSERParserTestCase.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2013-2014 Manuel Gaupp + * Portions Copyright 2026 3A Systems, LLC. */ package org.forgerock.opendj.ldap; @@ -165,7 +166,9 @@ public Object[][] createIntegerValues() { {"", false}, {"0xFF", false}, {"NULL", false}, - {"Not a Number", false} + {"Not a Number", false}, + {"2147483648", false}, + {"99999999999", false} }; } diff --git a/opendj-core/src/test/java/org/forgerock/opendj/ldap/MemoryBackendTestCase.java b/opendj-core/src/test/java/org/forgerock/opendj/ldap/MemoryBackendTestCase.java index ef9ffd01cc..6304de5d5c 100644 --- a/opendj-core/src/test/java/org/forgerock/opendj/ldap/MemoryBackendTestCase.java +++ b/opendj-core/src/test/java/org/forgerock/opendj/ldap/MemoryBackendTestCase.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2013-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.forgerock.opendj.ldap; @@ -548,6 +549,22 @@ public void testSearchPagedResults() throws Exception { assertThat(cookie.isEmpty()).isTrue(); } + @Test + public void testSearchPagedResultsForgedCookie() throws Exception { + final Connection connection = getConnection(); + final SearchRequest search = + Requests.newSearchRequest("ou=people,dc=example,dc=com", SearchScope.WHOLE_SUBTREE, + "(uid=*)"); + search.addControl( + SimplePagedResultsControl.newControl(true, 2, ByteString.valueOfUtf8("forged"))); + try { + connection.search(search, new ArrayList()); + TestCaseUtils.failWasExpected(LdapException.class); + } catch (LdapException e) { + assertThat(e.getResult().getResultCode()).isEqualTo(ResultCode.PROTOCOL_ERROR); + } + } + @Test public void testSimpleBind() throws Exception { final Connection connection = getConnection(); diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/ChangelogBackend.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/ChangelogBackend.java index 81f1774f3e..e294f8ea31 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/ChangelogBackend.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/ChangelogBackend.java @@ -746,7 +746,7 @@ else if (matches(filter, FilterType.EQUALITY, "replicationcsn")) { // == exact CSN // validate provided CSN is correct - decodeCSN(filter.getAssertionValue()); + validateCSN(filter.getAssertionValue()); } else if (filter.getFilterType() == FilterType.AND) { @@ -801,17 +801,17 @@ private static long decodeChangeNumber(final ByteString assertionValue) } } - private static CSN decodeCSN(final ByteString assertionValue) + private static void validateCSN(final ByteString assertionValue) throws DirectoryException { try { - return new CSN(assertionValue.toString()); + new CSN(assertionValue.toString()); } catch (IllegalArgumentException e) { throw new DirectoryException(ResultCode.INVALID_ATTRIBUTE_SYNTAX, - LocalizableMessage.raw("Could not convert value '%s' to a CSN", assertionValue)); + LocalizableMessage.raw("Could not convert value '%s' to a CSN", assertionValue), e); } } diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java index b0deaf1c35..f752ca518e 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java @@ -58,11 +58,14 @@ private static long getCacheTtlMillis() { final String ttl = System.getProperty(TTL_PROPERTY); if (ttl != null) { try { - return Long.parseLong(ttl.trim()); - } catch (NumberFormatException e) { - logger.warn(LocalizableMessage.raw("Ignoring invalid value \"%s\" of the %s property, using %d ms", - ttl, TTL_PROPERTY, DEFAULT_TTL_MS)); + final long millis = Long.parseLong(ttl.trim()); + if (millis >= 0) { + return millis; + } + } catch (NumberFormatException ignored) { } + logger.warn(LocalizableMessage.raw("Ignoring invalid value \"%s\" of the %s property, using %d ms", + ttl, TTL_PROPERTY, DEFAULT_TTL_MS)); } return DEFAULT_TTL_MS; } diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/common/CSN.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/common/CSN.java index 74083b4a38..fc1fdcdbcb 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/replication/common/CSN.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/common/CSN.java @@ -74,6 +74,8 @@ public class CSN implements Serializable, Comparable * @param s * The string to be parsed. * @return The parsed CSN. + * @throws LocalizedIllegalArgumentException + * If the provided string is not a valid {@link #toString()} representation of a CSN * @see #toString() */ public static CSN valueOf(String s) diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/ByteArrayScanner.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/ByteArrayScanner.java index f247f25fd5..6d098501ca 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/ByteArrayScanner.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/ByteArrayScanner.java @@ -284,7 +284,7 @@ public CSN nextCSNUTF8() throws DataFormatException { return CSN.valueOf(nextString()); } - catch (IndexOutOfBoundsException e) + catch (LocalizedIllegalArgumentException | IndexOutOfBoundsException e) { throw new DataFormatException(e.getMessage()); } diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/FileReplicaDB.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/FileReplicaDB.java index 709b87ec77..ac4ec81c8d 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/FileReplicaDB.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/FileReplicaDB.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.replication.server.changelog.file; @@ -25,6 +26,7 @@ import net.jcip.annotations.Immutable; +import org.forgerock.i18n.LocalizedIllegalArgumentException; import org.forgerock.opendj.config.server.ConfigException; import org.forgerock.opendj.ldap.ByteString; import org.forgerock.opendj.ldap.ByteStringBuilder; @@ -415,7 +417,15 @@ public Record decodeRecord(final ByteString data) throws Decodin @Override public CSN decodeKeyFromString(String key) throws ChangelogException { - return new CSN(key); + try + { + return new CSN(key); + } + catch (LocalizedIllegalArgumentException e) + { + throw new ChangelogException( + ERR_CHANGELOG_UNABLE_TO_DECODE_KEY_FROM_STRING.get(key), e); + } } @Override diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/ReplicationEnvironment.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/ReplicationEnvironment.java index 870133929f..ee1282caa6 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/ReplicationEnvironment.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/ReplicationEnvironment.java @@ -699,6 +699,11 @@ private CSN readOfflineStateFile(final File offlineFile, DN domainDN) throws Cha } return new CSN(line); } + catch(LocalizedIllegalArgumentException e) + { + throw new ChangelogException(ERR_CHANGELOG_INVALID_REPLICA_OFFLINE_STATE_FILE.get( + domainDN.toString(), offlineFile.getPath()), e); + } catch(IOException e) { throw new ChangelogException(ERR_CHANGELOG_UNABLE_TO_READ_REPLICA_OFFLINE_STATE_FILE.get( diff --git a/opendj-server-legacy/src/main/java/org/opends/server/types/SubtreeSpecification.java b/opendj-server-legacy/src/main/java/org/opends/server/types/SubtreeSpecification.java index b3acb6f92e..7ff72f467c 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/types/SubtreeSpecification.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/types/SubtreeSpecification.java @@ -575,7 +575,7 @@ private int nextInt() throws InputMismatchException, catch (NumberFormatException e) { // The token matches the integer pattern but is too big to fit in an int. - throw new InputMismatchException(e.getMessage()); + throw new InputMismatchException(); } } diff --git a/opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java b/opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java index 2840b9da35..8eb09b4d1f 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java @@ -1569,7 +1569,7 @@ private static int getHighestSuffixNumberForPath(final String basePath) throws I { final File baseFile = new File(basePath).getCanonicalFile(); final File[] existingFiles = baseFile.getParentFile().listFiles(); - final Pattern pattern = Pattern.compile(baseFile + "\\d*"); + final Pattern pattern = Pattern.compile(Pattern.quote(baseFile.getPath()) + "\\d*"); int highestNumber = 0; for (File file : existingFiles) { @@ -1600,6 +1600,8 @@ private static int parseSuffixNumber(final String numberAsString) } catch (NumberFormatException e) { + logger.trace("Ignoring file suffix \"%s\" which is too big to have been generated by this class", + numberAsString); return 0; } } diff --git a/opendj-server-legacy/src/test/java/org/opends/server/replication/common/CSNTest.java b/opendj-server-legacy/src/test/java/org/opends/server/replication/common/CSNTest.java index 1c65a05d1b..d92e96d429 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/replication/common/CSNTest.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/replication/common/CSNTest.java @@ -22,6 +22,7 @@ import java.util.Iterator; import java.util.List; +import org.forgerock.i18n.LocalizedIllegalArgumentException; import org.opends.server.replication.ReplicationTestCase; import org.opends.server.util.TimeThread; import org.testng.annotations.DataProvider; @@ -74,6 +75,28 @@ public void csnEncodeDecode(long time, int seq, int id, String str) throws Excep "The encoding/decoding of CSN is not reversible for toString()"); } + /** Create invalid CSN string representations. */ + @DataProvider(name = "invalidCSNStrings") + public Object[][] createInvalidCSNStrings() + { + return new Object[][] { + { null }, + { "" }, + { "\u0001" }, // truncated CSN read from a legacy replication message + { "0000000000012abc002d0000007" }, // one character too short + { "000000000001zabc002d0000007b" }, // non hexadecimal timestamp + { "0000000000012abc002d0000007z" }, // non hexadecimal seqnum + }; + } + + /** Test constructor from an invalid String. */ + @Test(dataProvider = "invalidCSNStrings", + expectedExceptions = LocalizedIllegalArgumentException.class) + public void csnDecodeInvalidString(String str) throws Exception + { + new CSN(str); + } + /** Create CSN. */ @DataProvider(name = "createCSN") public Object[][] createCSNData() diff --git a/opendj-server-legacy/src/test/java/org/opends/server/replication/protocol/ByteArrayTest.java b/opendj-server-legacy/src/test/java/org/opends/server/replication/protocol/ByteArrayTest.java index a10c53f011..05cb4f2545 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/replication/protocol/ByteArrayTest.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/replication/protocol/ByteArrayTest.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.replication.protocol; @@ -208,6 +209,13 @@ public void testByteArrayScanner_nextCSNUTF8_throwsExceptionWhenInvalidCSN() thr new ByteArrayScanner(new byte[] { 1, 0 }).nextCSNUTF8(); } + @Test(expectedExceptions = DataFormatException.class) + public void testByteArrayScanner_nextCSNUTF8_throwsExceptionWhenNonHexCSN() throws Exception + { + final byte[] bytes = new ByteArrayBuilder().appendString("000000000001zabc002d0000007b").toByteArray(); + new ByteArrayScanner(bytes).nextCSNUTF8(); + } + @Test(expectedExceptions = DataFormatException.class) public void testByteArrayScanner_nextDN_throwsExceptionWhenInvalidDN() throws Exception {