Skip to content

Commit 674d67f

Browse files
sfan5sfan5
committedFeb 2, 2021
Encode high codepoints as surrogates to safely transport wchar_t over network
fixes #7643
·
5.11.05.4.0
1 parent c834d2a commit 674d67f

File tree

5 files changed

+79
-25
lines changed

5 files changed

+79
-25
lines changed
 

Diff for: ‎src/network/networkpacket.cpp

+40-9
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ void NetworkPacket::checkReadOffset(u32 from_offset, u32 field_size)
5050
}
5151
}
5252

53-
void NetworkPacket::putRawPacket(u8 *data, u32 datasize, session_t peer_id)
53+
void NetworkPacket::putRawPacket(const u8 *data, u32 datasize, session_t peer_id)
5454
{
5555
// If a m_command is already set, we are rewriting on same packet
5656
// This is not permitted
@@ -145,6 +145,8 @@ void NetworkPacket::putLongString(const std::string &src)
145145
putRawString(src.c_str(), msgsize);
146146
}
147147

148+
static constexpr bool NEED_SURROGATE_CODING = sizeof(wchar_t) > 2;
149+
148150
NetworkPacket& NetworkPacket::operator>>(std::wstring& dst)
149151
{
150152
checkReadOffset(m_read_offset, 2);
@@ -160,9 +162,16 @@ NetworkPacket& NetworkPacket::operator>>(std::wstring& dst)
160162
checkReadOffset(m_read_offset, strLen * 2);
161163

162164
dst.reserve(strLen);
163-
for(u16 i=0; i<strLen; i++) {
164-
wchar_t c16 = readU16(&m_data[m_read_offset]);
165-
dst.append(&c16, 1);
165+
for (u16 i = 0; i < strLen; i++) {
166+
wchar_t c = readU16(&m_data[m_read_offset]);
167+
if (NEED_SURROGATE_CODING && c >= 0xD800 && c < 0xDC00 && i+1 < strLen) {
168+
i++;
169+
m_read_offset += sizeof(u16);
170+
171+
wchar_t c2 = readU16(&m_data[m_read_offset]);
172+
c = 0x10000 + ( ((c & 0x3ff) << 10) | (c2 & 0x3ff) );
173+
}
174+
dst.push_back(c);
166175
m_read_offset += sizeof(u16);
167176
}
168177

@@ -175,15 +184,37 @@ NetworkPacket& NetworkPacket::operator<<(const std::wstring &src)
175184
throw PacketError("String too long");
176185
}
177186

178-
u16 msgsize = src.size();
187+
if (!NEED_SURROGATE_CODING || src.size() == 0) {
188+
*this << static_cast<u16>(src.size());
189+
for (u16 i = 0; i < src.size(); i++)
190+
*this << static_cast<u16>(src[i]);
179191

180-
*this << msgsize;
192+
return *this;
193+
}
181194

182-
// Write string
183-
for (u16 i=0; i<msgsize; i++) {
184-
*this << (u16) src[i];
195+
// write dummy value, to be overwritten later
196+
const u32 len_offset = m_read_offset;
197+
u32 written = 0;
198+
*this << static_cast<u16>(0xfff0);
199+
200+
for (u16 i = 0; i < src.size(); i++) {
201+
wchar_t c = src[i];
202+
if (c > 0xffff) {
203+
// Encode high code-points as surrogate pairs
204+
u32 n = c - 0x10000;
205+
*this << static_cast<u16>(0xD800 | (n >> 10))
206+
<< static_cast<u16>(0xDC00 | (n & 0x3ff));
207+
written += 2;
208+
} else {
209+
*this << static_cast<u16>(c);
210+
written++;
211+
}
185212
}
186213

214+
if (written > WIDE_STRING_MAX_LEN)
215+
throw PacketError("String too long");
216+
writeU16(&m_data[len_offset], written);
217+
187218
return *this;
188219
}
189220

Diff for: ‎src/network/networkpacket.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class NetworkPacket
3434

3535
~NetworkPacket();
3636

37-
void putRawPacket(u8 *data, u32 datasize, session_t peer_id);
37+
void putRawPacket(const u8 *data, u32 datasize, session_t peer_id);
3838
void clear();
3939

4040
// Getters

Diff for: ‎src/network/serverpackethandler.cpp

+1-14
Original file line numberDiff line numberDiff line change
@@ -752,21 +752,8 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt)
752752

753753
void Server::handleCommand_ChatMessage(NetworkPacket* pkt)
754754
{
755-
/*
756-
u16 command
757-
u16 length
758-
wstring message
759-
*/
760-
u16 len;
761-
*pkt >> len;
762-
763755
std::wstring message;
764-
for (u16 i = 0; i < len; i++) {
765-
u16 tmp_wchar;
766-
*pkt >> tmp_wchar;
767-
768-
message += (wchar_t)tmp_wchar;
769-
}
756+
*pkt >> message;
770757

771758
session_t peer_id = pkt->getPeerId();
772759
RemotePlayer *player = m_env->getPlayer(peer_id);

Diff for: ‎src/server.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -1482,7 +1482,8 @@ void Server::SendChatMessage(session_t peer_id, const ChatMessage &message)
14821482
NetworkPacket pkt(TOCLIENT_CHAT_MESSAGE, 0, peer_id);
14831483
u8 version = 1;
14841484
u8 type = message.type;
1485-
pkt << version << type << std::wstring(L"") << message.message << (u64)message.timestamp;
1485+
pkt << version << type << message.sender << message.message
1486+
<< static_cast<u64>(message.timestamp);
14861487

14871488
if (peer_id != PEER_ID_INEXISTENT) {
14881489
RemotePlayer *player = m_env->getPlayer(peer_id);

Diff for: ‎src/unittest/test_connection.cpp

+35
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class TestConnection : public TestBase {
3939

4040
void runTests(IGameDef *gamedef);
4141

42+
void testNetworkPacketSerialize();
4243
void testHelpers();
4344
void testConnectSendReceive();
4445
};
@@ -47,6 +48,7 @@ static TestConnection g_test_instance;
4748

4849
void TestConnection::runTests(IGameDef *gamedef)
4950
{
51+
TEST(testNetworkPacketSerialize);
5052
TEST(testHelpers);
5153
TEST(testConnectSendReceive);
5254
}
@@ -78,6 +80,39 @@ struct Handler : public con::PeerHandler
7880
const char *name;
7981
};
8082

83+
void TestConnection::testNetworkPacketSerialize()
84+
{
85+
const static u8 expected[] = {
86+
0x00, 0x7b,
87+
0x00, 0x02, 0xd8, 0x42, 0xdf, 0x9a
88+
};
89+
90+
if (sizeof(wchar_t) == 2)
91+
warningstream << __func__ << " may fail on this platform." << std::endl;
92+
93+
{
94+
NetworkPacket pkt(123, 0);
95+
96+
// serializing wide strings should do surrogate encoding, we test that here
97+
pkt << std::wstring(L"\U00020b9a");
98+
99+
SharedBuffer<u8> buf = pkt.oldForgePacket();
100+
UASSERTEQ(int, buf.getSize(), sizeof(expected));
101+
UASSERT(!memcmp(expected, &buf[0], buf.getSize()));
102+
}
103+
104+
{
105+
NetworkPacket pkt;
106+
pkt.putRawPacket(expected, sizeof(expected), 0);
107+
108+
// same for decoding
109+
std::wstring pkt_s;
110+
pkt >> pkt_s;
111+
112+
UASSERT(pkt_s == L"\U00020b9a");
113+
}
114+
}
115+
81116
void TestConnection::testHelpers()
82117
{
83118
// Some constants for testing

0 commit comments

Comments
 (0)