Upstream
/
Waterfall
mirror of https://github.com/PaperMC/Waterfall.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Additional DoS Mitigations for the login sequence (#603) 

(mitigates/Fixes #600 )
This commit is contained in:
FivePB (Xer) 2021-01-31 23:04:10 +01:00 committed by GitHub
parent b3e581412f
commit 2c5c9541c9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 245 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

245
BungeeCord-Patches/0060-Additional-DoS-mitigations.patch Normal file
View File

@ -0,0 +1,245 @@
From 0fd0bf29d77463349c44fc92f9ccdcae63af89db Mon Sep 17 00:00:00 2001
From: "Five (Xer)" <admin@fivepb.me>
Date: Sat, 30 Jan 2021 18:04:14 +0100
Subject: [PATCH] Additional DoS mitigations
Some stricter length checks and cached exceptions to withstand more illegitimate connections
Courtesy of Tux and the Velocity Contributors. See:
https://github.com/VelocityPowered/Velocity/commit/5ceac16a821ea35572ff11412ace8929fd06e278
diff --git a/protocol/src/main/java/net/md_5/bungee/protocol/DefinedPacket.java b/protocol/src/main/java/net/md_5/bungee/protocol/DefinedPacket.java
index d10cf2ed..31a95495 100644
--- a/protocol/src/main/java/net/md_5/bungee/protocol/DefinedPacket.java
+++ b/protocol/src/main/java/net/md_5/bungee/protocol/DefinedPacket.java
@@ -44,6 +44,7 @@ public abstract class DefinedPacket
int len = readVarInt( buf );
if ( len > maxLen * 4 )
{
+ if(!MinecraftDecoder.DEBUG) throw STRING_TOO_MANY_BYTES_EXCEPTION; // Waterfall start: Additional DoS mitigations
throw new OverflowPacketException( String.format( "Cannot receive string longer than %d (got %d bytes)", maxLen * 4, len ) );
}
@@ -53,6 +54,7 @@ public abstract class DefinedPacket
String s = new String( b, Charsets.UTF_8 );
if ( s.length() > maxLen )
{
+ if(!MinecraftDecoder.DEBUG) throw STRING_TOO_LONG_EXCEPTION; // Waterfall start: Additional DoS mitigations
throw new OverflowPacketException( String.format( "Cannot receive string longer than %d (got %d characters)", maxLen, s.length() ) );
}
@@ -284,4 +286,21 @@ public abstract class DefinedPacket
@Override
public abstract String toString();
+
+ // Waterfall start: Additional DoS mitigations, courtesy of Velocity
+ private static final OverflowPacketException STRING_TOO_LONG_EXCEPTION
+ = new OverflowPacketException("A string was longer than allowed. For more "
+ + "information, launch Waterfall with -Dwaterfall.packet-decode-logging=true");
+ private static final OverflowPacketException STRING_TOO_MANY_BYTES_EXCEPTION
+ = new OverflowPacketException("A string had more data than allowed. For more "
+ + "information, launch Waterfall with -Dwaterfall.packet-decode-logging=true");
+
+ public int expectedMaxLength(ByteBuf buf, ProtocolConstants.Direction direction, int protocolVersion) {
+ return -1;
+ }
+
+ public int expectedMinLength(ByteBuf buf, ProtocolConstants.Direction direction, int protocolVersion) {
+ return 0;
+ }
+ // Waterfall end
}
diff --git a/protocol/src/main/java/net/md_5/bungee/protocol/MinecraftDecoder.java b/protocol/src/main/java/net/md_5/bungee/protocol/MinecraftDecoder.java
index 1a647f2b..ac9f114d 100644
--- a/protocol/src/main/java/net/md_5/bungee/protocol/MinecraftDecoder.java
+++ b/protocol/src/main/java/net/md_5/bungee/protocol/MinecraftDecoder.java
@@ -3,7 +3,7 @@ package net.md_5.bungee.protocol;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufUtil;
import io.netty.channel.ChannelHandlerContext;
-import io.netty.handler.codec.DecoderException;
+import io.netty.handler.codec.CorruptedFrameException;
import io.netty.handler.codec.MessageToMessageDecoder;
import java.util.List;
import lombok.AllArgsConstructor;
@@ -56,10 +56,16 @@ public class MinecraftDecoder extends MessageToMessageDecoder<ByteBuf>
if ( packet != null )
{
packetTypeInfo = packet.getClass();
+ doLengthSanityChecks(in, packet, prot.getDirection(), packetId); // Waterfall: Additional DoS mitigations
packet.read( in, prot.getDirection(), protocolVersion );
if ( in.isReadable() )
{
+ // Waterfall start: Additional DoS mitigations
+ if(!DEBUG) {
+ throw PACKET_NOT_READ_TO_END;
+ }
+ // Waterfall end
throw new BadPacketException( "Did not read all bytes from packet " + packet.getClass() + " " + packetId + " Protocol " + protocol + " Direction " + prot.getDirection() );
}
} else
@@ -70,6 +76,11 @@ public class MinecraftDecoder extends MessageToMessageDecoder<ByteBuf>
out.add( new PacketWrapper( packet, slice ) );
slice = null;
} catch (BadPacketException | IndexOutOfBoundsException e) {
+ // Waterfall start: Additional DoS mitigations
+ if(!DEBUG) {
+ throw e;
+ }
+ // Waterfall end
final String packetTypeStr;
if (packetTypeInfo instanceof Integer) {
packetTypeStr = "id " + Integer.toHexString((Integer) packetTypeInfo);
@@ -87,4 +98,52 @@ public class MinecraftDecoder extends MessageToMessageDecoder<ByteBuf>
}
}
}
+
+ // Waterfall start: Additional DoS mitigations, courtesy of Velocity
+ public static final boolean DEBUG = Boolean.getBoolean("waterfall.packet-decode-logging");
+
+ // Cached Exceptions:
+ private static final CorruptedFrameException PACKET_LENGTH_OVERSIZED =
+ new CorruptedFrameException("A packet could not be decoded because it was too large. For more "
+ + "information, launch Waterfall with -Dwaterfall.packet-decode-logging=true");
+ private static final CorruptedFrameException PACKET_LENGTH_UNDERSIZED =
+ new CorruptedFrameException("A packet could not be decoded because it was smaller than allowed. For more "
+ + "information, launch Waterfall with -Dwaterfall.packet-decode-logging=true");
+ private static final BadPacketException PACKET_NOT_READ_TO_END =
+ new BadPacketException("Couldn't read all bytes from a packet. For more "
+ + "information, launch Waterfall with -Dwaterfall.packet-decode-logging=true");
+
+
+ private void doLengthSanityChecks(ByteBuf buf, DefinedPacket packet,
+ ProtocolConstants.Direction direction, int packetId) throws Exception {
+ int expectedMinLen = packet.expectedMinLength(buf, direction, protocolVersion);
+ int expectedMaxLen = packet.expectedMaxLength(buf, direction, protocolVersion);
+ if (expectedMaxLen != -1 && buf.readableBytes() > expectedMaxLen) {
+ throw handleOverflow(packet, expectedMaxLen, buf.readableBytes(), packetId);
+ }
+ if (buf.readableBytes() < expectedMinLen) {
+ throw handleUnderflow(packet, expectedMaxLen, buf.readableBytes(), packetId);
+ }
+ }
+
+ private Exception handleOverflow(DefinedPacket packet, int expected, int actual, int packetId) {
+ if (DEBUG) {
+ throw new CorruptedFrameException( "Packet " + packet.getClass() + " " + packetId
+ + " Protocol " + protocolVersion + " was too big (expected "
+ + expected + " bytes, got " + actual + " bytes)");
+ } else {
+ return PACKET_LENGTH_OVERSIZED;
+ }
+ }
+
+ private Exception handleUnderflow(DefinedPacket packet, int expected, int actual, int packetId) {
+ if (DEBUG) {
+ throw new CorruptedFrameException( "Packet " + packet.getClass() + " " + packetId
+ + " Protocol " + protocolVersion + " was too small (expected "
+ + expected + " bytes, got " + actual + " bytes)");
+ } else {
+ return PACKET_LENGTH_UNDERSIZED;
+ }
+ }
+ // Waterfall end
}
diff --git a/protocol/src/main/java/net/md_5/bungee/protocol/packet/EncryptionResponse.java b/protocol/src/main/java/net/md_5/bungee/protocol/packet/EncryptionResponse.java
index 388f6cdb..53575ce0 100644
--- a/protocol/src/main/java/net/md_5/bungee/protocol/packet/EncryptionResponse.java
+++ b/protocol/src/main/java/net/md_5/bungee/protocol/packet/EncryptionResponse.java
@@ -38,4 +38,16 @@ public class EncryptionResponse extends DefinedPacket
{
handler.handle( this );
}
+
+ // Waterfall start: Additional DoS mitigations, courtesy of Velocity
+ public int expectedMaxLength(ByteBuf buf, ProtocolConstants.Direction direction, int protocolVersion) {
+ // It turns out these come out to the same length, whether we're talking >=1.8 or not.
+ // The length prefix always winds up being 2 bytes.
+ return 260;
+ }
+
+ public int expectedMinLength(ByteBuf buf, ProtocolConstants.Direction direction, int protocolVersion) {
+ return expectedMaxLength(buf, direction, protocolVersion);
+ }
+ // Waterfall end
}
diff --git a/protocol/src/main/java/net/md_5/bungee/protocol/packet/LoginRequest.java b/protocol/src/main/java/net/md_5/bungee/protocol/packet/LoginRequest.java
index a691f962..a4c0a6a5 100644
--- a/protocol/src/main/java/net/md_5/bungee/protocol/packet/LoginRequest.java
+++ b/protocol/src/main/java/net/md_5/bungee/protocol/packet/LoginRequest.java
@@ -11,6 +11,7 @@ import lombok.EqualsAndHashCode;
import lombok.NoArgsConstructor;
import net.md_5.bungee.protocol.AbstractPacketHandler;
import net.md_5.bungee.protocol.DefinedPacket;
+import net.md_5.bungee.protocol.ProtocolConstants;
@Data
@NoArgsConstructor
@@ -38,4 +39,12 @@ public class LoginRequest extends DefinedPacket
{
handler.handle( this );
}
+
+ // Waterfall start: Additional DoS mitigations, courtesy of Velocity
+ public int expectedMaxLength(ByteBuf buf, ProtocolConstants.Direction direction, int protocolVersion) {
+ // Accommodate the rare (but likely malicious) use of UTF-8 usernames, since it is technically
+ // legal on the protocol level.
+ return 1 + (16 * 4);
+ }
+ // Waterfall end
}
diff --git a/protocol/src/main/java/net/md_5/bungee/protocol/packet/PingPacket.java b/protocol/src/main/java/net/md_5/bungee/protocol/packet/PingPacket.java
index 5f24d425..3163a771 100644
--- a/protocol/src/main/java/net/md_5/bungee/protocol/packet/PingPacket.java
+++ b/protocol/src/main/java/net/md_5/bungee/protocol/packet/PingPacket.java
@@ -7,6 +7,7 @@ import lombok.EqualsAndHashCode;
import lombok.NoArgsConstructor;
import net.md_5.bungee.protocol.AbstractPacketHandler;
import net.md_5.bungee.protocol.DefinedPacket;
+import net.md_5.bungee.protocol.ProtocolConstants;
@Data
@NoArgsConstructor
@@ -34,4 +35,14 @@ public class PingPacket extends DefinedPacket
{
handler.handle( this );
}
+
+ // Waterfall start: Additional DoS mitigations, courtesy of Velocity
+ public int expectedMaxLength(ByteBuf buf, ProtocolConstants.Direction direction, int protocolVersion) {
+ return 8;
+ }
+
+ public int expectedMinLength(ByteBuf buf, ProtocolConstants.Direction direction, int protocolVersion) {
+ return 8;
+ }
+ // Waterfall end
}
diff --git a/protocol/src/main/java/net/md_5/bungee/protocol/packet/StatusRequest.java b/protocol/src/main/java/net/md_5/bungee/protocol/packet/StatusRequest.java
index 738f0c92..ec33d337 100644
--- a/protocol/src/main/java/net/md_5/bungee/protocol/packet/StatusRequest.java
+++ b/protocol/src/main/java/net/md_5/bungee/protocol/packet/StatusRequest.java
@@ -6,6 +6,7 @@ import lombok.EqualsAndHashCode;
import lombok.NoArgsConstructor;
import net.md_5.bungee.protocol.AbstractPacketHandler;
import net.md_5.bungee.protocol.DefinedPacket;
+import net.md_5.bungee.protocol.ProtocolConstants;
@Data
@NoArgsConstructor
@@ -28,4 +29,10 @@ public class StatusRequest extends DefinedPacket
{
handler.handle( this );
}
+
+ // Waterfall start: Additional DoS mitigations, courtesy of Velocity
+ public int expectedMaxLength(ByteBuf buf, ProtocolConstants.Direction direction, int protocolVersion) {
+ return 0;
+ }
+ // Waterfall end
}
--
2.30.0