Mirrored_Repos/DarkflameServer
1
0
Fork 0
mirror of https://github.com/DarkflameUniverse/DarkflameServer.git synced 2026-11-15 08:17:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: check a length read from a message against the data left in it

Browse Source 
Several game messages read a 32 bit length and then read that many
characters without checking it, so the loop ran for the whole length
even after the message had ended. The length now has to fit in the
data the message has left. This covers the activity state, cinematic
path, death type, removed item info and inventory group messages and
the shared wide string reader.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
This commit is contained in:
Aaron Kimbrell
2026-11-10 13:04:12 -06:00
parent 43cea3a245
commit 1ba8f26d2b
3 changed files with 48 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
dCommon/GeneralUtils.cpp
View File

@@ -292,10 +292,12 @@ std::vector<std::string> GeneralUtils::SplitString(const std::string_view str, c
}
std::u16string GeneralUtils::ReadWString(RakNet::BitStream& inStream) {
uint32_t length;
uint32_t length = 0;
inStream.Read<uint32_t>(length);
std::u16string string;
if (!StreamHolds(inStream, length, sizeof(uint16_t))) return string;
for (uint32_t i = 0; i < length; ++i) {
uint16_t c;
inStream.Read(c);

12
dCommon/GeneralUtils.h
View File

@@ -143,6 +143,18 @@ namespace GeneralUtils {
bool ReplaceInString(std::string& str, const std::string_view from, const std::string_view to);
/**
* Whether a stream still holds a number of values of a size. A length read from a stream is checked with this
* before that many values are read, as the stream can not hold more values than it has bytes left.
* @param inStream The stream the values are read from
* @param count The number of values
* @param valueSize The size of one value in bytes
*/
inline bool StreamHolds(const RakNet::BitStream& inStream, const uint64_t count, const uint64_t valueSize = 1) {
// Neither side can wrap: the count is at most 32 bits wide wherever it is read from a stream
return count <= UINT32_MAX && count * valueSize * 8 <= static_cast<uint64_t>(inStream.GetNumberOfUnreadBits());
}
std::u16string ReadWString(RakNet::BitStream& inStream);
std::vector<std::u16string> SplitString(const std::u16string_view str, const char16_t delimiter);

33
tests/dCommonTests/TestGeneralUtils.cpp
View File

@@ -33,3 +33,36 @@ TEST(GeneralUtilsTest, UnpredictableNumberFillsEveryByte) {
EXPECT_EQ(seenBits, 0xFFFFFFFFFFFFFFFFULL);
}
TEST(GeneralUtilsTest, StreamHoldsCountsTheBytesLeft) {
RakNet::BitStream stream;
stream.Write<uint32_t>(4);
for (uint16_t i = 0; i < 4; i++) stream.Write<uint16_t>(u'a' + i);
uint32_t length = 0;
stream.Read(length);
EXPECT_TRUE(GeneralUtils::StreamHolds(stream, 0));
EXPECT_TRUE(GeneralUtils::StreamHolds(stream, 8));
EXPECT_FALSE(GeneralUtils::StreamHolds(stream, 9));
EXPECT_TRUE(GeneralUtils::StreamHolds(stream, 4, sizeof(uint16_t)));
EXPECT_FALSE(GeneralUtils::StreamHolds(stream, 5, sizeof(uint16_t)));
EXPECT_FALSE(GeneralUtils::StreamHolds(stream, UINT32_MAX, sizeof(uint16_t)));
EXPECT_FALSE(GeneralUtils::StreamHolds(stream, UINT64_MAX, sizeof(uint16_t)));
// What a negative 32 bit length becomes
EXPECT_FALSE(GeneralUtils::StreamHolds(stream, static_cast<int32_t>(-1), sizeof(uint16_t)));
}
TEST(GeneralUtilsTest, ReadWStringReadsWhatTheStreamHolds) {
RakNet::BitStream stream;
stream.Write<uint32_t>(4);
for (uint16_t i = 0; i < 4; i++) stream.Write<uint16_t>(u'a' + i);
EXPECT_EQ(GeneralUtils::ReadWString(stream), u"abcd");
RakNet::BitStream tooLong;
tooLong.Write<uint32_t>(0xFFFFFFFF);
tooLong.Write<uint16_t>(u'a');
EXPECT_EQ(GeneralUtils::ReadWString(tooLong), u"");
RakNet::BitStream empty;
EXPECT_EQ(GeneralUtils::ReadWString(empty), u"");
}