From 5221db1178dd487fed029d36cedac49bafda37b6 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 22 Jan 2019 17:22:11 +0000 Subject: [PATCH 1/5] Updated BinaryUtils dependency --- composer.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/composer.lock b/composer.lock index 31a9736f5..9f4f4ffd5 100644 --- a/composer.lock +++ b/composer.lock @@ -92,16 +92,16 @@ }, { "name": "pocketmine/binaryutils", - "version": "0.1.7", + "version": "0.1.8", "source": { "type": "git", "url": "https://github.com/pmmp/BinaryUtils.git", - "reference": "3403751da9d39853b43426085cd242173baadd2b" + "reference": "33f511715d22418c03368b49b45a6c25d6b33806" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/pmmp/BinaryUtils/zipball/3403751da9d39853b43426085cd242173baadd2b", - "reference": "3403751da9d39853b43426085cd242173baadd2b", + "url": "https://api.github.com/repos/pmmp/BinaryUtils/zipball/33f511715d22418c03368b49b45a6c25d6b33806", + "reference": "33f511715d22418c03368b49b45a6c25d6b33806", "shasum": "" }, "require": { @@ -119,10 +119,10 @@ ], "description": "Classes and methods for conveniently handling binary data", "support": { - "source": "https://github.com/pmmp/BinaryUtils/tree/0.1.7", + "source": "https://github.com/pmmp/BinaryUtils/tree/0.1.8", "issues": "https://github.com/pmmp/BinaryUtils/issues" }, - "time": "2019-01-07T15:59:50+00:00" + "time": "2019-01-16T17:31:44+00:00" }, { "name": "pocketmine/math", From feaaa925a70b3daa08256792cc1189bc976ff807 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 22 Jan 2019 22:05:15 +0000 Subject: [PATCH 2/5] Fixed a series of denial-of-service bugs in RCON Packets with a too-short payload would either cause the RCON thread to hang until the client disconnected, or crash the RCON thread entirely. commit 90bb1894d7f87645b806f5fc67d1b877bb963180 Author: Dylan K. Taylor Date: Tue Jan 22 18:15:46 2019 +0000 fix some bugs in RCON --- src/pocketmine/network/rcon/RCONInstance.php | 47 ++++++++++++++------ 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/src/pocketmine/network/rcon/RCONInstance.php b/src/pocketmine/network/rcon/RCONInstance.php index ccd167785..ee4471aa9 100644 --- a/src/pocketmine/network/rcon/RCONInstance.php +++ b/src/pocketmine/network/rcon/RCONInstance.php @@ -29,7 +29,6 @@ use pocketmine\utils\Binary; use function count; use function ltrim; use function microtime; -use function rtrim; use function socket_accept; use function socket_close; use function socket_getpeername; @@ -37,11 +36,14 @@ use function socket_last_error; use function socket_read; use function socket_select; use function socket_set_block; +use function socket_set_nonblock; use function socket_set_option; use function socket_shutdown; +use function socket_strerror; use function socket_write; use function str_replace; use function strlen; +use function substr; use function trim; use const PTHREADS_INHERIT_NONE; use const SO_KEEPALIVE; @@ -103,24 +105,41 @@ class RCONInstance extends Thread{ private function readPacket($client, ?int &$requestID, ?int &$packetType, ?string &$payload){ $d = @socket_read($client, 4); - if($this->stop){ - return false; - }elseif($d === false){ - if(socket_last_error($client) === SOCKET_ECONNRESET){ //client crashed, terminate connection - return false; + + socket_getpeername($client, $ip, $port); + if($d === false){ + $err = socket_last_error($client); + if($err !== SOCKET_ECONNRESET){ + $this->logger->debug("Connection error with $ip $port: " . trim(socket_strerror($err))); + } + return false; + } + if(strlen($d) !== 4){ + if($d !== ""){ //empty data is returned on disconnection + $this->logger->debug("Truncated packet from $ip $port (want 4 bytes, have " . strlen($d) . "), disconnecting"); } - return null; - }elseif($d === "" or strlen($d) < 4){ return false; } - $size = Binary::readLInt($d); if($size < 0 or $size > 65535){ + $this->logger->debug("Packet with too-large length header $size from $ip $port, disconnecting"); return false; } - $requestID = Binary::readLInt(socket_read($client, 4)); - $packetType = Binary::readLInt(socket_read($client, 4)); - $payload = rtrim(socket_read($client, $size + 2)); //Strip two null bytes + $buf = @socket_read($client, $size); + if($buf === false){ + $err = socket_last_error($client); + if($err !== SOCKET_ECONNRESET){ + $this->logger->debug("Connection error with $ip $port: " . trim(socket_strerror($err))); + } + return false; + } + if(strlen($buf) !== $size){ + $this->logger->debug("Truncated packet from $ip $port (want $size bytes, have " . strlen($buf) . "), disconnecting"); + return false; + } + $requestID = Binary::readLInt(substr($buf, 0, 4)); + $packetType = Binary::readLInt(substr($buf, 4, 4)); + $payload = substr($buf, 8, -2); //Strip two null bytes return true; } @@ -157,7 +176,7 @@ class RCONInstance extends Thread{ if(count($clients) >= $this->maxClients){ @socket_close($client); }else{ - socket_set_block($client); + socket_set_nonblock($client); socket_set_option($client, SOL_SOCKET, SO_KEEPALIVE, 1); $id = $nextClientId++; @@ -233,11 +252,13 @@ class RCONInstance extends Thread{ } private function disconnectClient($client) : void{ + socket_getpeername($client, $ip, $port); @socket_set_option($client, SOL_SOCKET, SO_LINGER, ["l_onoff" => 1, "l_linger" => 1]); @socket_shutdown($client, 2); @socket_set_block($client); @socket_read($client, 1); @socket_close($client); + $this->logger->info("Disconnected client: /$ip:$port"); } public function getThreadName() : string{ From 45a4252c266817b74906f730a95911bed35b3b9d Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 22 Jan 2019 22:11:32 +0000 Subject: [PATCH 3/5] RCON: Explicitly specify connection backlog size, fixes #2685 I believe this is caused by a bug in the linux kernel, since it only impacts certain machines I tested (one, to be specific). Whatever the case, setting a max backlog size is prudent anyway, and fixes the problem. --- src/pocketmine/network/rcon/RCON.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pocketmine/network/rcon/RCON.php b/src/pocketmine/network/rcon/RCON.php index b1df366c5..01e14043e 100644 --- a/src/pocketmine/network/rcon/RCON.php +++ b/src/pocketmine/network/rcon/RCON.php @@ -74,7 +74,7 @@ class RCON{ $this->socket = socket_create(AF_INET, SOCK_STREAM, SOL_TCP); - if($this->socket === false or !@socket_bind($this->socket, $interface, $port) or !@socket_listen($this->socket)){ + if($this->socket === false or !@socket_bind($this->socket, $interface, $port) or !@socket_listen($this->socket, 5)){ throw new \RuntimeException(trim(socket_strerror(socket_last_error()))); } From 4b8e4123afb7ab0b0a9798c23a45e18d2a51c1c0 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 22 Jan 2019 22:13:35 +0000 Subject: [PATCH 4/5] Release 3.5.7 --- src/pocketmine/PocketMine.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pocketmine/PocketMine.php b/src/pocketmine/PocketMine.php index 3a06df7a9..96f924566 100644 --- a/src/pocketmine/PocketMine.php +++ b/src/pocketmine/PocketMine.php @@ -38,7 +38,7 @@ namespace pocketmine { const NAME = "PocketMine-MP"; const BASE_VERSION = "3.5.7"; - const IS_DEVELOPMENT_BUILD = true; + const IS_DEVELOPMENT_BUILD = false; const BUILD_NUMBER = 0; const MIN_PHP_VERSION = "7.2.0"; From 5a8812b1dc12027f8c99e843f87bdad66af0a242 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 22 Jan 2019 22:14:19 +0000 Subject: [PATCH 5/5] 3.5.8 is next --- src/pocketmine/PocketMine.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pocketmine/PocketMine.php b/src/pocketmine/PocketMine.php index 96f924566..76c263bc9 100644 --- a/src/pocketmine/PocketMine.php +++ b/src/pocketmine/PocketMine.php @@ -37,8 +37,8 @@ namespace pocketmine { use pocketmine\wizard\SetupWizard; const NAME = "PocketMine-MP"; - const BASE_VERSION = "3.5.7"; - const IS_DEVELOPMENT_BUILD = false; + const BASE_VERSION = "3.5.8"; + const IS_DEVELOPMENT_BUILD = true; const BUILD_NUMBER = 0; const MIN_PHP_VERSION = "7.2.0";