From b19d36e93969188451a56bda37eb8253b8c38b74 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 9 Jun 2019 04:23:21 -0400 Subject: [PATCH] Strictly enforce packet size limits for incoming compressed packets --- .../natives/compression/CompressorUtils.java | 28 +++++++++++++++++++ .../compression/JavaVelocityCompressor.java | 6 ++-- .../compression/NativeVelocityCompressor.java | 10 ++++--- .../compression/VelocityCompressor.java | 2 +- .../compression/VelocityCompressorTest.java | 2 +- .../netty/MinecraftCompressDecoder.java | 12 ++++---- 6 files changed, 45 insertions(+), 15 deletions(-) create mode 100644 native/src/main/java/com/velocitypowered/natives/compression/CompressorUtils.java diff --git a/native/src/main/java/com/velocitypowered/natives/compression/CompressorUtils.java b/native/src/main/java/com/velocitypowered/natives/compression/CompressorUtils.java new file mode 100644 index 000000000..e31c6b5da --- /dev/null +++ b/native/src/main/java/com/velocitypowered/natives/compression/CompressorUtils.java @@ -0,0 +1,28 @@ +package com.velocitypowered.natives.compression; + +import io.netty.buffer.ByteBuf; +import java.util.zip.DataFormatException; + +class CompressorUtils { + /** + * The default preferred output buffer size for zlib. + */ + static final int ZLIB_BUFFER_SIZE = 8192; + + /** + * Ensures that the buffer does not go over {@code max}. + * @param buf the buffer for check + * @param max the maximum size for the buffer + * @throws DataFormatException if the buffer becomes too bug + */ + static void ensureMaxSize(ByteBuf buf, int max) throws DataFormatException { + int len = buf.readableBytes(); + if (len > max) { + throw new DataFormatException("Got too much data (" + len + " > " + max + ")"); + } + } + + private CompressorUtils() { + throw new AssertionError(); + } +} diff --git a/native/src/main/java/com/velocitypowered/natives/compression/JavaVelocityCompressor.java b/native/src/main/java/com/velocitypowered/natives/compression/JavaVelocityCompressor.java index c948e6ee4..416a5e6fa 100644 --- a/native/src/main/java/com/velocitypowered/natives/compression/JavaVelocityCompressor.java +++ b/native/src/main/java/com/velocitypowered/natives/compression/JavaVelocityCompressor.java @@ -1,6 +1,7 @@ package com.velocitypowered.natives.compression; -import static com.velocitypowered.natives.util.NativeConstants.ZLIB_BUFFER_SIZE; +import static com.velocitypowered.natives.compression.CompressorUtils.ZLIB_BUFFER_SIZE; +import static com.velocitypowered.natives.compression.CompressorUtils.ensureMaxSize; import com.google.common.base.Preconditions; import io.netty.buffer.ByteBuf; @@ -24,7 +25,7 @@ public class JavaVelocityCompressor implements VelocityCompressor { } @Override - public void inflate(ByteBuf source, ByteBuf destination) throws DataFormatException { + public void inflate(ByteBuf source, ByteBuf destination, int max) throws DataFormatException { ensureNotDisposed(); if (source.hasArray()) { @@ -37,6 +38,7 @@ public class JavaVelocityCompressor implements VelocityCompressor { } while (!inflater.finished()) { + ensureMaxSize(destination, max); int read = inflater.inflate(buf); destination.writeBytes(buf, 0, read); } diff --git a/native/src/main/java/com/velocitypowered/natives/compression/NativeVelocityCompressor.java b/native/src/main/java/com/velocitypowered/natives/compression/NativeVelocityCompressor.java index 8e6f59a1c..54b6daf62 100644 --- a/native/src/main/java/com/velocitypowered/natives/compression/NativeVelocityCompressor.java +++ b/native/src/main/java/com/velocitypowered/natives/compression/NativeVelocityCompressor.java @@ -1,6 +1,7 @@ package com.velocitypowered.natives.compression; -import static com.velocitypowered.natives.util.NativeConstants.ZLIB_BUFFER_SIZE; +import static com.velocitypowered.natives.compression.CompressorUtils.ZLIB_BUFFER_SIZE; +import static com.velocitypowered.natives.compression.CompressorUtils.ensureMaxSize; import com.google.common.base.Preconditions; import io.netty.buffer.ByteBuf; @@ -22,18 +23,19 @@ public class NativeVelocityCompressor implements VelocityCompressor { } @Override - public void inflate(ByteBuf source, ByteBuf destination) throws DataFormatException { + public void inflate(ByteBuf source, ByteBuf destination, int max) throws DataFormatException { ensureNotDisposed(); source.memoryAddress(); destination.memoryAddress(); while (!inflate.finished && source.isReadable()) { if (!destination.isWritable()) { + ensureMaxSize(destination, max); destination.ensureWritable(ZLIB_BUFFER_SIZE); } int produced = inflate.process(inflateCtx, source.memoryAddress() + source.readerIndex(), - source.readableBytes(), - destination.memoryAddress() + destination.writerIndex(), destination.writableBytes()); + source.readableBytes(), destination.memoryAddress() + destination.writerIndex(), + destination.writableBytes()); source.readerIndex(source.readerIndex() + inflate.consumed); destination.writerIndex(destination.writerIndex() + produced); } diff --git a/native/src/main/java/com/velocitypowered/natives/compression/VelocityCompressor.java b/native/src/main/java/com/velocitypowered/natives/compression/VelocityCompressor.java index be9bc6c65..839b64569 100644 --- a/native/src/main/java/com/velocitypowered/natives/compression/VelocityCompressor.java +++ b/native/src/main/java/com/velocitypowered/natives/compression/VelocityCompressor.java @@ -9,7 +9,7 @@ import java.util.zip.DataFormatException; * Provides an interface to inflate and deflate {@link ByteBuf}s using zlib. */ public interface VelocityCompressor extends Disposable, Native { - void inflate(ByteBuf source, ByteBuf destination) throws DataFormatException; + void inflate(ByteBuf source, ByteBuf destination, int max) throws DataFormatException; void deflate(ByteBuf source, ByteBuf destination) throws DataFormatException; } diff --git a/native/src/test/java/com/velocitypowered/natives/compression/VelocityCompressorTest.java b/native/src/test/java/com/velocitypowered/natives/compression/VelocityCompressorTest.java index 0d37f8f67..40f74999e 100644 --- a/native/src/test/java/com/velocitypowered/natives/compression/VelocityCompressorTest.java +++ b/native/src/test/java/com/velocitypowered/natives/compression/VelocityCompressorTest.java @@ -70,7 +70,7 @@ class VelocityCompressorTest { try { compressor.deflate(source, dest); - compressor.inflate(dest, decompressed); + compressor.inflate(dest, decompressed, Integer.MAX_VALUE); source.readerIndex(0); assertTrue(ByteBufUtil.equals(source, decompressed)); } finally { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressDecoder.java index 4a0a51b3f..56af37e65 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressDecoder.java @@ -13,7 +13,7 @@ import java.util.List; public class MinecraftCompressDecoder extends MessageToMessageDecoder { - private static final int MAXIMUM_INITIAL_BUFFER_SIZE = 65536; // 64KiB + private static final int MAXIMUM_UNCOMPRESSED_SIZE = 2 * 1024 * 1024; // 2MiB private final int threshold; private final VelocityCompressor compressor; @@ -35,14 +35,12 @@ public class MinecraftCompressDecoder extends MessageToMessageDecoder { checkFrame(expectedSize >= threshold, "Uncompressed size %s is greater than threshold %s", expectedSize, threshold); + checkFrame(expectedSize <= MAXIMUM_UNCOMPRESSED_SIZE, "Expected uncompressed size" + + "%s is larger than protocol maximum of %s", expectedSize, MAXIMUM_UNCOMPRESSED_SIZE); ByteBuf compatibleIn = ensureCompatible(ctx.alloc(), compressor, in); - int initialCapacity = Math.min(expectedSize, MAXIMUM_INITIAL_BUFFER_SIZE); - ByteBuf uncompressed = preferredBuffer(ctx.alloc(), compressor, initialCapacity); + ByteBuf uncompressed = preferredBuffer(ctx.alloc(), compressor, expectedSize); try { - compressor.inflate(compatibleIn, uncompressed); - checkFrame(expectedSize == uncompressed.readableBytes(), - "Mismatched compression sizes (got %s, expected %s)", - uncompressed.readableBytes(), expectedSize); + compressor.inflate(compatibleIn, uncompressed, expectedSize); out.add(uncompressed); } catch (Exception e) { uncompressed.release();