Harden deserialization against malicious client input
Add three defenses to the Streamable deserialization system to prevent exploitation by hacked clients: - Class whitelist: ObjectInputStream.setAllowedClassPrefixes() validates class names before Class.forName(), blocking arbitrary class loading from crafted wire data. Callers configure at startup; unconfigured streams retain existing behavior for backward compatibility. - Container size caps: BasicStreamers.validateSize() rejects arrays, collections, and maps with negative or >65536 element counts. Prevents OOM from a small frame declaring a massive allocation. Applied to all 12 array/collection read paths and Streamer.ArrayStreamer. - Recursion depth limit: ObjectInputStream.readObject() tracks nesting depth and throws at 32 levels, preventing stack overflow from maliciously nested structures (e.g., list-of-list-of-list).
This commit is contained in:
@@ -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<Class<?>, Streamer> BSTREAMERS =
|
||||
ImmutableMap.<Class<?>, 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<Object> 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<Object, Object> 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<Object> 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();
|
||||
|
||||
@@ -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<String> 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,23 @@ public class ObjectInputStream extends DataInputStream
|
||||
protected ClassMapping createClassMapping (short code, String cname)
|
||||
throws IOException, ClassNotFoundException
|
||||
{
|
||||
// validate the class name against the whitelist before loading
|
||||
if (_allowedPrefixes != null) {
|
||||
boolean allowed = false;
|
||||
for (String prefix : _allowedPrefixes) {
|
||||
if (cname.startsWith(prefix)) {
|
||||
allowed = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!allowed) {
|
||||
log.warning("Blocked deserialization of non-whitelisted class",
|
||||
"code", code, "class", cname);
|
||||
throw new IOException(
|
||||
"Deserialization of class not allowed: " + cname);
|
||||
}
|
||||
}
|
||||
|
||||
// resolve the class and streamer
|
||||
ClassLoader loader = (_loader != null) ? _loader :
|
||||
Thread.currentThread().getContextClassLoader();
|
||||
@@ -331,6 +372,12 @@ public class ObjectInputStream extends DataInputStream
|
||||
/** An optional set of class name translations to use when unserializing objects. */
|
||||
protected Map<String, String> _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<String> _allowedPrefixes;
|
||||
|
||||
/** Used to activate verbose debug logging. */
|
||||
protected static final boolean STREAM_DEBUG = false;
|
||||
}
|
||||
|
||||
@@ -636,7 +636,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 + "]'.");
|
||||
|
||||
Reference in New Issue
Block a user