diff --git a/core/src/main/java/com/threerings/io/BasicStreamers.java b/core/src/main/java/com/threerings/io/BasicStreamers.java index a275c141e..09059afd2 100644 --- a/core/src/main/java/com/threerings/io/BasicStreamers.java +++ b/core/src/main/java/com/threerings/io/BasicStreamers.java @@ -27,6 +27,29 @@ import com.google.common.collect.Sets; */ public class BasicStreamers { + /** + * The maximum number of elements allowed in a deserialized array, collection, or map. This + * prevents a malicious client from sending a huge size value to trigger an OutOfMemoryError. + */ + public static final int MAX_CONTAINER_SIZE = 65536; + + /** + * Validates that a container size read from the stream is non-negative and within bounds. + * + * @param size the size value read from the stream + * @return the validated size + * @throws IOException if the size is negative or exceeds {@link #MAX_CONTAINER_SIZE} + */ + public static int validateSize (int size) + throws IOException + { + if (size < 0 || size > MAX_CONTAINER_SIZE) { + throw new IOException( + "Invalid container size: " + size + " (max " + MAX_CONTAINER_SIZE + ")"); + } + return size; + } + public static final Map, Streamer> BSTREAMERS = ImmutableMap., Streamer>builder() .put(Boolean.class, new BooleanStreamer()) @@ -445,7 +468,7 @@ public class BasicStreamers public Object createObject (ObjectInputStream in) throws IOException, ClassNotFoundException { - int size = in.readInt(); + int size = validateSize(in.readInt()); Collection coll = createCollection(size); for (int ii = 0; ii < size; ii++) { coll.add(in.readObject()); @@ -514,7 +537,7 @@ public class BasicStreamers public Object createObject (ObjectInputStream in) throws IOException, ClassNotFoundException { - int size = in.readInt(); + int size = validateSize(in.readInt()); Map map = createMap(size); for (int ii = 0; ii < size; ii++) { map.put(in.readObject(), in.readObject()); @@ -550,7 +573,7 @@ public class BasicStreamers public Object createObject (ObjectInputStream in) throws IOException, ClassNotFoundException { - int size = in.readInt(); + int size = validateSize(in.readInt()); Multiset set = createMultiset(size); for (int ii = 0; ii < size; ii++) { set.add(in.readObject(), in.readInt()); @@ -586,7 +609,7 @@ public class BasicStreamers public static boolean[] readBooleanArray (ObjectInputStream ins) throws IOException { - boolean[] value = new boolean[ins.readInt()]; + boolean[] value = new boolean[validateSize(ins.readInt())]; int ecount = value.length; for (int ii = 0; ii < ecount; ii++) { value[ii] = ins.readBoolean(); @@ -597,7 +620,7 @@ public class BasicStreamers public static byte[] readByteArray (ObjectInputStream ins) throws IOException { - byte[] value = new byte[ins.readInt()]; + byte[] value = new byte[validateSize(ins.readInt())]; int remain = value.length, offset = 0, read; while (remain > 0) { if ((read = ins.read(value, offset, remain)) > 0) { @@ -613,7 +636,7 @@ public class BasicStreamers public static short[] readShortArray (ObjectInputStream ins) throws IOException { - short[] value = new short[ins.readInt()]; + short[] value = new short[validateSize(ins.readInt())]; int ecount = value.length; for (int ii = 0; ii < ecount; ii++) { value[ii] = ins.readShort(); @@ -624,7 +647,7 @@ public class BasicStreamers public static char[] readCharArray (ObjectInputStream ins) throws IOException { - char[] value = new char[ins.readInt()]; + char[] value = new char[validateSize(ins.readInt())]; int ecount = value.length; for (int ii = 0; ii < ecount; ii++) { value[ii] = ins.readChar(); @@ -635,7 +658,7 @@ public class BasicStreamers public static int[] readIntArray (ObjectInputStream ins) throws IOException { - int[] value = new int[ins.readInt()]; + int[] value = new int[validateSize(ins.readInt())]; int ecount = value.length; for (int ii = 0; ii < ecount; ii++) { value[ii] = ins.readInt(); @@ -646,7 +669,7 @@ public class BasicStreamers public static long[] readLongArray (ObjectInputStream ins) throws IOException { - long[] value = new long[ins.readInt()]; + long[] value = new long[validateSize(ins.readInt())]; int ecount = value.length; for (int ii = 0; ii < ecount; ii++) { value[ii] = ins.readLong(); @@ -657,7 +680,7 @@ public class BasicStreamers public static float[] readFloatArray (ObjectInputStream ins) throws IOException { - float[] value = new float[ins.readInt()]; + float[] value = new float[validateSize(ins.readInt())]; int ecount = value.length; for (int ii = 0; ii < ecount; ii++) { value[ii] = ins.readFloat(); @@ -668,7 +691,7 @@ public class BasicStreamers public static double[] readDoubleArray (ObjectInputStream ins) throws IOException { - double[] value = new double[ins.readInt()]; + double[] value = new double[validateSize(ins.readInt())]; int ecount = value.length; for (int ii = 0; ii < ecount; ii++) { value[ii] = ins.readDouble(); @@ -679,7 +702,7 @@ public class BasicStreamers public static Object[] readObjectArray (ObjectInputStream ins) throws IOException, ClassNotFoundException { - Object[] value = new Object[ins.readInt()]; + Object[] value = new Object[validateSize(ins.readInt())]; int ecount = value.length; for (int ii = 0; ii < ecount; ii++) { value[ii] = ins.readObject(); diff --git a/core/src/main/java/com/threerings/io/ObjectInputStream.java b/core/src/main/java/com/threerings/io/ObjectInputStream.java index 108fb315d..c12330d5c 100644 --- a/core/src/main/java/com/threerings/io/ObjectInputStream.java +++ b/core/src/main/java/com/threerings/io/ObjectInputStream.java @@ -7,11 +7,13 @@ package com.threerings.io; import java.util.List; import java.util.Map; +import java.util.Set; import java.io.DataInputStream; import java.io.IOException; import java.io.InputStream; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -27,6 +29,21 @@ import static com.threerings.NaryaLog.log; */ public class ObjectInputStream extends DataInputStream { + /** The maximum allowed recursion depth when reading nested objects. */ + public static final int MAX_OBJECT_DEPTH = 32; + + /** + * Configures the set of allowed package prefixes for class deserialization. Only classes whose + * fully qualified names start with one of the specified prefixes will be allowed to load. This + * should be called once at server startup before any client connections are accepted. + * + * @param prefixes the allowed package prefixes (e.g., "com.threerings.", "com.samskivert.") + */ + public static void setAllowedClassPrefixes (Set prefixes) + { + _allowedPrefixes = ImmutableSet.copyOf(prefixes); + } + /** * Constructs an object input stream which will read its data from the supplied source stream. */ @@ -63,6 +80,11 @@ public class ObjectInputStream extends DataInputStream public Object readObject () throws IOException, ClassNotFoundException { + if (_depth >= MAX_OBJECT_DEPTH) { + throw new IOException("Maximum object nesting depth exceeded (" + + MAX_OBJECT_DEPTH + "); possible malicious input"); + } + _depth++; try { // read the class mapping ClassMapping cmap = readClassMapping(); @@ -84,6 +106,8 @@ public class ObjectInputStream extends DataInputStream } catch (OutOfMemoryError oome) { throw (IOException)new IOException("Malformed object data").initCause(oome); + } finally { + _depth--; } } @@ -222,6 +246,10 @@ public class ObjectInputStream extends DataInputStream protected ClassMapping createClassMapping (short code, String cname) throws IOException, ClassNotFoundException { + if (_allowedPrefixes != null) { + validateClassPrefix(code, cname); + } + // resolve the class and streamer ClassLoader loader = (_loader != null) ? _loader : Thread.currentThread().getContextClassLoader(); @@ -241,6 +269,40 @@ public class ObjectInputStream extends DataInputStream return new ClassMapping(code, sclass, streamer); } + /** + * Validates that a class name is allowed by the configured prefix whitelist. Array prefixes + * ({@code [}) are stripped first. Primitive type descriptors ({@code I}, {@code Z}, etc.) are + * always allowed. Object type descriptors ({@code Lcom.threerings.Foo;}) are checked against + * the whitelist. + */ + protected void validateClassPrefix (short code, String cname) + throws IOException + { + // strip array prefixes to get the component type descriptor + int start = 0; + while (start < cname.length() && cname.charAt(start) == '[') { + start++; + } + String desc = cname.substring(start); + + // primitive descriptors (I, Z, B, S, C, J, F, D) are always safe + if (!desc.startsWith("L") || !desc.endsWith(";")) { + return; + } + + // extract the class name from "Lcom.threerings.Foo;" + String className = desc.substring(1, desc.length() - 1); + for (String prefix : _allowedPrefixes) { + if (className.startsWith(prefix)) { + return; + } + } + + log.warning("Blocked deserialization of non-whitelisted class", + "code", code, "class", cname); + throw new IOException("Deserialization of class not allowed: " + cname); + } + /** * Reads an object from the input stream that was previously written with {@link * ObjectOutputStream#writeBareObject(Object)}. @@ -331,6 +393,12 @@ public class ObjectInputStream extends DataInputStream /** An optional set of class name translations to use when unserializing objects. */ protected Map _translations; + /** Tracks the current object nesting depth to prevent stack overflow from malicious input. */ + protected int _depth; + + /** If set, only classes with names starting with one of these prefixes may be deserialized. */ + protected static volatile Set _allowedPrefixes; + /** Used to activate verbose debug logging. */ protected static final boolean STREAM_DEBUG = false; } diff --git a/core/src/main/java/com/threerings/io/Streamer.java b/core/src/main/java/com/threerings/io/Streamer.java index c693f5007..5e725b4a6 100644 --- a/core/src/main/java/com/threerings/io/Streamer.java +++ b/core/src/main/java/com/threerings/io/Streamer.java @@ -639,7 +639,7 @@ public abstract class Streamer public Object createObject (ObjectInputStream in) throws IOException, ClassNotFoundException { - int length = in.readInt(); + int length = BasicStreamers.validateSize(in.readInt()); if (ObjectInputStream.STREAM_DEBUG) { log.info(in.hashCode() + ": Creating array '" + _componentType.getName() + "[" + length + "]'."); diff --git a/core/src/main/java/com/threerings/presents/client/BlockingCommunicator.java b/core/src/main/java/com/threerings/presents/client/BlockingCommunicator.java index 574a5143c..e86eaad11 100644 --- a/core/src/main/java/com/threerings/presents/client/BlockingCommunicator.java +++ b/core/src/main/java/com/threerings/presents/client/BlockingCommunicator.java @@ -660,6 +660,7 @@ public class BlockingCommunicator extends Communicator InetAddress host = InetAddress.getByName(_client.getHostname()); openChannel(host); _channel.configureBlocking(true); + _channel.socket().setKeepAlive(true); // our messages are framed (preceded by their length), so we use these helper streams // to manage the framing diff --git a/core/src/main/java/com/threerings/presents/net/PingRequest.java b/core/src/main/java/com/threerings/presents/net/PingRequest.java index c401b8de1..77b207bb4 100644 --- a/core/src/main/java/com/threerings/presents/net/PingRequest.java +++ b/core/src/main/java/com/threerings/presents/net/PingRequest.java @@ -17,7 +17,7 @@ public class PingRequest extends UpstreamMessage { /** The number of milliseconds of idle upstream that are allowed to elapse before the client * sends a ping message to the server to let it know that we're still alive. */ - public static final long PING_INTERVAL = 60 * 1000L; + public static final long PING_INTERVAL = 30 * 1000L; /** * Zero argument constructor used when unserializing an instance.