mirror of
https://github.com/PaperMC/Waterfall.git
synced 2025-02-16 03:41:20 +01:00
Additional DoS Mitigations for the login sequence (#603)
(mitigates/Fixes #600 )
This commit is contained in:
FivePB (Xer)
GitHub
parent
b3e581412f
commit
2c5c9541c9
245
BungeeCord-Patches/0060-Additional-DoS-mitigations.patch
Normal file
245
BungeeCord-Patches/0060-Additional-DoS-mitigations.patch
Normal 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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user