Skip to content

Commit

Permalink
Reimplemented some of the NRSSR checks with changes so they handle al…
Browse files Browse the repository at this point in the history
…l push message types. Also modified the ds2 min version supported so we don't allow running the old version which doesn't have any RCE mitigations for it.
  • Loading branch information
TLeonardUK committed Mar 23, 2024
1 parent 855770a commit f2657af
Show file tree
Hide file tree
Showing 18 changed files with 473 additions and 25 deletions.
1 change: 1 addition & 0 deletions Source/Server.DarkSouls2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ SET(SOURCES

Server/GameService/Utils/DS2_GameIds.h
Server/GameService/Utils/DS2_CellAndAreaId.h
Server/GameService/Utils/DS2_NRSSRSanitizer.h

Server/GameService/Utils/Ids/BonfireId.inc
Server/GameService/Utils/Ids/CharacterTypeId.inc
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "Server/Streams/Frpg2ReliableUdpMessage.h"
#include "Server/Streams/Frpg2ReliableUdpMessageStream.h"
#include "Server/Streams/DS2_Frpg2ReliableUdpMessage.h"
#include "Server/GameService/Utils/DS2_NRSSRSanitizer.h"
#include "Protobuf/DS2_Protobufs.h"

#include "Config/RuntimeConfig.h"
Expand Down Expand Up @@ -192,6 +193,20 @@ MessageHandleResult DS2_BloodMessageManager::Handle_RequestCreateBloodMessage(Ga
std::vector<uint8_t> MessageData;
MessageData.assign(Request->message_data().data(), Request->message_data().data() + Request->message_data().size());

// There is no NRSSR struct in blood messsage data, but we still make sure the size-delimited entry list is valid.
if (BuildConfig::NRSSR_SANITY_CHECKS)
{
auto ValidationResult = DS2_NRSSRSanitizer::ValidateEntryList(MessageData.data(), MessageData.size());
if (ValidationResult != DS2_NRSSRSanitizer::ValidationResult::Valid)
{
WarningS(Client->GetName().c_str(), "Blood message data recieved from client is invalid (error code %i).",
static_cast<uint32_t>(ValidationResult));

// Simply ignore the request. Perhaps sending a response with an invalid sign id or disconnecting the client would be better?
return MessageHandleResult::Handled;
}
}

if (std::shared_ptr<BloodMessage> ActiveMessage = Database.CreateBloodMessage((uint32_t)Request->online_area_id(), (uint64_t)Request->cell_id(), Player.GetPlayerId(), Player.GetSteamId(), Request->character_id(), MessageData))
{
Response.set_message_id(ActiveMessage->MessageId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "Server/Server.h"

#include "Config/BuildConfig.h"
#include "Server/GameService/Utils/DS2_NRSSRSanitizer.h"

#include "Shared/Core/Utils/Logging.h"
#include "Shared/Core/Utils/Strings.h"
Expand Down Expand Up @@ -100,6 +101,27 @@ MessageHandleResult DS2_BloodstainManager::Handle_RequestCreateBloodstain(GameCl
Data.assign(Request->data().data(), Request->data().data() + Request->data().size());
GhostData.assign(Request->ghost_data().data(), Request->ghost_data().data() + Request->ghost_data().size());

// There is no NRSSR struct in bloodstain or ghost data, but we still make sure the size-delimited entry list is valid.
if (BuildConfig::NRSSR_SANITY_CHECKS)
{
auto ValidationResult = DS2_NRSSRSanitizer::ValidateEntryList(Data.data(), Data.size());
if (ValidationResult != DS2_NRSSRSanitizer::ValidationResult::Valid)
{
WarningS(Client->GetName().c_str(), "Bloodstain metadata recieved from client is invalid (error code %i).",
static_cast<uint32_t>(ValidationResult));

return MessageHandleResult::Handled;
}
ValidationResult = DS2_NRSSRSanitizer::ValidateEntryList(GhostData.data(), GhostData.size());
if (ValidationResult != DS2_NRSSRSanitizer::ValidationResult::Valid)
{
WarningS(Client->GetName().c_str(), "Ghost data recieved from client is invalid (error code %i).",
static_cast<uint32_t>(ValidationResult));

return MessageHandleResult::Handled;
}
}

std::shared_ptr<Bloodstain> ActiveStain = nullptr;
if (Config.BloodstainMemoryCacheOnly)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "Server/Server.h"

#include "Config/BuildConfig.h"
#include "Server/GameService/Utils/DS2_NRSSRSanitizer.h"

#include "Shared/Core/Utils/Logging.h"
#include "Shared/Core/Utils/Strings.h"
Expand Down Expand Up @@ -90,6 +91,19 @@ MessageHandleResult DS2_GhostManager::Handle_RequestCreateGhostData(GameClient*
std::vector<uint8_t> Data;
Data.assign(Request->data().data(), Request->data().data() + Request->data().size());

// There is no NRSSR struct in ghost data, but we still make sure the size-delimited entry list is valid.
if (BuildConfig::NRSSR_SANITY_CHECKS)
{
auto ValidationResult = DS2_NRSSRSanitizer::ValidateEntryList(Data.data(), Data.size());
if (ValidationResult != DS2_NRSSRSanitizer::ValidationResult::Valid)
{
WarningS(Client->GetName().c_str(), "Ghost data recieved from client is invalid (error code %i).",
static_cast<uint32_t>(ValidationResult));

return MessageHandleResult::Handled;
}
}

std::shared_ptr<Ghost> ActiveGhost = nullptr;
if (Config.GhostMemoryCacheOnly)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "Server/Server.h"

#include "Config/BuildConfig.h"
#include "Server/GameService/Utils/DS2_NRSSRSanitizer.h"

#include "Shared/Core/Utils/Logging.h"
#include "Shared/Core/Utils/File.h"
Expand Down Expand Up @@ -168,6 +169,20 @@ MessageHandleResult DS2_MirrorKnightManager::Handle_RequestCreateMirrorKnightSig

DS2_Frpg2RequestMessage::RequestCreateMirrorKnightSign* Request = (DS2_Frpg2RequestMessage::RequestCreateMirrorKnightSign*)Message.Protobuf.get();

// There is no NRSSR struct in the sign metadata, but we still make sure the size-delimited entry list is valid.
if (BuildConfig::NRSSR_SANITY_CHECKS)
{
auto ValidationResult = DS2_NRSSRSanitizer::ValidateEntryList(Request->data().data(), Request->data().size());
if (ValidationResult != DS2_NRSSRSanitizer::ValidationResult::Valid)
{
WarningS(Client->GetName().c_str(), "RequestCreateMirrorKnightSign message recieved from client contains ill formated binary data (error code %i).",
static_cast<uint32_t>(ValidationResult));

// Simply ignore the request. Perhaps sending a response with an invalid sign id or disconnecting the client would be better?
return MessageHandleResult::Handled;
}
}

std::shared_ptr<SummonSign> Sign = std::make_shared<SummonSign>();
Sign->SignId = NextSignId++;
Sign->PlayerId = Player.GetPlayerId();
Expand Down Expand Up @@ -250,6 +265,20 @@ MessageHandleResult DS2_MirrorKnightManager::Handle_RequestSummonMirrorKnightSig
DS2_Frpg2RequestMessage::RequestSummonMirrorKnightSign* Request = (DS2_Frpg2RequestMessage::RequestSummonMirrorKnightSign*)Message.Protobuf.get();

bool bSuccess = true;

// Make sure the NRSSR data contained within this message is valid (if the CVE-2022-24126 fix is enabled)
if (BuildConfig::NRSSR_SANITY_CHECKS)
{
auto ValidationResult = DS2_NRSSRSanitizer::ValidateEntryList(Request->player_struct().data(), Request->player_struct().size());
if (ValidationResult != DS2_NRSSRSanitizer::ValidationResult::Valid)
{
WarningS(Client->GetName().c_str(), "RequestSummonSign message recieved from client contains ill formated binary data (error code %i).",
static_cast<uint32_t>(ValidationResult));

bSuccess = false;
}
}

DS2_Frpg2RequestMessage::SummonErrorId SummonError = DS2_Frpg2RequestMessage::SummonErrorId_NoLongerBeSummonable;

// First check the sign still exists, if it doesn't, send a reject message as its probably already used.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "Server/Streams/DS2_Frpg2ReliableUdpMessage.h"

#include "Server/GameService/Utils/DS2_GameIds.h"
#include "Server/GameService/Utils/DS2_NRSSRSanitizer.h"

#include "Config/RuntimeConfig.h"
#include "Server/Server.h"
Expand Down Expand Up @@ -64,30 +65,32 @@ MessageHandleResult DS2_MiscManager::Handle_RequestSendMessageToPlayers(GameClie
// So this check is being disabled for now, should be revisted in future.
bool ShouldProcessRequest = true;

#if 0
// RequestSendMessageToPlayers sanity checks (patch CVE-2022-24125)
// The game seems to only use this function in the BreakIn (invasions) implementation.
// Hence we should make sure that a malicious client does not use this send arbitrary data to other players.
if constexpr (BuildConfig::SEND_MESSAGE_TO_PLAYERS_SANITY_CHECKS)
{
// The request should contain a PushRequestBreakInTarget message
DS2_Frpg2RequestMessage::PushRequestAllowBreakInTarget EmbdeddedMsg;

// The request should specify exactly one player
if (Request->player_ids_size() > 1)
// Limit player ids this can be sent to - for arenas we can send this to more than one player.
if (Request->player_ids_size() > 6)
{
WarningS(Client->GetName().c_str(), "Client attempted to send message to more than one (%i) client. This is not normal behaviour.", Request->player_ids_size());
WarningS(Client->GetName().c_str(), "Client attempted to send message to too many (%i) clients. This is not normal behaviour.", Request->player_ids_size());
ShouldProcessRequest = false;
}
// The request should only ever contain a PushRequestAllowBreakInTarget message
// TODO: Removed the PushID check, ds2 (and I actually think ds3) uses SendMessageToPlayers for more than just
if (!EmbdeddedMsg.ParseFromString(Request->message()))

// Validate PushRequestAllowBreakInTarget. The game also uses this function for other message types (during areans for example), we should
// track down and add validation to those messages as well.
if (BuildConfig::NRSSR_SANITY_CHECKS)
{
WarningS(Client->GetName().c_str(), "RequestSendMessageToPlayers embedded message provided by client is not a valid PushRequestAllowBreakInTarget message.");
ShouldProcessRequest = false;
auto ValidationResult = DS2_NRSSRSanitizer::ValidatePushMessages(Request->message());
if (ValidationResult != DS2_NRSSRSanitizer::ValidationResult::Valid)
{
WarningS(Client->GetName().c_str(), "PushRequestAllowBreakInTarget message recieved from client contains ill formated binary data (error code %i).",
static_cast<uint32_t>(ValidationResult));

ShouldProcessRequest = false;
}
}
}
#endif

if (ShouldProcessRequest)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "Server/Server.h"

#include "Config/BuildConfig.h"
#include "Server/GameService/Utils/DS2_NRSSRSanitizer.h"

#include "Shared/Core/Utils/Logging.h"
#include "Shared/Core/Utils/File.h"
Expand Down Expand Up @@ -229,6 +230,20 @@ MessageHandleResult DS2_SignManager::Handle_RequestCreateSign(GameClient* Client

DS2_Frpg2RequestMessage::RequestCreateSign* Request = (DS2_Frpg2RequestMessage::RequestCreateSign*)Message.Protobuf.get();

// There is no NRSSR struct in the sign metadata, but we still make sure the size-delimited entry list is valid.
if (BuildConfig::NRSSR_SANITY_CHECKS)
{
auto ValidationResult = DS2_NRSSRSanitizer::ValidateEntryList(Request->player_struct().data(), Request->player_struct().size());
if (ValidationResult != DS2_NRSSRSanitizer::ValidationResult::Valid)
{
WarningS(Client->GetName().c_str(), "RequestCreateSign message recieved from client contains ill formated binary data (error code %i).",
static_cast<uint32_t>(ValidationResult));

// Simply ignore the request. Perhaps sending a response with an invalid sign id or disconnecting the client would be better?
return MessageHandleResult::Handled;
}
}

std::shared_ptr<SummonSign> Sign = std::make_shared<SummonSign>();
Sign->SignId = NextSignId++;
Sign->OnlineAreaId = (uint32_t)Request->online_area_id();
Expand Down Expand Up @@ -348,6 +363,20 @@ MessageHandleResult DS2_SignManager::Handle_RequestSummonSign(GameClient* Client
DS2_Frpg2RequestMessage::RequestSummonSign* Request = (DS2_Frpg2RequestMessage::RequestSummonSign*)Message.Protobuf.get();

bool bSuccess = true;

// Make sure the NRSSR data contained within this message is valid (if the CVE-2022-24126 fix is enabled)
if (BuildConfig::NRSSR_SANITY_CHECKS)
{
auto ValidationResult = DS2_NRSSRSanitizer::ValidateEntryList(Request->player_struct().data(), Request->player_struct().size());
if (ValidationResult != DS2_NRSSRSanitizer::ValidationResult::Valid)
{
WarningS(Client->GetName().c_str(), "RequestSummonSign message recieved from client contains ill formated binary data (error code %i).",
static_cast<uint32_t>(ValidationResult));

bSuccess = false;
}
}

DS2_Frpg2RequestMessage::SummonErrorId SummonError = DS2_Frpg2RequestMessage::SummonErrorId_NoLongerBeSummonable;

DS2_CellAndAreaId LocationId = { (uint64_t)Request->cell_id(), (DS2_OnlineAreaId)Request->online_area_id() };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "Server/Server.h"

#include "Config/BuildConfig.h"
#include "Server/GameService/Utils/DS2_NRSSRSanitizer.h"

#include "Shared/Core/Utils/Logging.h"
#include "Shared/Core/Utils/Strings.h"
Expand Down Expand Up @@ -134,6 +135,19 @@ MessageHandleResult DS2_VisitorManager::Handle_RequestVisit(GameClient* Client,
DS2_Frpg2RequestMessage::RequestVisit* Request = (DS2_Frpg2RequestMessage::RequestVisit*)Message.Protobuf.get();

bool bSuccess = true;

// Make sure the NRSSR data contained within this message is valid (if the CVE-2022-24126 fix is enabled)
if (BuildConfig::NRSSR_SANITY_CHECKS)
{
auto ValidationResult = DS2_NRSSRSanitizer::ValidateEntryList(Request->player_struct().data(), Request->player_struct().size());
if (ValidationResult != DS2_NRSSRSanitizer::ValidationResult::Valid)
{
WarningS(Client->GetName().c_str(), "RequestVisit message recieved from client contains ill formated binary data (error code %i).",
static_cast<uint32_t>(ValidationResult));

bSuccess = false;
}
}

// Check client still exists.
std::shared_ptr<GameClient> TargetClient = GameServiceInstance->FindClientByPlayerId(Request->player_id());
Expand Down
112 changes: 112 additions & 0 deletions Source/Server.DarkSouls2/Server/GameService/Utils/DS2_NRSSRSanitizer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/*
* NRSSRSanitizer.h
* Copyright (C) 2022 William Tremblay
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in the
* Software without restriction, including without limitation the rights to use, copy,
* modify, merge, publish, distribute, sublicense, and/or sell copies of the Software,
* and to permit persons to whom the Software is furnished to do so, subject to the
* following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
* INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
* PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
* FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
* OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
*/

#pragma once

#include <inttypes.h>

#ifdef _WIN32
#include <corecrt_wstring.h>
#else
#include <wchar.h>
#include <byteswap.h>
#endif

// Static class that holds methods intended to validate the structure
// of the size-delimited entry lists and serialized NRSessionSearchResult
// data for security purposes, namely patching out-of-bounds read crashes
// and a reliable remote code execution exploit (CVE-2022-24126).
//
// This specifically relates to the game's V1.15 version. It will most
// likely have to be reworked if a new client version comes out, but this
// should not be necessary as the only reason for a client update would be
// patching this bug.
class DS2_NRSSRSanitizer
{
public:

DS2_NRSSRSanitizer() = delete;

// The game checks if these two match and rejects the data if they dont,
// so we can use them to detect NRSSR data.
inline static const uint32_t SIGNATURE = 0x5652584E;
inline static const uint16_t VERSION_NUMBER = 0x8405;

// The size of the session data information in bytes, stored in big-endian in the packet.
// This is always 8 on PC and holds the ID of the Steam lobby the client should connect to.
inline static const uint16_t SESSION_DATA_SIZE = 8;

// The size of the host online id field in NRSSR data. On PC this is 8 (size of a CSteamID).
inline static const size_t HOST_ONLINE_ID_SIZE = 8;

// Game stack buffer sizes for property and name strings (in number of uint16_t (note:not wchar_t!!! wchar_t size is compiler dependent))
inline static const size_t MAX_PROP_WSTR_SIZE = 1024;
inline static const size_t MAX_NAME_WSTR_SIZE = 256;

// Possible outcomes of size-delimited entry list validation
enum class ValidationResult
{
// The entry list data is valid.
Valid,
// One of the entry size fields is invalid.
EntryList_SizeMismatch,
// The signature or version of NRSSR data does not match expected values.
NRSSR_SignatureOrVersion_Mismatch,
// There is not enough data left in the buffer to read NRSSR property metadata.
NRSSR_PropertyMetadata_InsufficientData,
// The provided NRSSR property type is invalid.
NRSSR_PropertyMetadata_InvalidType,
// There is not enough data left in the buffer to read a 4-byte NRSSR property.
NRSSR_Property4Byte_InsufficientData,
// There is not enough data left in the buffer to read an 8-byte NRSRR property.
NRSSR_Property8Byte_InsufficientData,
// A string NRSSR property is either longer than the client's buffer or is not null-terminated.
NRSSR_PropertyString_Overflow,
// The NRSSR host name string is either longer than the client's buffer or is not null-terminated.
NRSSR_NameString_Overflow,
// The amount of data left in the NRSSR struct after the name string is abnormal.
NRSSR_RemainingDataSize_Mismatch,
// NRSSR session size field does not match with the expected value.
NRSSR_SessionSize_Abnormal
};

// Verify if length-delimited entry list data used to store session join information is valid.
// Will also validate any NRSSR data stored in the entries.
static ValidationResult ValidateEntryList(const uint8_t* EntryList, size_t Size)
{
// Format is different in DS2.
// If we want to enable support for older versions, we should decipher it.
return ValidationResult::Valid;
}

static ValidationResult ValidateEntryList(const char* EntryList, size_t Size)
{
return ValidateEntryList(reinterpret_cast<const uint8_t*>(EntryList), Size);
}

static ValidationResult ValidatePushMessages(const std::string& Data)
{
// Format is different in DS2.
// If we want to enable support for older versions, we should decipher it.
return ValidationResult::Valid;
}
};
Loading

0 comments on commit f2657af

Please sign in to comment.