You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ah...@apache.org on 2019/10/02 16:24:44 UTC
[commons-rng] 01/02: RNG-120: Fix security issues in serialization
code for Random instances.
This is an automated email from the ASF dual-hosted git repository.
aherbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-rng.git
commit f8edf66d33ef6bf7af7ca52224d940b583e7ad65
Author: Alex Herbert <ah...@apache.org>
AuthorDate: Tue Oct 1 00:06:42 2019 +0100
RNG-120: Fix security issues in serialization code for Random instances.
---
.../commons/rng/core/source32/JDKRandom.java | 37 ++++++++++++-
.../commons/rng/core/source32/JDKRandomTest.java | 63 ++++++++++++++++++++++
.../apache/commons/rng/simple/JDKRandomBridge.java | 17 ++++--
.../commons/rng/simple/JDKRandomBridgeTest.java | 2 +-
4 files changed, 114 insertions(+), 5 deletions(-)
diff --git a/commons-rng-core/src/main/java/org/apache/commons/rng/core/source32/JDKRandom.java b/commons-rng-core/src/main/java/org/apache/commons/rng/core/source32/JDKRandom.java
index e9245b1..c152c0a 100644
--- a/commons-rng-core/src/main/java/org/apache/commons/rng/core/source32/JDKRandom.java
+++ b/commons-rng-core/src/main/java/org/apache/commons/rng/core/source32/JDKRandom.java
@@ -18,7 +18,9 @@ package org.apache.commons.rng.core.source32;
import java.util.Random;
import java.io.IOException;
+import java.io.InputStream;
import java.io.ObjectOutputStream;
+import java.io.ObjectStreamClass;
import java.io.ObjectInputStream;
import java.io.ByteArrayOutputStream;
@@ -47,6 +49,38 @@ public class JDKRandom extends IntProvider {
private Random delegate;
/**
+ * An <code>ObjectInputStream</code> that's restricted to deserialize
+ * only {@link java.util.Random} using look-ahead deserialization.
+ *
+ * <p>Adapted from o.a.c.io.serialization.ValidatingObjectInputStream.</p>
+ *
+ * @see <a href="http://www.ibm.com/developerworks/library/se-lookahead/">
+ * IBM DeveloperWorks Article: Look-ahead Java deserialization</a>
+ */
+ private static class ValidatingObjectInputStream extends ObjectInputStream {
+ /**
+ * @param in Input stream
+ * @throws IOException Signals that an I/O exception has occurred.
+ */
+ ValidatingObjectInputStream(final InputStream in) throws IOException {
+ super(in);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ protected Class<?> resolveClass(final ObjectStreamClass osc) throws IOException,
+ ClassNotFoundException {
+ // For legacy reasons the Random class is serialized using only primitives
+ // even though modern implementations use AtomicLong.
+ // The only expected class is java.util.Random.
+ if (!Random.class.getName().equals(osc.getName())) {
+ throw new IllegalStateException("Stream does not contain java.util.Random: " + osc.getName());
+ }
+ return super.resolveClass(osc);
+ }
+ }
+
+ /**
* Creates an instance with the given seed.
*
* @param seed Initial seed.
@@ -98,9 +132,10 @@ public class JDKRandom extends IntProvider {
// Second obtain the state
final byte[][] c = splitStateInternal(s2[1], stateSize);
+ // Use look-ahead deserialization to validate the state byte[] contains java.util.Random.
try {
final ByteArrayInputStream bis = new ByteArrayInputStream(c[0]);
- final ObjectInputStream ois = new ObjectInputStream(bis);
+ final ObjectInputStream ois = new ValidatingObjectInputStream(bis);
delegate = (Random) ois.readObject();
} catch (ClassNotFoundException e) {
diff --git a/commons-rng-core/src/test/java/org/apache/commons/rng/core/source32/JDKRandomTest.java b/commons-rng-core/src/test/java/org/apache/commons/rng/core/source32/JDKRandomTest.java
index 0e86baf..39baabd 100644
--- a/commons-rng-core/src/test/java/org/apache/commons/rng/core/source32/JDKRandomTest.java
+++ b/commons-rng-core/src/test/java/org/apache/commons/rng/core/source32/JDKRandomTest.java
@@ -16,13 +16,47 @@
*/
package org.apache.commons.rng.core.source32;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
import java.util.Random;
import org.apache.commons.rng.RandomProviderState;
+import org.apache.commons.rng.core.RandomProviderDefaultState;
+import org.apache.commons.rng.core.util.NumberFactory;
import org.junit.Assert;
import org.junit.Test;
public class JDKRandomTest {
+ /**
+ * A class that is Serializable.
+ * It contains member fields so there is something to serialize and malicious
+ * deserialization code.
+ */
+ static class SerializableTestObject implements Serializable {
+ private static final long serialVersionUID = 1L;
+
+ private int state0;
+ private double state1;
+ private long state2;
+ private boolean stte3;
+
+ /**
+ * This simulates doing something malicious when deserializing.
+ *
+ * @param input Input stream.
+ * @throws IOException if an error occurs.
+ * @throws ClassNotFoundException if an error occurs.
+ */
+ private void readObject(ObjectInputStream input)
+ throws IOException,
+ ClassNotFoundException {
+ Assert.fail("This should not be run during the test");
+ }
+ }
+
@Test
public void testReferenceCode() {
final long refSeed = -1357111213L;
@@ -64,4 +98,33 @@ public class JDKRandomTest {
Assert.assertEquals(r + " nextInt", rng1.nextInt(), rng2.nextInt());
}
}
+
+ /**
+ * Test the deserialization code identifies bad states that do not contain a Random instance.
+ * This test exercises the code that uses a custom deserialization ObjectInputStream.
+ *
+ * @throws IOException Signals that an I/O exception has occurred.
+ */
+ @Test(expected = IllegalStateException.class)
+ public void testRestoreWithInvalidClass() throws IOException {
+ // Serialize something
+ final ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ final ObjectOutputStream oos = new ObjectOutputStream(bos);
+ oos.writeObject(new SerializableTestObject());
+ oos.close();
+
+ // Compose the size with the state.
+ // This is what is expected by the JDKRandom class.
+ final byte[] state = bos.toByteArray();
+ final int stateSize = state.length;
+ final byte[] sizeAndState = new byte[4 + stateSize];
+ System.arraycopy(NumberFactory.makeByteArray(stateSize), 0, sizeAndState, 0, 4);
+ System.arraycopy(state, 0, sizeAndState, 4, stateSize);
+
+ final RandomProviderDefaultState dummyState = new RandomProviderDefaultState(sizeAndState);
+
+ final JDKRandom rng = new JDKRandom(13L);
+ // This should throw
+ rng.restoreState(dummyState);
+ }
}
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 78fc60b..02b0f29 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
@@ -106,8 +106,13 @@ public final class JDKRandomBridge extends Random {
// Write non-transient fields.
output.defaultWriteObject();
- // Save current state.
- output.writeObject(((RandomProviderDefaultState) delegate.saveState()).getState());
+ // Save current state and size.
+ // Avoid the use of ObjectOutputStream.writeObject(Object) to save the state.
+ // This allows deserialization to avoid security issues in using readObject().
+ final byte[] state = ((RandomProviderDefaultState) delegate.saveState()).getState();
+ final int size = state.length;
+ output.writeInt(size);
+ output.write(state);
}
}
@@ -125,6 +130,12 @@ public final class JDKRandomBridge extends Random {
// Recreate the "delegate" from serialized info.
delegate = RandomSource.create(source);
// And restore its state.
- delegate.restoreState(new RandomProviderDefaultState((byte[]) input.readObject()));
+ // 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.
+ final int size = input.readInt();
+ 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 70fa450..6e735c7 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
@@ -63,7 +63,7 @@ public class JDKRandomBridgeTest {
ObjectInputStream ois = new ObjectInputStream(bis);
final Random serialRng = (Random) (ois.readObject());
- // Check that the serialized data recreated the orginal state.
+ // Check that the serialized data recreated the original state.
checkSameSequence(rng, serialRng);
}