fix #6395

rts/Game/ChatMessage.cpp

@@ -1,50 +1,60 @@
 /* This file is part of the Spring engine (GPL v2 or later), see LICENSE.html */
 
-
 #include "ChatMessage.h"
 
 #include "Net/Protocol/BaseNetProtocol.h"
 #include "System/Net/PackPacket.h"
 #include "System/Net/UnpackPacket.h"
-#include <cinttypes>
-
-using namespace netcode;
 
 ChatMessage::ChatMessage(int from, int dest, const std::string& chat)
 	: fromPlayer(from)
 	, destination(dest)
-	, msg(chat)
 {
+	// restrict all (player and autohost) chat-messages before they get Pack()'ed
+	msg.resize(std::min(chat.size(), MAX_MSG_SIZE));
+
+	const auto beg = chat.begin();
+	const auto end = chat.begin() + msg.size();
+
+	std::fill(msg.begin(), msg.end(), 0);
+	std::copy(beg, end, msg.begin());
 }
 
 ChatMessage::ChatMessage(std::shared_ptr<const netcode::RawPacket> data)
 {
 	assert(data->data[0] == NETMSG_CHAT);
-	UnpackPacket packet(data, 2);
-	unsigned char from;
-	unsigned char dest;
+	assert(data->length <= (4 * sizeof(uint8_t) + MAX_MESSAGE_SIZE + 1));
+
+	netcode::UnpackPacket packet(data, 2);
+
+	uint8_t from;
+	uint8_t dest;
+
 	packet >> from;
 	packet >> dest;
 	packet >> msg;
+
 	fromPlayer = from;
 	destination = dest;
 }
 
 const netcode::RawPacket* ChatMessage::Pack() const
 {
-	unsigned size = (4 * sizeof(unsigned char)) + (msg.size() + 1);
-	std::uint8_t csize = (size > UINT8_MAX) ? UINT8_MAX : size;
-
-	PackPacket* buffer = new PackPacket(size, NETMSG_CHAT);
-	*buffer << csize;
-	*buffer << (unsigned char)fromPlayer;
-	*buffer << (unsigned char)destination;
-	if (size > UINT16_MAX) {
-		std::string msg_(msg);
-		msg_.resize(size - csize);
-		*buffer << msg_;
-	} else {
-		*buffer << msg;
-	}
+	// message id (uint8), size (uint8), from (uint8), dest (uint8), len(msg)+null
+	// msg.size() itself is clamped to MAX_MSG_SIZE == UINT8_MAX/2 by construction
+	constexpr uint8_t  headerSize = 4 * sizeof(uint8_t);
+	const     uint8_t messageSize = static_cast<uint8_t>(msg.size() + 1);
+	const     uint8_t  packetSize = headerSize + messageSize;
+
+	assert(packetSize <= (headerSize + MAX_MESSAGE_SIZE + 1));
+
+	netcode::PackPacket* buffer = new netcode::PackPacket(packetSize, NETMSG_CHAT);
+
+	*buffer << packetSize;
+	*buffer << static_cast<uint8_t>(fromPlayer);
+	*buffer << static_cast<uint8_t>(destination);
+	*buffer << msg;
+
 	return buffer;
 }
+

rts/Game/ChatMessage.h

