From bfed0b8f320aa15d24da84ffe117d166a5b062bd Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Mon, 27 Jul 2026 17:22:35 +0000
Subject: [PATCH] Fix java/tainted-arithmetic CodeQL alerts (#765)
---
opendj-server-legacy/src/main/java/org/opends/server/replication/common/CSN.java | 5 +-
opendj-core/src/main/java/org/forgerock/opendj/ldap/ByteSequenceReader.java | 6 ++
opendj-server-legacy/src/test/java/org/opends/server/replication/common/CSNTest.java | 27 ++++++++++++-
opendj-core/src/test/java/org/forgerock/opendj/ldap/ByteSequenceReaderTest.java | 9 ++++
opendj-core/src/main/java/org/forgerock/opendj/ldap/GeneralizedTime.java | 27 ++++++++++++-
opendj-core/src/test/java/org/forgerock/opendj/ldap/GeneralizedTimeTest.java | 7 +++
6 files changed, 71 insertions(+), 10 deletions(-)
diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/ByteSequenceReader.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/ByteSequenceReader.java
index c09d546..2331d15 100644
--- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/ByteSequenceReader.java
+++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/ByteSequenceReader.java
@@ -499,7 +499,11 @@
* of the underlying byte sequence.
*/
public void skip(final int length) {
- position(pos + length);
+ final long newPos = (long) pos + length;
+ if (newPos < 0 || newPos > sequence.length()) {
+ throw new IndexOutOfBoundsException();
+ }
+ position((int) newPos);
}
@Override
diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/GeneralizedTime.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/GeneralizedTime.java
index 6045ac8..d58dbcf 100644
--- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/GeneralizedTime.java
+++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/GeneralizedTime.java
@@ -12,6 +12,7 @@
* information: "Portions Copyright [year] [name of copyright owner]".
*
* Copyright 2012-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.forgerock.opendj.ldap;
@@ -653,15 +654,35 @@
throw new LocalizedIllegalArgumentException(message);
}
- final Double fractionValue = Double.parseDouble(fractionBuffer.toString());
- final int additionalMilliseconds = (int) Math.round(fractionValue * multiplier);
+ final double fractionValue;
+ try {
+ fractionValue = Double.parseDouble(fractionBuffer.toString());
+ } catch (final NumberFormatException e) {
+ final LocalizableMessage message =
+ WARN_ATTR_SYNTAX_GENERALIZED_TIME_ILLEGAL_TIME.get(value, fractionBuffer);
+ throw new LocalizedIllegalArgumentException(message, e);
+ }
+ if (fractionValue < 0.0d || fractionValue >= 1.0d) {
+ // "0." followed by 17 nines parses as exactly 1.0, so reject before scaling.
+ final LocalizableMessage message =
+ WARN_ATTR_SYNTAX_GENERALIZED_TIME_ILLEGAL_TIME.get(value, fractionBuffer);
+ throw new LocalizedIllegalArgumentException(message);
+ }
+ final long additionalMilliseconds = Math.round(fractionValue * multiplier);
+ if (additionalMilliseconds >= multiplier) {
+ // 16 nines: 0.9999999999999999 * 1000 still rounds up to 1000. The calendar below
+ // only validates lazily, so reject the out-of-range value here.
+ final LocalizableMessage message =
+ WARN_ATTR_SYNTAX_GENERALIZED_TIME_ILLEGAL_TIME.get(value, fractionBuffer);
+ throw new LocalizedIllegalArgumentException(message);
+ }
try {
final GregorianCalendar calendar = new GregorianCalendar();
calendar.setLenient(false);
calendar.setTimeZone(timeZone);
calendar.set(year, month, day, hour, minute, second);
- calendar.set(Calendar.MILLISECOND, additionalMilliseconds);
+ calendar.set(Calendar.MILLISECOND, (int) additionalMilliseconds);
return new GeneralizedTime(calendar, null, Long.MIN_VALUE, value);
} catch (final Exception e) {
// This should only happen if the provided date wasn't legal
diff --git a/opendj-core/src/test/java/org/forgerock/opendj/ldap/ByteSequenceReaderTest.java b/opendj-core/src/test/java/org/forgerock/opendj/ldap/ByteSequenceReaderTest.java
index 2b60f18..d7f3358 100644
--- a/opendj-core/src/test/java/org/forgerock/opendj/ldap/ByteSequenceReaderTest.java
+++ b/opendj-core/src/test/java/org/forgerock/opendj/ldap/ByteSequenceReaderTest.java
@@ -13,6 +13,7 @@
*
* Copyright 2009 Sun Microsystems, Inc.
* Portions Copyright 2014-2015 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.forgerock.opendj.ldap;
@@ -347,6 +348,14 @@
reader.skip(1);
}
+ @Test(dataProvider = "readerProvider", expectedExceptions = IndexOutOfBoundsException.class)
+ public void testSkipOverflow(ByteSequenceReader reader, byte[] ba) {
+ reader.rewind();
+ reader.skip(ba.length);
+ // pos + Integer.MAX_VALUE overflows int; must throw, not wrap around.
+ reader.skip(Integer.MAX_VALUE);
+ }
+
@Test(dataProvider = "readerProvider")
public void testPeek(ByteSequenceReader reader, byte[] ba) {
reader.rewind();
diff --git a/opendj-core/src/test/java/org/forgerock/opendj/ldap/GeneralizedTimeTest.java b/opendj-core/src/test/java/org/forgerock/opendj/ldap/GeneralizedTimeTest.java
index a08e020..4066fa4 100644
--- a/opendj-core/src/test/java/org/forgerock/opendj/ldap/GeneralizedTimeTest.java
+++ b/opendj-core/src/test/java/org/forgerock/opendj/ldap/GeneralizedTimeTest.java
@@ -13,6 +13,7 @@
*
* Copyright 2009 Sun Microsystems, Inc.
* Portions copyright 2012-2015 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.forgerock.opendj.ldap;
@@ -67,7 +68,11 @@
{ "2006122a235959Z" }, { "20060031235959Z" }, { "20061331235959Z" },
{ "20062231235959Z" }, { "20061232235959Z" }, { "2006123123595aZ" },
{ "200a1231235959Z" }, { "2006j231235959Z" }, { "200612-1235959Z" },
- { "20061231#35959Z" }, { "2006" }, };
+ { "20061231#35959Z" }, { "2006" },
+ // Double.parseDouble rounds 17 nines up to exactly 1.0.
+ { "20240101000000.99999999999999999Z" },
+ // 16 nines stay below 1.0 but scale and round to 1000 milliseconds.
+ { "20240101000000.9999999999999999Z" }, };
}
@Test
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 24f2c04..0026223 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
@@ -13,6 +13,7 @@
*
* Copyright 2006-2009 Sun Microsystems, Inc.
* Portions Copyright 2013-2015 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.replication.common;
@@ -319,11 +320,11 @@
}
else if (csn1.seqnum != csn2.seqnum)
{
- return csn1.seqnum - csn2.seqnum;
+ return Integer.compare(csn1.seqnum, csn2.seqnum);
}
else
{
- return csn1.serverId - csn2.serverId;
+ return Integer.compare(csn1.serverId, csn2.serverId);
}
}
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 307f399..1c65a05 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
@@ -13,6 +13,7 @@
*
* Copyright 2006-2009 Sun Microsystems, Inc.
* Portions Copyright 2013-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.replication.common;
@@ -77,9 +78,9 @@
@DataProvider(name = "createCSN")
public Object[][] createCSNData()
{
- long time[] = {1, TimeThread.getTime()};
- int seq[] = {0, 123};
- int id [] = {1, 45};
+ long time[] = {1, TimeThread.getTime(), 1, 1};
+ int seq[] = {0, 123, Integer.MIN_VALUE, Integer.MAX_VALUE - 1};
+ int id [] = {1, 45, 1, 1};
Object[][] obj = new Object[time.length][5];
for (int i=0; i<time.length; i++)
@@ -140,6 +141,26 @@
assertTrue(CSN.compare(csn5, csn1) > 0);
}
+ /**
+ * Test that {@link CSN#compare(CSN, CSN)} orders extreme seqnums correctly and
+ * transitively: subtraction-based comparison overflowed for MIN_VALUE vs
+ * MAX_VALUE at equal timestamps and inverted the sign, which is fatal for
+ * TreeMaps keyed on CSN.
+ */
+ @Test
+ public void csnCompareExtremeSeqnums() throws Exception
+ {
+ final CSN minSeqnum = new CSN(1, Integer.MIN_VALUE, 1);
+ final CSN zeroSeqnum = new CSN(1, 0, 1);
+ final CSN maxSeqnum = new CSN(1, Integer.MAX_VALUE, 1);
+
+ assertTrue(CSN.compare(minSeqnum, zeroSeqnum) < 0);
+ assertTrue(CSN.compare(zeroSeqnum, maxSeqnum) < 0);
+ assertTrue(CSN.compare(minSeqnum, maxSeqnum) < 0);
+ assertTrue(CSN.compare(maxSeqnum, minSeqnum) > 0);
+ assertEquals(CSN.compare(maxSeqnum, maxSeqnum), 0);
+ }
+
/** Test {@link CSN#isOlderThan(CSN)} method. */
@Test(dataProvider = "createCSN")
public void csnOlder(CSN csn1, CSN csn2, CSN csn3, CSN csn4, CSN csn5) throws Exception
--
Gitblit v1.10.0