Encode high codepoints as surrogates to safely transport wchar_t over network

fixes #7643

5 files changed, 79 insertions, 25 deletions

diff --git a/src/network/networkpacket.cpp b/src/network/networkpacket.cpp
index 6d0abb12c..a71e26572 100644
--- a/src/network/networkpacket.cpp
+++ b/src/network/networkpacket.cpp
@@ -50,7 +50,7 @@ void NetworkPacket::checkReadOffset(u32 from_offset, u32 field_size)
 	}
 }
 
-void NetworkPacket::putRawPacket(u8 *data, u32 datasize, session_t peer_id)
+void NetworkPacket::putRawPacket(const u8 *data, u32 datasize, session_t peer_id)
 {
 	// If a m_command is already set, we are rewriting on same packet
 	// This is not permitted
@@ -145,6 +145,8 @@ void NetworkPacket::putLongString(const std::string &src)
 	putRawString(src.c_str(), msgsize);
 }
 
+static constexpr bool NEED_SURROGATE_CODING = sizeof(wchar_t) > 2;
+
 NetworkPacket& NetworkPacket::operator>>(std::wstring& dst)
 {
 	checkReadOffset(m_read_offset, 2);
@@ -160,9 +162,16 @@ NetworkPacket& NetworkPacket::operator>>(std::wstring& dst)
 	checkReadOffset(m_read_offset, strLen * 2);
 
 	dst.reserve(strLen);
-	for(u16 i=0; i<strLen; i++) {
-		wchar_t c16 = readU16(&m_data[m_read_offset]);
-		dst.append(&c16, 1);
+	for (u16 i = 0; i < strLen; i++) {
+		wchar_t c = readU16(&m_data[m_read_offset]);
+		if (NEED_SURROGATE_CODING && c >= 0xD800 && c < 0xDC00 && i+1 < strLen) {
+			i++;
+			m_read_offset += sizeof(u16);
+
+			wchar_t c2 = readU16(&m_data[m_read_offset]);
+			c = 0x10000 + ( ((c & 0x3ff) << 10) | (c2 & 0x3ff) );
+		}
+		dst.push_back(c);
 		m_read_offset += sizeof(u16);
 	}
 
@@ -175,15 +184,37 @@ NetworkPacket& NetworkPacket::operator<<(const std::wstring &src)
 		throw PacketError("String too long");
 	}
 
-	u16 msgsize = src.size();
+	if (!NEED_SURROGATE_CODING || src.size() == 0) {
+		*this << static_cast<u16>(src.size());
+		for (u16 i = 0; i < src.size(); i++)
+			*this << static_cast<u16>(src[i]);
 
-	*this << msgsize;
+		return *this;
+	}
 
-	// Write string
-	for (u16 i=0; i<msgsize; i++) {
-		*this << (u16) src[i];
+	// write dummy value, to be overwritten later
+	const u32 len_offset = m_read_offset;
+	u32 written = 0;
+	*this << static_cast<u16>(0xfff0);
+
+	for (u16 i = 0; i < src.size(); i++) {
+		wchar_t c = src[i];
+		if (c > 0xffff) {
+			// Encode high code-points as surrogate pairs
+			u32 n = c - 0x10000;
+			*this << static_cast<u16>(0xD800 | (n >> 10))
+				<< static_cast<u16>(0xDC00 | (n & 0x3ff));
+			written += 2;
+		} else {
+			*this << static_cast<u16>(c);
+			written++;
+		}
 	}
 
+	if (written > WIDE_STRING_MAX_LEN)
+		throw PacketError("String too long");
+	writeU16(&m_data[len_offset], written);
+
 	return *this;
 }
 
diff --git a/src/network/networkpacket.h b/src/network/networkpacket.h
index e77bfb744..c7ff03b8e 100644
--- a/src/network/networkpacket.h
+++ b/src/network/networkpacket.h
@@ -34,7 +34,7 @@ public:
 
 	~NetworkPacket();
 
-	void putRawPacket(u8 *data, u32 datasize, session_t peer_id);
+	void putRawPacket(const u8 *data, u32 datasize, session_t peer_id);
 	void clear();
 
 	// Getters
diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp
index 02af06abc..270b8e01f 100644
--- a/src/network/serverpackethandler.cpp
+++ b/src/network/serverpackethandler.cpp
@@ -752,21 +752,8 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt)
 
 void Server::handleCommand_ChatMessage(NetworkPacket* pkt)
 {
-	/*
-		u16 command
-		u16 length
-		wstring message
-	*/
-	u16 len;
-	*pkt >> len;
-
 	std::wstring message;
-	for (u16 i = 0; i < len; i++) {
-		u16 tmp_wchar;
-		*pkt >> tmp_wchar;
-
-		message += (wchar_t)tmp_wchar;
-	}
+	*pkt >> message;
 
 	session_t peer_id = pkt->getPeerId();
 	RemotePlayer *player = m_env->getPlayer(peer_id);
diff --git a/src/server.cpp b/src/server.cpp
index 907bc6d24..b815558fb 100644
--- a/src/server.cpp
+++ b/src/server.cpp
@@ -1482,7 +1482,8 @@ void Server::SendChatMessage(session_t peer_id, const ChatMessage &message)
 	NetworkPacket pkt(TOCLIENT_CHAT_MESSAGE, 0, peer_id);
 	u8 version = 1;
 	u8 type = message.type;
-	pkt << version << type << std::wstring(L"") << message.message << (u64)message.timestamp;
+	pkt << version << type << message.sender << message.message
+		<< static_cast<u64>(message.timestamp);
 
 	if (peer_id != PEER_ID_INEXISTENT) {
 		RemotePlayer *player = m_env->getPlayer(peer_id);
diff --git a/src/unittest/test_connection.cpp b/src/unittest/test_connection.cpp
index c5e4085e1..c3aacc536 100644
--- a/src/unittest/test_connection.cpp
+++ b/src/unittest/test_connection.cpp
@@ -39,6 +39,7 @@ public:
 
 	void runTests(IGameDef *gamedef);
 
+	void testNetworkPacketSerialize();
 	void testHelpers();
 	void testConnectSendReceive();
 };
@@ -47,6 +48,7 @@ static TestConnection g_test_instance;
 
 void TestConnection::runTests(IGameDef *gamedef)
 {
+	TEST(testNetworkPacketSerialize);
 	TEST(testHelpers);
 	TEST(testConnectSendReceive);
 }
@@ -78,6 +80,39 @@ struct Handler : public con::PeerHandler
 	const char *name;
 };
 
+void TestConnection::testNetworkPacketSerialize()
+{
+	const static u8 expected[] = {
+		0x00, 0x7b,
+		0x00, 0x02, 0xd8, 0x42, 0xdf, 0x9a
+	};
+
+	if (sizeof(wchar_t) == 2)
+		warningstream << __func__ << " may fail on this platform." << std::endl;
+
+	{
+		NetworkPacket pkt(123, 0);
+
+		// serializing wide strings should do surrogate encoding, we test that here
+		pkt << std::wstring(L"\U00020b9a");
+
+		SharedBuffer<u8> buf = pkt.oldForgePacket();
+		UASSERTEQ(int, buf.getSize(), sizeof(expected));
+		UASSERT(!memcmp(expected, &buf[0], buf.getSize()));
+	}
+
+	{
+		NetworkPacket pkt;
+		pkt.putRawPacket(expected, sizeof(expected), 0);
+
+		// same for decoding
+		std::wstring pkt_s;
+		pkt >> pkt_s;
+
+		UASSERT(pkt_s == L"\U00020b9a");
+	}
+}
+
 void TestConnection::testHelpers()
 {
 	// Some constants for testing