@@ -13,19 +13,21 @@ namespace netcode {
 class ChatMessage
 {
 public:
-	ChatMessage(int fromP, int dest, const std::string& chat);
+	ChatMessage(int from, int dest, const std::string& chat);
 	ChatMessage(std::shared_ptr<const netcode::RawPacket> packet);
 
 	const netcode::RawPacket* Pack() const;
 
-	static const int TO_ALLIES = 252;
-	static const int TO_SPECTATORS = 253;
-	static const int TO_EVERYONE = 254;
+	static constexpr size_t MAX_MSG_SIZE = UINT8_MAX / 2;
 
-	int fromPlayer;
+	static constexpr int TO_ALLIES     = 252;
+	static constexpr int TO_SPECTATORS = 253;
+	static constexpr int TO_EVERYONE   = 254;
+
+	int fromPlayer = -1;
+	/// can be TO_ALLIES, TO_SPECTATORS, TO_EVERYONE, or a player number
+	int destination = -1;
 
-	/// the destination can be: TO_ALLIES, TO_SPECTATORS, TO_EVERYONE or a playernumber
-	int destination;
 	std::string msg;
 };
 

rts/Game/Game.cpp

@@ -1636,18 +1636,20 @@ void CGame::SendNetChat(std::string message, int destination)
 		destination = ChatMessage::TO_EVERYONE;
 
 		if ((message.length() >= 2) && (message[1] == ':')) {
-			const char lower = tolower(message[0]);
-			if (lower == 'a') {
-				destination = ChatMessage::TO_ALLIES;
-				message = message.substr(2);
-			} else if (lower == 's') {
-				destination = ChatMessage::TO_SPECTATORS;
-				message = message.substr(2);
+			switch (tolower(message[0])) {
+				case 'a': {
+					destination = ChatMessage::TO_ALLIES;
+					message = message.substr(2);
+				} break;
+				case 's': {
+					destination = ChatMessage::TO_SPECTATORS;
+					message = message.substr(2);
+				} break;
+				default: {
+				} break;
 			}
 		}
 	}
-	if (message.size() > 128)
-		message.resize(128); // safety
 
 	ChatMessage buf(gu->myPlayerNum, destination, message);
 	clientNet->Send(buf.Pack());

rts/System/Net/UnpackPacket.h

@@ -5,6 +5,7 @@
 
 #include "RawPacket.h"
 
+#include <algorithm>
 #include <string>
 #include <vector>
 #include <cstring>
@@ -27,37 +28,39 @@ class UnpackPacket
 	template <typename T>
 	void operator>>(T& t)
 	{
-		if ((pos + sizeof(T)) > pckt->length) {
+		if ((pos + sizeof(T)) > pckt->length)
 			throw UnpackPacketException("Unpack failure (type)");
-		}
-		t = *reinterpret_cast<T*>(pckt->data + pos);
+
+		std::memcpy(reinterpret_cast<void*>(&t), pckt->data + pos, sizeof(T));
 		pos += sizeof(T);
 	}
 
-	template <typename element>
-	void operator>>(std::vector<element>& vec)
+	template <typename T>
+	void operator>>(std::vector<T>& vec)
 	{
-		if ((pckt->length - pos) < (vec.size() * sizeof(element))) {
+		const size_t size = vec.size() * sizeof(T);
+
+		if ((pckt->length - pos) < size)
 			throw UnpackPacketException("Unpack failure (vector)");
-		}
-		const size_t toCopy = vec.size() * sizeof(element);
-		std::memcpy((void*)(&vec[0]), pckt->data + pos, toCopy);
-		pos += toCopy;
+
+		std::memcpy(reinterpret_cast<void*>(&vec[0]), pckt->data + pos, size);
+		pos += size;
 	}
 
 	void operator>>(std::string& text)
 	{
-		int i = pos;
-		for (; i < pckt->length; ++i) {
-			if (pckt->data[i] == '\0') {
-				break;
-			}
-		}
-		if (i >= pckt->length) {
+		const auto beg = pckt->data + pos;
+		const auto end = pckt->data + pckt->length;
+		const auto  it = std::find(beg, end, 0);
+
+		// for strings, require a null-term to have been added during packing
+		if (it == end)
 			throw UnpackPacketException("Unpack failure (string)");
-		}
-		text = std::string((char*)(pckt->data + pos));
-		pos += text.size() + 1;
+
+		text.clear();
+		text.assign(reinterpret_cast<const char*>(beg), it - beg);
+
+		pos += (text.size() + 1);
 	}
 
 private: