mirror of
https://github.com/DarkflameUniverse/DarkflameServer.git
synced 2026-10-02 02:43:44 +00:00
fix: bound AMF3 decoding and stop hashing client keys
Client-sent AMF (CONTROL_BEHAVIORS) was decoded without limits on the associative part of arrays, on nesting depth, or on the total number of values. Associative entries went into an unordered_map with the standard unseeded string hash, so a client could pick colliding keys and make insertion quadratic, and deeply nested arrays recursed until the stack overflowed, crashing the world server. - The associative map is now an ordered std::map (O(log n) whatever the keys, deterministic serialization order). - Each array allows at most 10,000 associative entries, the same as the existing dense limit, which is now checked before anything is read. - Arrays may nest at most 32 deep and one deserializer reads at most 100,000 values. Every limit throws, which the only caller already catches and drops the message. - Inserting a duplicate key keeps the last value and no longer returns a reference to a value that was destroyed when the key already held null. Verified with new unit tests for each limit (including a 100,000 deep nesting that previously overflowed the stack) and the existing live packet test, which still decodes. Fixes #2035 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -11,6 +11,13 @@
|
||||
*/
|
||||
|
||||
std::unique_ptr<AMFBaseValue> AMFDeserialize::Read(RakNet::BitStream& inStream) {
|
||||
// Every value counts against the budget for this deserializer, so one message
|
||||
// cannot make the server build an unbounded tree of values.
|
||||
if (++m_ValuesRead > MaxValues) {
|
||||
LOG("AMF value budget of %u exceeded, possible spoof, aborting deserialize.", MaxValues);
|
||||
throw std::invalid_argument("AMF value budget exceeded");
|
||||
}
|
||||
|
||||
// Read in the value type from the bitStream
|
||||
eAmf marker;
|
||||
inStream.Read(marker);
|
||||
@@ -112,27 +119,41 @@ std::unique_ptr<AMFDoubleValue> AMFDeserialize::ReadAmfDouble(RakNet::BitStream&
|
||||
}
|
||||
|
||||
std::unique_ptr<AMFArrayValue> AMFDeserialize::ReadAmfArray(RakNet::BitStream& inStream) {
|
||||
// Arrays are the only values that nest, so bound the recursion here.
|
||||
if (m_Depth >= MaxDepth) {
|
||||
LOG("AMF arrays nested deeper than %u, possible spoof, aborting deserialize.", MaxDepth);
|
||||
throw std::invalid_argument("AMF arrays nested too deeply");
|
||||
}
|
||||
++m_Depth;
|
||||
|
||||
auto arrayValue = std::make_unique<AMFArrayValue>();
|
||||
|
||||
// Read size of dense array
|
||||
const auto sizeOfDenseArray = (ReadU29(inStream) >> 1);
|
||||
if (sizeOfDenseArray > MaxArraySize) {
|
||||
LOG("Someone sent %u dense array entries, probably a bad packet.", sizeOfDenseArray);
|
||||
throw std::invalid_argument("Too many dense AMF array entries");
|
||||
}
|
||||
|
||||
// Then read associative portion
|
||||
uint32_t associativeEntries = 0;
|
||||
while (true) {
|
||||
const auto key = ReadString(inStream);
|
||||
// No more associative values when we encounter an empty string key
|
||||
if (key.size() == 0) break;
|
||||
if (++associativeEntries > MaxArraySize) {
|
||||
LOG("Someone sent more than %u associative array entries, probably a bad packet.", MaxArraySize);
|
||||
throw std::invalid_argument("Too many associative AMF array entries");
|
||||
}
|
||||
arrayValue->Insert(key, Read(inStream));
|
||||
}
|
||||
|
||||
constexpr int32_t maxArraySize = 10'000;
|
||||
if (sizeOfDenseArray > maxArraySize) {
|
||||
LOG("Someone sent 10,000 dense array entries, probably a bad packet.");
|
||||
throw std::invalid_argument("Someone sent 10,000 dense array entries, probably a bad packet.");
|
||||
}
|
||||
// Finally read dense portion
|
||||
for (uint32_t i = 0; i < sizeOfDenseArray; i++) {
|
||||
arrayValue->Insert(i, Read(inStream));
|
||||
}
|
||||
|
||||
--m_Depth;
|
||||
return arrayValue;
|
||||
}
|
||||
|
||||
|
||||
@@ -9,6 +9,15 @@
|
||||
|
||||
class AMFDeserialize {
|
||||
public:
|
||||
// Most entries one array may hold, in each of its dense and associative parts.
|
||||
static constexpr uint32_t MaxArraySize = 10'000;
|
||||
|
||||
// Deepest nesting of arrays allowed. Client UI messages nest a handful of levels.
|
||||
static constexpr uint32_t MaxDepth = 32;
|
||||
|
||||
// Most values one deserializer will read over its lifetime.
|
||||
static constexpr uint32_t MaxValues = 100'000;
|
||||
|
||||
/**
|
||||
* Read an AMF3 value from a bitstream.
|
||||
*
|
||||
@@ -69,4 +78,10 @@ private:
|
||||
* List of strings read so far saved to be read by reference.
|
||||
*/
|
||||
std::vector<std::string> accessedElements;
|
||||
|
||||
// How deeply nested the array being read is.
|
||||
uint32_t m_Depth = 0;
|
||||
|
||||
// How many values have been read so far.
|
||||
uint32_t m_ValuesRead = 0;
|
||||
};
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
#include "Logger.h"
|
||||
#include "Game.h"
|
||||
#include <type_traits>
|
||||
#include <unordered_map>
|
||||
#include <map>
|
||||
#include <vector>
|
||||
|
||||
enum class eAmf : uint8_t {
|
||||
@@ -115,8 +115,9 @@ using AMFDoubleValue = AMFValue<double>;
|
||||
* and are not to be deleted by a caller.
|
||||
*/
|
||||
class AMFArrayValue : public AMFBaseValue {
|
||||
using AMFAssociative =
|
||||
std::unordered_map<std::string, std::unique_ptr<AMFBaseValue>, GeneralUtils::transparent_string_hash, std::equal_to<>>;
|
||||
// An ordered map keeps lookups and inserts O(log n) no matter which keys a client sends.
|
||||
// A hash map here let a client pick colliding keys and make decoding quadratic (HashDoS).
|
||||
using AMFAssociative = std::map<std::string, std::unique_ptr<AMFBaseValue>, std::less<>>;
|
||||
|
||||
using AMFDense = std::vector<std::unique_ptr<AMFBaseValue>>;
|
||||
|
||||
@@ -222,9 +223,9 @@ public:
|
||||
*/
|
||||
template<typename AmfType>
|
||||
AmfType& Insert(const std::string_view key, std::unique_ptr<AmfType> value) {
|
||||
const auto element = m_Associative.find(key);
|
||||
auto& toReturn = *value;
|
||||
if (element != m_Associative.cend() && element->second) {
|
||||
const auto element = m_Associative.find(key);
|
||||
if (element != m_Associative.cend()) {
|
||||
element->second = std::move(value);
|
||||
} else {
|
||||
m_Associative.emplace(key, std::move(value));
|
||||
|
||||
Reference in New Issue
Block a user