From 674d67f312c815e7f10dc00705e352bc392fc2af Mon Sep 17 00:00:00 2001 From: sfan5 Date: Fri, 29 Jan 2021 15:24:07 +0100 Subject: [PATCH] Encode high codepoints as surrogates to safely transport wchar_t over network fixes #7643 --- src/network/networkpacket.cpp | 51 +++++++++++++++++++++++------ src/network/networkpacket.h | 2 +- src/network/serverpackethandler.cpp | 15 +-------- src/server.cpp | 3 +- src/unittest/test_connection.cpp | 35 ++++++++++++++++++++ 5 files changed, 80 insertions(+), 26 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= 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(src.size()); + for (u16 i = 0; i < src.size(); i++) + *this << static_cast(src[i]); - *this << msgsize; - - // Write string - for (u16 i=0; i(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(0xD800 | (n >> 10)) + << static_cast(0xDC00 | (n & 0x3ff)); + written += 2; + } else { + *this << static_cast(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(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 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