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:43 UTC
[commons-rng] branch master updated (6678166 -> d7133fe)
This is an automated email from the ASF dual-hosted git repository.
aherbert pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/commons-rng.git.
from 6678166 Refactored the JMH examples to packages for each component.
new f8edf66 RNG-120: Fix security issues in serialization code for Random instances.
new d7133fe RNG-120: Track changes.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../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 +-
src/changes/changes.xml | 4 ++
5 files changed, 118 insertions(+), 5 deletions(-)
[commons-rng] 01/02: RNG-120: Fix security issues in serialization
code for Random instances.
Posted by ah...@apache.org.
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);
}
[commons-rng] 02/02: RNG-120: Track changes.
Posted by ah...@apache.org.
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 d7133fec195a687bfdddfd79f4c2b1ea6a940bbc
Author: aherbert <ah...@apache.org>
AuthorDate: Wed Oct 2 17:24:39 2019 +0100
RNG-120: Track changes.
---
src/changes/changes.xml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index d75ec75..795e256 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -75,6 +75,10 @@ re-run tests that fail, and pass the build if they succeed
within the allotted number of reruns (the test will be marked
as 'flaky' in the report).
">
+ <action dev="aherbert" type="update" issue="RNG-120">
+ Update security of serialization code for java.util.Random instances. Implement
+ look-ahead deserialization or remove the use of ObjectInputStream.readObject().
+ </action>
<action dev="aherbert" type="update" issue="RNG-76">
"SplitMix64": Added primitive long constructor.
</action>