Forcibly disconnect sessions on split packet errors

since split packets are usually reliable, we're breaking the reliability promise if we don't fully process these packets.
worse, if the packet in question is reliable-ordered (always the case with Minecraft), no further packets will be processed anyway, causing the session to break down.

src/generic/DisconnectReason.php

@@ -22,6 +22,10 @@ final class DisconnectReason{
 	public const PEER_TIMEOUT = 2;
 	public const CLIENT_RECONNECT = 3;
 	public const SERVER_SHUTDOWN = 4; //TODO: do we really need a separate reason for this in addition to SERVER_DISCONNECT?
+	public const SPLIT_PACKET_TOO_LARGE = 5;
+	public const SPLIT_PACKET_TOO_MANY_CONCURRENT = 6;
+	public const SPLIT_PACKET_INVALID_PART_INDEX = 7;
+	public const SPLIT_PACKET_INCONSISTENT_HEADER = 8;
 
 	public static function toString(int $reason) : string{
 		return match($reason){
@@ -30,6 +34,10 @@ public static function toString(int $reason) : string{
 			self::PEER_TIMEOUT => "timeout",
 			self::CLIENT_RECONNECT => "new session established on same address and port",
 			self::SERVER_SHUTDOWN => "server shutdown",
+			self::SPLIT_PACKET_TOO_LARGE => "received packet split into more parts than allowed",
+			self::SPLIT_PACKET_TOO_MANY_CONCURRENT => "too many received split packets being reassembled at once",
+			self::SPLIT_PACKET_INVALID_PART_INDEX => "invalid split packet part index",
+			self::SPLIT_PACKET_INCONSISTENT_HEADER => "received split packet header inconsistent with previous fragments",
 			default => "Unknown reason $reason"
 		};
 	}

src/generic/PacketHandlingException.php

@@ -0,0 +1,38 @@
+<?php
+
+/*
+ * This file is part of RakLib.
+ * Copyright (C) 2014-2022 PocketMine Team <https://github.com/pmmp/RakLib>
+ *
+ * RakLib is not affiliated with Jenkins Software LLC nor RakNet.
+ *
+ * RakLib is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ */
+
+declare(strict_types=1);
+
+namespace raklib\generic;
+
+class PacketHandlingException extends \RuntimeException{
+
+	/** @phpstan-var DisconnectReason::* */
+	private int $disconnectReason;
+
+	/**
+	 * @phpstan-param DisconnectReason::* $disconnectReason
+	 */
+	public function __construct(string $message, int $disconnectReason, int $code = 0, ?\Throwable $previous = null){
+		$this->disconnectReason = $disconnectReason;
+		parent::__construct($message, $code, $previous);
+	}
+
+	/**
+	 * @phpstan-return DisconnectReason::*
+	 */
+	public function getDisconnectReason() : int{
+		return $this->disconnectReason;
+	}
+}

src/generic/ReceiveReliabilityLayer.php

@@ -84,33 +84,36 @@ private function handleEncapsulatedPacketRoute(EncapsulatedPacket $pk) : void{
 
 	/**
 	 * Processes a split part of an encapsulated packet.
+	 * If an error occurs (limit exceeded, inconsistent header, etc.) a PacketHandlingException is thrown.
+	 *
+	 * An error processing a split packet means we can't fulfill reliability promises, which at best would lead to some
+	 * packets not arriving, and in the worst case (reliable-ordered) cause no more packets to be processed.
+	 * Therefore, the owning session MUST disconnect the peer if this exception is thrown.
 	 *
 	 * @return null|EncapsulatedPacket Reassembled packet if we have all the parts, null otherwise.
+	 * @throws PacketHandlingException if there was a problem with processing the split packet.
 	 */
 	private function handleSplit(EncapsulatedPacket $packet) : ?EncapsulatedPacket{
 		if($packet->splitInfo === null){
 			return $packet;
 		}
 		$totalParts = $packet->splitInfo->getTotalPartCount();
 		$partIndex = $packet->splitInfo->getPartIndex();
-		if(
-			$totalParts >= $this->maxSplitPacketPartCount or $totalParts < 0 or
-			$partIndex >= $totalParts or $partIndex < 0
-		){
-			$this->logger->debug("Invalid split packet part, too many parts or invalid split index (part index $partIndex, part count $totalParts)");
-			return null;
+		if($totalParts >= $this->maxSplitPacketPartCount || $totalParts < 0){
+			throw new PacketHandlingException("Invalid split packet part count ($totalParts)", DisconnectReason::SPLIT_PACKET_TOO_LARGE);
+		}
+		if($partIndex >= $totalParts || $partIndex < 0){
+			throw new PacketHandlingException("Invalid split packet part index (part index $partIndex, part count $totalParts)", DisconnectReason::SPLIT_PACKET_INVALID_PART_INDEX);
 		}
 
 		$splitId = $packet->splitInfo->getId();
 		if(!isset($this->splitPackets[$splitId])){
 			if(count($this->splitPackets) >= $this->maxConcurrentSplitPackets){
-				$this->logger->debug("Ignored split packet part because reached concurrent split packet limit of $this->maxConcurrentSplitPackets");
-				return null;
+				throw new PacketHandlingException("Exceeded concurrent split packet reassembly limit of $this->maxConcurrentSplitPackets", DisconnectReason::SPLIT_PACKET_TOO_MANY_CONCURRENT);
 			}
 			$this->splitPackets[$splitId] = array_fill(0, $totalParts, null);
 		}elseif(count($this->splitPackets[$splitId]) !== $totalParts){
-			$this->logger->debug("Wrong split count $totalParts for split packet $splitId, expected " . count($this->splitPackets[$splitId]));
-			return null;
+			throw new PacketHandlingException("Wrong split count $totalParts for split packet $splitId, expected " . count($this->splitPackets[$splitId]), DisconnectReason::SPLIT_PACKET_INCONSISTENT_HEADER);
 		}
 
 		$this->splitPackets[$splitId][$partIndex] = $packet;
@@ -142,6 +145,9 @@ private function handleSplit(EncapsulatedPacket $packet) : ?EncapsulatedPacket{
 		return $pk;
 	}
 
+	/**
+	 * @throws PacketHandlingException
+	 */
 	private function handleEncapsulatedPacket(EncapsulatedPacket $packet) : void{
 		if($packet->messageIndex !== null){
 			//check for duplicates or out of range
@@ -209,6 +215,9 @@ private function handleEncapsulatedPacket(EncapsulatedPacket $packet) : void{
 		}
 	}
 
+	/**
+	 * @throws PacketHandlingException
+	 */
 	public function onDatagram(Datagram $packet) : void{
 		if($packet->seqNumber < $this->windowStart or $packet->seqNumber > $this->windowEnd or isset($this->ACKQueue[$packet->seqNumber])){
 			$this->logger->debug("Received duplicate or out-of-window packet (sequence number $packet->seqNumber, window " . $this->windowStart . "-" . $this->windowEnd . ")");

src/generic/Session.php

@@ -155,6 +155,10 @@ protected function getRakNetTimeMS() : int{
 		return intdiv(hrtime(true), 1_000_000);
 	}
 
+	public function getLogger() : \Logger{
+		return $this->logger;
+	}
+
 	public function getAddress() : InternetAddress{
 		return $this->address;
 	}
@@ -275,6 +279,9 @@ private function handlePong(int $sendPingTime, int $sendPongTime) : void{
 		}
 	}
 
+	/**
+	 * @throws PacketHandlingException
+	 */
 	public function handlePacket(Packet $packet) : void{
 		$this->isActive = true;
 		$this->lastUpdate = microtime(true);

src/server/Server.php

@@ -20,6 +20,7 @@
 use raklib\generic\DisconnectReason;
 use raklib\generic\Session;
 use raklib\generic\SocketException;
+use raklib\generic\PacketHandlingException;
 use raklib\protocol\ACK;
 use raklib\protocol\Datagram;
 use raklib\protocol\EncapsulatedPacket;
@@ -247,7 +248,12 @@ private function receivePacket() : bool{
 						$packet = new Datagram();
 					}
 					$packet->decode(new PacketSerializer($buffer));
-					$session->handlePacket($packet);
+					try{
+						$session->handlePacket($packet);
+					}catch(PacketHandlingException $e){
+						$session->getLogger()->error("Error receiving packet: " . $e->getMessage());
+						$session->forciblyDisconnect($e->getDisconnectReason());
+					}
 					return true;
 				}elseif($session->isConnected()){
 					//allows unconnected packets if the session is stuck in DISCONNECTING state, useful if the client