diasurgical/DevilutionX
1
0
Fork 0
mirror of https://github.com/diasurgical/DevilutionX.git synced 2026-10-11 22:29:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Do not process packet headers or buffered messages from invalid players

Browse Source
This commit is contained in:
Stephen C. Wills
2026-04-10 22:15:14 +02:00
committed by GitHub
parent 4900391053
commit 2172583c1b
5 changed files with 137 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Source/CMakeLists.txt
+2
View File
@@ -142,6 +142,8 @@ set(libdevilutionx_SRCS
platform/locale.cpp
players/validation.cpp
portals/validation.cpp
qol/autopickup.cpp
Source/msg.cpp
+6
View File
@@ -48,6 +48,7 @@
#include "pack.h"
#include "pfile.h"
#include "player.h"
#include "players/validation.hpp"
#include "plrmsg.h"
#include "portals/validation.hpp"
#include "quests/validation.hpp"
@@ -452,6 +453,11 @@ void PrePacket()
return;
}
if (!IsNetPlayerValid(playerId)) {
Log("Source of network message is no longer valid");
return;
}
const size_t size = ParseCmd(playerId, reinterpret_cast<TCmd *>(data), remainingBytes);
if (size == 0) {
Log("Discarding bad network message");
Source/multi.cpp
+79 -70
View File
@@ -35,6 +35,7 @@
#include "options.h"
#include "pfile.h"
#include "player.h"
#include "players/validation.hpp"
#include "plrmsg.h"
#include "qol/chatlog.h"
#include "storm/storm_net.hpp"
@@ -183,16 +184,6 @@ void NetReceivePlayerData(TPkt *pkt)
pkt->hdr.pdir = static_cast<uint8_t>(myPlayer._pdir);
}
bool IsNetPlayerValid(const Player &player)
{
// we no longer check character level here, players with out-of-range clevels are not allowed to join the game and we don't observe change clevel messages that would set it out of range
// (there's no code path that would result in _pLevel containing an out of range value in the DevilutionX code)
return static_cast<uint8_t>(player._pClass) < GetNumPlayerClasses()
&& player.plrlevel < NUMLEVELS
&& InDungeonBounds(player.position.tile)
&& !std::string_view(player._pName).empty();
}
void CheckPlayerInfoTimeouts()
{
for (uint8_t i = 0; i < Players.size(); i++) {
@@ -366,6 +357,60 @@ void BeginTimeout()
CheckDropPlayer();
}
void SyncPacketHeaderData(Player &player, const TPktHdr &pkt)
{
const Point syncPosition = { pkt.px, pkt.py };
player.position.last = syncPosition;
if (&player != MyPlayer) {
assert(gbBufferMsgs != 2);
player._pHitPoints = Swap32LE(pkt.php);
player._pMaxHP = Swap32LE(pkt.pmhp);
player._pMana = Swap32LE(pkt.mana);
player._pMaxMana = Swap32LE(pkt.maxmana);
const bool cond = gbBufferMsgs == 1;
player._pBaseStr = pkt.bstr;
player._pBaseMag = pkt.bmag;
player._pBaseDex = pkt.bdex;
if (!cond && player.plractive && !player.hasNoLife()) {
if (player.isOnActiveLevel() && !player._pLvlChanging) {
const uint8_t rawDir = pkt.pdir;
if (rawDir <= static_cast<uint8_t>(Direction::SouthEast)) {
const auto newDir = static_cast<Direction>(rawDir);
if (player._pdir != newDir && player._pmode == PM_STAND) {
player._pdir = newDir;
StartStand(player, newDir);
}
}
if (player.position.tile.WalkingDistance(syncPosition) > 3 && PosOkPlayer(player, syncPosition)) {
// got out of sync, clear the tiles around where we last thought the player was located
FixPlrWalkTags(player);
player.position.old = player.position.tile;
// then just in case clear the tiles around the current position (probably unnecessary)
FixPlrWalkTags(player);
player.position.tile = syncPosition;
player.position.future = syncPosition;
if (player.isWalking())
player.position.temp = syncPosition;
SetPlayerOld(player);
player.occupyTile(player.position.tile, false);
}
if (player.position.future.WalkingDistance(player.position.tile) > 1) {
player.position.future = player.position.tile;
}
const Point target = { pkt.targx, pkt.targy };
if (target != Point {}) // does the client send a desired (future) position of remote player?
MakePlrPath(player, target, true);
} else {
player.position.tile = syncPosition;
player.position.future = syncPosition;
SetPlayerOld(player);
}
}
}
}
void HandleAllPackets(uint8_t pnum, const std::byte *data, size_t size)
{
for (size_t offset = 0; offset < size;) {
@@ -692,78 +737,42 @@ void ProcessGameMessagePackets()
uint8_t playerId = std::numeric_limits<uint8_t>::max();
TPktHdr *pkt;
size_t dwMsgSize = 0;
while (SNetReceiveMessage(&playerId, (void **)&pkt, &dwMsgSize)) {
size_t totalPacketSize = 0;
while (SNetReceiveMessage(&playerId, (void **)&pkt, &totalPacketSize)) {
dwRecCount++;
ClearPlayerLeftState();
if (dwMsgSize < sizeof(TPktHdr))
if (totalPacketSize < sizeof(TPktHdr))
continue;
if (playerId >= Players.size())
continue;
if (pkt->wCheck != HeaderCheckVal)
continue;
if (Swap16LE(pkt->wLen) != dwMsgSize)
if (Swap16LE(pkt->wLen) != totalPacketSize)
continue;
// Distrust all messages until player info is received
Player &player = Players[playerId];
if (!IsNetPlayerValid(player)) {
const _cmd_id cmd = *(const _cmd_id *)(pkt + 1);
if (gbBufferMsgs == 0 && IsNoneOf(cmd, CMD_SEND_PLRINFO, CMD_ACK_PLRINFO)) {
// Distrust all messages until
// player info is received
const bool isTrustedPacket = IsNetPlayerValid(player);
if (isTrustedPacket) {
SyncPacketHeaderData(player, *pkt);
}
const bool isBufferingMessages = gbBufferMsgs != 0;
const std::byte *message = (const std::byte *)(pkt + 1);
const size_t messageSize = totalPacketSize - sizeof(TPktHdr);
// It's okay to buffer untrusted messages
// because the player will be validated again later
if (!isTrustedPacket && !isBufferingMessages) {
if (messageSize < 1)
continue;
}
}
const Point syncPosition = { pkt->px, pkt->py };
player.position.last = syncPosition;
if (&player != MyPlayer) {
assert(gbBufferMsgs != 2);
player._pHitPoints = Swap32LE(pkt->php);
player._pMaxHP = Swap32LE(pkt->pmhp);
player._pMana = Swap32LE(pkt->mana);
player._pMaxMana = Swap32LE(pkt->maxmana);
const bool cond = gbBufferMsgs == 1;
player._pBaseStr = pkt->bstr;
player._pBaseMag = pkt->bmag;
player._pBaseDex = pkt->bdex;
if (!cond && player.plractive && !player.hasNoLife()) {
if (player.isOnActiveLevel() && !player._pLvlChanging) {
const uint8_t rawDir = pkt->pdir;
if (rawDir <= static_cast<uint8_t>(Direction::SouthEast)) {
const auto newDir = static_cast<Direction>(rawDir);
if (player._pdir != newDir && player._pmode == PM_STAND) {
player._pdir = newDir;
StartStand(player, newDir);
}
}
if (player.position.tile.WalkingDistance(syncPosition) > 3 && PosOkPlayer(player, syncPosition)) {
// got out of sync, clear the tiles around where we last thought the player was located
FixPlrWalkTags(player);
player.position.old = player.position.tile;
// then just in case clear the tiles around the current position (probably unnecessary)
FixPlrWalkTags(player);
player.position.tile = syncPosition;
player.position.future = syncPosition;
if (player.isWalking())
player.position.temp = syncPosition;
SetPlayerOld(player);
player.occupyTile(player.position.tile, false);
}
if (player.position.future.WalkingDistance(player.position.tile) > 1) {
player.position.future = player.position.tile;
}
const Point target = { pkt->targx, pkt->targy };
if (target != Point {}) // does the client send a desired (future) position of remote player?
MakePlrPath(player, target, true);
} else {
player.position.tile = syncPosition;
player.position.future = syncPosition;
SetPlayerOld(player);
}
}
const _cmd_id cmd = static_cast<_cmd_id>(message[0]);
if (IsNoneOf(cmd, CMD_SEND_PLRINFO, CMD_ACK_PLRINFO))
continue;
}
HandleAllPackets(playerId, (const std::byte *)(pkt + 1), dwMsgSize - sizeof(TPktHdr));
HandleAllPackets(playerId, message, messageSize);
}
CheckPlayerInfoTimeouts();
}
Source/players/validation.cpp
+34
View File
@@ -0,0 +1,34 @@
/**
* @file players/validation.cpp
*
* Implementation of functions for validation of player data.
*/
#include "players/validation.hpp"
#include <string_view>
#include "diablo.h"
#include "levels/gendung.h"
#include "player.h"
#include "tables/playerdat.hpp"
namespace devilution {
bool IsNetPlayerValid(const Player &player)
{
// we no longer check character level here, players with out-of-range clevels are not allowed to join the game and we don't observe change clevel messages that would set it out of range
// (the only code path that would result in _pLevel containing an out of range value in the DevilutionX code is a zero-initialized player, in which case _pName is empty)
return static_cast<uint8_t>(player._pClass) < GetNumPlayerClasses()
&& player.plrlevel < NUMLEVELS
&& InDungeonBounds(player.position.tile)
&& !std::string_view(player._pName).empty();
}
bool IsNetPlayerValid(size_t playerId)
{
const Player &player = Players[playerId];
return IsNetPlayerValid(player);
}
} // namespace devilution
Source/players/validation.hpp
+16
View File
@@ -0,0 +1,16 @@
/**
* @file players/validation.hpp
*
* Interface of functions for validation of player data.
*/
#pragma once
#include <cstddef>
namespace devilution {
struct Player;
bool IsNetPlayerValid(const Player &player);
bool IsNetPlayerValid(size_t playerId);
} // namespace devilution