This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-rng.git
The following commit(s) were added to refs/heads/master by this push:
new f7937b99 RNG-199: Validate state size before byte allocation
f7937b99 is described below
commit f7937b999d510ac0b2c65a74dfb001f927453c27
Author: Alex Herbert <[email protected]>
AuthorDate: Sun Aug 23 09:04:43 2026 +0100
RNG-199: Validate state size before byte allocation
---
.../apache/commons/rng/simple/JDKRandomBridge.java | 14 +++
.../commons/rng/simple/JDKRandomBridgeTest.java | 110 ++++++++++++++++++++-
src/changes/changes.xml | 5 +
3 files changed, 126 insertions(+), 3 deletions(-)
diff --git
a/commons-rng-simple/src/main/java/org/apache/commons/rng/simple/JDKRandomBridge.java
b/commons-rng-simple/src/main/java/org/apache/commons/rng/simple/JDKRandomBridge.java
index 57749d75..9f601081 100644
---
a/commons-rng-simple/src/main/java/org/apache/commons/rng/simple/JDKRandomBridge.java
+++
b/commons-rng-simple/src/main/java/org/apache/commons/rng/simple/JDKRandomBridge.java
@@ -19,6 +19,7 @@ package org.apache.commons.rng.simple;
import java.io.IOException;
import java.io.ObjectOutputStream;
import java.io.ObjectInputStream;
+import java.io.StreamCorruptedException;
import java.util.Random;
import java.util.concurrent.locks.ReentrantLock;
import org.apache.commons.rng.RestorableUniformRandomProvider;
@@ -42,6 +43,13 @@ import
org.apache.commons.rng.core.RandomProviderDefaultState;
public final class JDKRandomBridge extends Random {
/** Serializable version identifier. */
private static final long serialVersionUID = 20161107L;
+ /**
+ * The maximum supported size of the generator state. This is well above
the
+ * state size of any shipped generator. It bounds the allocation used to
read
+ * the state so that a corrupted or malicious stream declaring a huge size
+ * fails fast instead of forcing an over-sized allocation.
+ */
+ private static final int MAX_STATE_SIZE = 0x10000;
/** Source. */
private final RandomSource source;
/** Delegate. */
@@ -150,7 +158,13 @@ public final class JDKRandomBridge extends Random {
// Avoid the use of input.readObject() to deserialize by manually
reading the byte[].
// Note: ObjectInputStream.readObject() will execute the readObject()
method of the named
// class in the stream which may contain potentially malicious code.
+ // Validate the size before allocation: a corrupted or malicious stream
+ // must not force an arbitrarily large allocation before readFully can
+ // detect truncation.
final int size = input.readInt();
+ if (size < 0 || size > MAX_STATE_SIZE) {
+ throw new StreamCorruptedException("Invalid state size: " + size);
+ }
final byte[] state = new byte[size];
input.readFully(state);
delegate.restoreState(new RandomProviderDefaultState(state));
diff --git
a/commons-rng-simple/src/test/java/org/apache/commons/rng/simple/JDKRandomBridgeTest.java
b/commons-rng-simple/src/test/java/org/apache/commons/rng/simple/JDKRandomBridgeTest.java
index 2d212215..a32f2396 100644
---
a/commons-rng-simple/src/test/java/org/apache/commons/rng/simple/JDKRandomBridgeTest.java
+++
b/commons-rng-simple/src/test/java/org/apache/commons/rng/simple/JDKRandomBridgeTest.java
@@ -21,9 +21,17 @@ import java.io.ObjectOutputStream;
import java.io.ObjectInputStream;
import java.io.ByteArrayOutputStream;
import java.io.ByteArrayInputStream;
+import java.io.StreamCorruptedException;
+import java.util.Arrays;
import java.util.Random;
+import java.util.stream.Stream;
+import org.apache.commons.rng.core.RandomProviderDefaultState;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.EnumSource.Mode;
/**
* Tests for the {@link JDKRandomBridge} adaptor class.
@@ -45,13 +53,22 @@ class JDKRandomBridgeTest {
checkSameSequence(rng1, rng2);
}
- @Test
- void testSerialization()
+ /**
+ * Test serialization with all sources. This ensures the maximum state
size limit
+ * is suitable for all implementations in the library.
+ *
+ * <p>Excludes TWO_CMRES_SELECT which is does not currently save the
subcycle generator
+ * instance in the state. The save/restore functionality is meant to
operate on the same
+ * instance of the generator where the subcycle generators are already
known.
+ */
+ @ParameterizedTest
+ @EnumSource(value=RandomSource.class, mode=Mode.EXCLUDE,
names={"TWO_CMRES_SELECT"})
+ void testSerialization(RandomSource source)
throws IOException,
ClassNotFoundException {
// Initialize.
final long seed = RandomSource.createLong();
- final Random rng = new JDKRandomBridge(RandomSource.SPLIT_MIX_64,
seed);
+ final Random rng = new JDKRandomBridge(source, seed);
// Serialize.
final ByteArrayOutputStream bos = new ByteArrayOutputStream();
@@ -72,6 +89,93 @@ class JDKRandomBridgeTest {
checkSameSequence(rng, serialRng);
}
+ static Stream<RandomSource> testDeserializationWithBadStateSizeThrows() {
+ // Note: This test is not valid for generators where the state bytes are
large
+ // and are written in multiple blocks, e.g. WELL_44497_A.
+ return Stream.of(RandomSource.SPLIT_MIX_64,
+ RandomSource.XO_SHI_RO_128_PP,
+ RandomSource.L128_X256_MIX);
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ void testDeserializationWithBadStateSizeThrows(RandomSource source) throws
IOException {
+ final long seed = 46531265234L;
+ final Random rng = new JDKRandomBridge(source, seed);
+
+ // Serialize.
+ final ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ final ObjectOutputStream oos = new ObjectOutputStream(bos);
+ oos.writeObject(rng);
+ oos.close();
+ final byte[] data = bos.toByteArray();
+
+ // Locate the state size written by the custom writeObject. The custom
class
+ // data is the last data in the stream:
+ // [... block-data header] [int size] [state bytes] [TC_ENDBLOCKDATA]
+ // The expected state size is obtained from an identical generator.
+ // TC_ENDBLOCKDATA = 1 byte:
+ // offset = data.length - 1 - size - 4
+ // Note: A large state may be written in multiple blocks so the test
is not
+ // valid for generators with a large state. These will fail the sanity
check.
+ final byte[] state = ((RandomProviderDefaultState)
+ source.create(seed).saveState()).getState();
+ final int size = state.length;
+ final int offset = data.length - 1 - size - 4;
+ Assertions.assertEquals(size, readInt(data, offset),
+ "Sanity check failed: unexpected state size location");
+
+ // Tamper the declared size: a huge value with no matching payload
must be
+ // rejected before any allocation is attempted.
+ writeInt(data, offset, Integer.MAX_VALUE);
+ assertDeserializationThrows(data);
+
+ // Tamper the declared size: a negative value must be rejected.
+ writeInt(data, offset, -1);
+ assertDeserializationThrows(data);
+ }
+
+ /**
+ * Assert deserialization of the data throws a {@link
StreamCorruptedException}.
+ *
+ * @param data Serialized data.
+ */
+ private static void assertDeserializationThrows(byte[] data) {
+ Assertions.assertThrows(StreamCorruptedException.class, () -> {
+ try (ObjectInputStream ois = new ObjectInputStream(new
ByteArrayInputStream(data))) {
+ ois.readObject();
+ }
+ });
+ }
+
+ /**
+ * Read a big-endian int from the data.
+ *
+ * @param data Data.
+ * @param offset Offset to read from.
+ * @return the int
+ */
+ private static int readInt(byte[] data, int offset) {
+ return ((data[offset] & 0xff) << 24) |
+ ((data[offset + 1] & 0xff) << 16) |
+ ((data[offset + 2] & 0xff) << 8) |
+ (data[offset + 3] & 0xff);
+ }
+
+ /**
+ * Write a big-endian int to the data.
+ *
+ * @param data Data.
+ * @param offset Offset to write to.
+ * @param value Value to write.
+ */
+ private static void writeInt(byte[] data, int offset, int value) {
+ data[offset] = (byte) (value >>> 24);
+ data[offset + 1] = (byte) (value >>> 16);
+ data[offset + 2] = (byte) (value >>> 8);
+ data[offset + 3] = (byte) value;
+ }
+
/**
* Ensure that both generators produce the same sequences.
*
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index fcd4b6ad..5dbbcd17 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -56,6 +56,11 @@ If the output is not quite correct, check for invisible
trailing spaces!
<release version="1.8" date="TBD" description="
New features, updates and bug fixes (requires Java 8).
">
+ <action dev="aherbert" type="update" due-to="Security scan, Alex
Herbert" issue="RNG-199">
+ "JDKRandomBridge": Validate state size before byte allocation during
+ deserialization. Avoids negative array size and fails fast for an
obviously
+ excessive size from a corrupted stream.
+ </action>
<action dev="aherbert" type="update" due-to="Alex Herbert"
issue="RNG-197">
"JDKRandomWrapper": Methods not directly implemented by
java.util.Random
use the default implementation in UniformRandomProvider with
Random.nextLong()
