From c545cc9d7039a89e23de160b1c93adc959eabda5 Mon Sep 17 00:00:00 2001
From: Rubidium <rubidium@openttd.org>
Date: Sun, 18 Apr 2021 09:01:27 +0200
Subject: [PATCH] Codechange: move more logic about packet size validity and
 reading into Packet

---
 src/network/core/packet.cpp | 23 ++++++++++++++++++++---
 src/network/core/packet.h   |  3 ++-
 src/network/core/tcp.cpp    | 10 ++++------
 src/network/core/udp.cpp    |  4 ++--
 4 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/src/network/core/packet.cpp b/src/network/core/packet.cpp
index 94ffcc5584..4eb0e929ee 100644
--- a/src/network/core/packet.cpp
+++ b/src/network/core/packet.cpp
@@ -180,14 +180,33 @@ bool Packet::CanReadFromPacket(uint bytes_to_read)
 	return true;
 }
 
+/**
+ * Check whether the packet, given the position of the "write" pointer, has read
+ * enough of the packet to contain its size.
+ * @return True iff there is enough data in the packet to contain the packet's size.
+ */
+bool Packet::HasPacketSizeData() const
+{
+	return this->pos >= sizeof(PacketSize);
+}
+
 /**
  * Reads the packet size from the raw packet and stores it in the packet->size
+ * @return True iff the packet size seems plausible.
  */
-void Packet::ReadRawPacketSize()
+bool Packet::ParsePacketSize()
 {
 	assert(this->cs != nullptr && this->next == nullptr);
 	this->size  = (PacketSize)this->buffer[0];
 	this->size += (PacketSize)this->buffer[1] << 8;
+
+	/* If the size of the packet is less than the bytes required for the size and type of
+	 * the packet, or more than the allowed limit, then something is wrong with the packet.
+	 * In those cases the packet can generally be regarded as containing garbage data. */
+	if (this->size < sizeof(PacketSize) + sizeof(PacketType) || this->size > SEND_MTU) return false;
+
+	this->pos = sizeof(PacketSize);
+	return true;
 }
 
 /**
@@ -195,8 +214,6 @@ void Packet::ReadRawPacketSize()
  */
 void Packet::PrepareToRead()
 {
-	this->ReadRawPacketSize();
-
 	/* Put the position on the right place */
 	this->pos = sizeof(PacketSize);
 }
diff --git a/src/network/core/packet.h b/src/network/core/packet.h
index c9be4eeb53..6e5c5509ce 100644
--- a/src/network/core/packet.h
+++ b/src/network/core/packet.h
@@ -71,7 +71,8 @@ public:
 	void Send_string(const char *data);
 
 	/* Reading/receiving of packets */
-	void ReadRawPacketSize();
+	bool HasPacketSizeData() const;
+	bool ParsePacketSize();
 	void PrepareToRead();
 
 	bool   CanReadFromPacket (uint bytes_to_read);
diff --git a/src/network/core/tcp.cpp b/src/network/core/tcp.cpp
index a51913d843..1461a92981 100644
--- a/src/network/core/tcp.cpp
+++ b/src/network/core/tcp.cpp
@@ -155,8 +155,8 @@ Packet *NetworkTCPSocketHandler::ReceivePacket()
 	Packet *p = this->packet_recv;
 
 	/* Read packet size */
-	if (p->pos < sizeof(PacketSize)) {
-		while (p->pos < sizeof(PacketSize)) {
+	if (!p->HasPacketSizeData()) {
+		while (!p->HasPacketSizeData()) {
 		/* Read the size of the packet */
 			res = recv(this->sock, (char*)p->buffer + p->pos, sizeof(PacketSize) - p->pos, 0);
 			if (res == -1) {
@@ -178,10 +178,8 @@ Packet *NetworkTCPSocketHandler::ReceivePacket()
 			p->pos += res;
 		}
 
-		/* Read the packet size from the received packet */
-		p->ReadRawPacketSize();
-
-		if (p->size > SEND_MTU) {
+		/* Parse the size in the received packet and if not valid, close the connection. */
+		if (!p->ParsePacketSize()) {
 			this->CloseConnection();
 			return nullptr;
 		}
diff --git a/src/network/core/udp.cpp b/src/network/core/udp.cpp
index 72fec49e1e..614ae8c3ba 100644
--- a/src/network/core/udp.cpp
+++ b/src/network/core/udp.cpp
@@ -134,14 +134,14 @@ void NetworkUDPSocketHandler::ReceivePackets()
 #endif
 
 			NetworkAddress address(client_addr, client_len);
-			p.PrepareToRead();
 
 			/* If the size does not match the packet must be corrupted.
 			 * Otherwise it will be marked as corrupted later on. */
-			if (nbytes != p.size) {
+			if (!p.ParsePacketSize() || nbytes != p.size) {
 				DEBUG(net, 1, "received a packet with mismatching size from %s", address.GetAddressAsString().c_str());
 				continue;
 			}
+			p.PrepareToRead();
 
 			/* Handle the packet */
 			this->HandleUDPPacket(&p, &address);