Cleanup behavior bitstream reads (#888)

* Add failArmor server side

Address out of bounds reading in behavior

Address the basicAttackBehavior reading out of bounds memory and reading bits that didnt exist, which occasionally caused crashes and also caused the behavior to do undefined behavior due to the bad reads.

Tested that attacking a wall anywhere with a projectile now does not crash the game.  Tested with logs that the behavior correctly returned when there were no allocated bits or returned when other states were met.

Add back logs and add fail handle

Remove comment block

Revert "Add back logs and add fail handle"

This reverts commit db19be0906fc8bf35bf89037e2bfba39f5ef9c0c.

Split out checks

* Cleanup Behavior streams
This commit is contained in:
David Markowitz 2022-12-16 13:23:02 -08:00 committed by GitHub
parent 1ed3af63b9
commit a2ca273370
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 138 additions and 56 deletions

View File

@ -2,11 +2,16 @@
#include "BehaviorBranchContext.h"
#include "BehaviorContext.h"
#include "EntityManager.h"
#include "Game.h"
#include "dLogger.h"
void AirMovementBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) {
uint32_t handle;
uint32_t handle{};
bitStream->Read(handle);
if (!bitStream->Read(handle)) {
Game::logger->Log("AirMovementBehavior", "Unable to read handle from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
}
context->RegisterSyncBehavior(handle, this, branch);
}
@ -17,14 +22,20 @@ void AirMovementBehavior::Calculate(BehaviorContext* context, RakNet::BitStream*
bitStream->Write(handle);
}
void AirMovementBehavior::Sync(BehaviorContext* context, RakNet::BitStream* bit_stream, BehaviorBranchContext branch) {
uint32_t behaviorId;
void AirMovementBehavior::Sync(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) {
uint32_t behaviorId{};
bit_stream->Read(behaviorId);
if (!bitStream->Read(behaviorId)) {
Game::logger->Log("AirMovementBehavior", "Unable to read behaviorId from bitStream, aborting Sync! %i", bitStream->GetNumberOfUnreadBits());
return;
}
LWOOBJID target;
LWOOBJID target{};
bit_stream->Read(target);
if (!bitStream->Read(target)) {
Game::logger->Log("AirMovementBehavior", "Unable to read target from bitStream, aborting Sync! %i", bitStream->GetNumberOfUnreadBits());
return;
}
auto* behavior = CreateBehavior(behaviorId);
@ -32,7 +43,7 @@ void AirMovementBehavior::Sync(BehaviorContext* context, RakNet::BitStream* bit_
branch.target = target;
}
behavior->Handle(context, bit_stream, branch);
behavior->Handle(context, bitStream, branch);
}
void AirMovementBehavior::Load() {

View File

@ -16,7 +16,7 @@ public:
void Calculate(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) override;
void Sync(BehaviorContext* context, RakNet::BitStream* bit_stream, BehaviorBranchContext branch) override;
void Sync(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) override;
void Load() override;
};

View File

@ -9,11 +9,16 @@
#include "BehaviorContext.h"
#include "RebuildComponent.h"
#include "DestroyableComponent.h"
#include "Game.h"
#include "dLogger.h"
void AreaOfEffectBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) {
uint32_t targetCount;
uint32_t targetCount{};
bitStream->Read(targetCount);
if (!bitStream->Read(targetCount)) {
Game::logger->Log("AreaOfEffectBehavior", "Unable to read targetCount from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
}
if (targetCount > this->m_maxTargets) {
return;
@ -24,9 +29,12 @@ void AreaOfEffectBehavior::Handle(BehaviorContext* context, RakNet::BitStream* b
targets.reserve(targetCount);
for (auto i = 0u; i < targetCount; ++i) {
LWOOBJID target;
LWOOBJID target{};
bitStream->Read(target);
if (!bitStream->Read(target)) {
Game::logger->Log("AreaOfEffectBehavior", "failed to read in target %i from bitStream, aborting target Handle!", i);
return;
};
targets.push_back(target);
}

View File

@ -5,9 +5,12 @@
#include "dLogger.h"
void AttackDelayBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStream, const BehaviorBranchContext branch) {
uint32_t handle;
uint32_t handle{};
bitStream->Read(handle);
if (!bitStream->Read(handle)) {
Game::logger->Log("AttackDelayBehavior", "Unable to read handle from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
};
for (auto i = 0u; i < this->m_numIntervals; ++i) {
context->RegisterSyncBehavior(handle, this, branch);

View File

@ -4,14 +4,19 @@
#include "dLogger.h"
void ChainBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStream, const BehaviorBranchContext branch) {
uint32_t chain_index;
uint32_t chainIndex{};
bitStream->Read(chain_index);
if (!bitStream->Read(chainIndex)) {
Game::logger->Log("ChainBehavior", "Unable to read chainIndex from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
}
chain_index--;
chainIndex--;
if (chain_index < this->m_behaviors.size()) {
this->m_behaviors.at(chain_index)->Handle(context, bitStream, branch);
if (chainIndex < this->m_behaviors.size()) {
this->m_behaviors.at(chainIndex)->Handle(context, bitStream, branch);
} else {
Game::logger->Log("ChainBehavior", "chainIndex out of bounds, aborting handle of chain %i bits unread %i", chainIndex, bitStream->GetNumberOfUnreadBits());
}
}

View File

@ -1,12 +1,16 @@
#include "ChargeUpBehavior.h"
#include "BehaviorBranchContext.h"
#include "BehaviorContext.h"
#include "Game.h"
#include "dLogger.h"
void ChargeUpBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStream, const BehaviorBranchContext branch) {
uint32_t handle;
uint32_t handle{};
bitStream->Read(handle);
if (!bitStream->Read(handle)) {
Game::logger->Log("ChargeUpBehavior", "Unable to read handle from bitStream, aborting Handle! variable_type");
return;
};
context->RegisterSyncBehavior(handle, this, branch);
}

View File

@ -3,23 +3,34 @@
#include "BehaviorContext.h"
#include "ControllablePhysicsComponent.h"
#include "EntityManager.h"
#include "Game.h"
#include "dLogger.h"
void ForceMovementBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStream, const BehaviorBranchContext branch) {
if (this->m_hitAction->m_templateId == BehaviorTemplates::BEHAVIOR_EMPTY && this->m_hitEnemyAction->m_templateId == BehaviorTemplates::BEHAVIOR_EMPTY && this->m_hitFactionAction->m_templateId == BehaviorTemplates::BEHAVIOR_EMPTY) {
return;
}
uint32_t handle;
bitStream->Read(handle);
uint32_t handle{};
if (!bitStream->Read(handle)) {
Game::logger->Log("ForceMovementBehavior", "Unable to read handle from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
}
context->RegisterSyncBehavior(handle, this, branch);
}
void ForceMovementBehavior::Sync(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) {
uint32_t next;
bitStream->Read(next);
uint32_t next{};
if (!bitStream->Read(next)) {
Game::logger->Log("ForceMovementBehavior", "Unable to read target from bitStream, aborting Sync! %i", bitStream->GetNumberOfUnreadBits());
return;
}
LWOOBJID target;
bitStream->Read(target);
LWOOBJID target{};
if (!bitStream->Read(target)) {
Game::logger->Log("ForceMovementBehavior", "Unable to read target from bitStream, aborting Sync! %i", bitStream->GetNumberOfUnreadBits());
return;
}
branch.target = target;
auto* behavior = CreateBehavior(next);

View File

@ -11,7 +11,10 @@ void InterruptBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitS
if (branch.target != context->originator) {
bool unknown = false;
bitStream->Read(unknown);
if (!bitStream->Read(unknown)) {
Game::logger->Log("InterruptBehavior", "Unable to read unknown1 from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
};
if (unknown) return;
}
@ -19,7 +22,10 @@ void InterruptBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitS
if (!this->m_interruptBlock) {
bool unknown = false;
bitStream->Read(unknown);
if (!bitStream->Read(unknown)) {
Game::logger->Log("InterruptBehavior", "Unable to read unknown2 from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
};
if (unknown) return;
}
@ -28,7 +34,10 @@ void InterruptBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitS
{
bool unknown = false;
bitStream->Read(unknown);
if (!bitStream->Read(unknown)) {
Game::logger->Log("InterruptBehavior", "Unable to read unknown3 from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
};
}
if (branch.target == context->originator) return;

View File

@ -6,11 +6,16 @@
#include "EntityManager.h"
#include "GameMessages.h"
#include "DestroyableComponent.h"
#include "Game.h"
#include "dLogger.h"
void KnockbackBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) {
bool unknown;
bool unknown{};
bitStream->Read(unknown);
if (!bitStream->Read(unknown)) {
Game::logger->Log("KnockbackBehavior", "Unable to read unknown from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
};
}
void KnockbackBehavior::Calculate(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) {

View File

@ -13,9 +13,11 @@ void MovementSwitchBehavior::Handle(BehaviorContext* context, RakNet::BitStream*
return;
}
uint32_t movementType;
bitStream->Read(movementType);
uint32_t movementType{};
if (!bitStream->Read(movementType)) {
Game::logger->Log("MovementSwitchBehavior", "Unable to read movementType from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
};
switch (movementType) {
case 1:
@ -55,4 +57,3 @@ void MovementSwitchBehavior::Load() {
this->m_jumpAction = GetAction("jump_action");
}

View File

@ -8,9 +8,12 @@
#include "../dWorldServer/ObjectIDManager.h"
void ProjectileAttackBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) {
LWOOBJID target;
LWOOBJID target{};
bitStream->Read(target);
if (!bitStream->Read(target)) {
Game::logger->Log("ProjectileAttackBehavior", "Unable to read target from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
};
auto* entity = EntityManager::Instance()->GetEntity(context->originator);
@ -30,15 +33,21 @@ void ProjectileAttackBehavior::Handle(BehaviorContext* context, RakNet::BitStrea
if (m_useMouseposit) {
NiPoint3 targetPosition = NiPoint3::ZERO;
bitStream->Read(targetPosition);
if (!bitStream->Read(targetPosition)) {
Game::logger->Log("ProjectileAttackBehavior", "Unable to read targetPosition from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
};
}
auto* targetEntity = EntityManager::Instance()->GetEntity(target);
for (auto i = 0u; i < this->m_projectileCount; ++i) {
LWOOBJID projectileId;
LWOOBJID projectileId{};
bitStream->Read(projectileId);
if (!bitStream->Read(projectileId)) {
Game::logger->Log("ProjectileAttackBehavior", "Unable to read projectileId from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
};
branch.target = target;
branch.isProjectile = true;
@ -98,7 +107,7 @@ void ProjectileAttackBehavior::Calculate(BehaviorContext* context, RakNet::BitSt
for (auto i = 0u; i < this->m_projectileCount; ++i) {
auto id = static_cast<LWOOBJID>(ObjectIDManager::Instance()->GenerateObjectID());
id = GeneralUtils::SetBit(id, OBJECT_BIT_CLIENT);
id = GeneralUtils::SetBit(id, OBJECT_BIT_SPAWNED);
bitStream->Write(id);

View File

@ -14,8 +14,11 @@ void StunBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStream
return;
}
bool blocked;
bitStream->Read(blocked);
bool blocked{};
if (!bitStream->Read(blocked)) {
Game::logger->Log("StunBehavior", "Unable to read blocked from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
};
auto* target = EntityManager::Instance()->GetEntity(branch.target);

View File

@ -10,7 +10,10 @@ void SwitchBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStre
auto state = true;
if (this->m_imagination > 0 || !this->m_isEnemyFaction) {
bitStream->Read(state);
if (!bitStream->Read(state)) {
Game::logger->Log("SwitchBehavior", "Unable to read state from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
return;
};
}
auto* entity = EntityManager::Instance()->GetEntity(context->originator);

View File

@ -9,10 +9,12 @@
#include "EntityManager.h"
void SwitchMultipleBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bit_stream, BehaviorBranchContext branch) {
float value;
void SwitchMultipleBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) {
float value{};
bit_stream->Read(value);
if (!bitStream->Read(value)) {
Game::logger->Log("SwitchMultipleBehavior", "Unable to read value from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
};
uint32_t trigger = 0;
@ -30,10 +32,10 @@ void SwitchMultipleBehavior::Handle(BehaviorContext* context, RakNet::BitStream*
auto* behavior = this->m_behaviors.at(trigger).second;
behavior->Handle(context, bit_stream, branch);
behavior->Handle(context, bitStream, branch);
}
void SwitchMultipleBehavior::Calculate(BehaviorContext* context, RakNet::BitStream* bit_stream, BehaviorBranchContext branch) {
void SwitchMultipleBehavior::Calculate(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) {
// TODO
}

View File

@ -20,12 +20,16 @@ void TacArcBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStre
bool hit = false;
bitStream->Read(hit);
if (!bitStream->Read(hit)) {
Game::logger->Log("TacArcBehavior", "Unable to read hit from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
};
if (this->m_checkEnv) {
bool blocked = false;
bitStream->Read(blocked);
if (!bitStream->Read(blocked)) {
Game::logger->Log("TacArcBehavior", "Unable to read blocked from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
};
if (blocked) {
this->m_blockedAction->Handle(context, bitStream, branch);
@ -37,7 +41,9 @@ void TacArcBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStre
if (hit) {
uint32_t count = 0;
bitStream->Read(count);
if (!bitStream->Read(count)) {
Game::logger->Log("TacArcBehavior", "Unable to read count from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
};
if (count > m_maxTargets && m_maxTargets > 0) {
count = m_maxTargets;
@ -46,9 +52,11 @@ void TacArcBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStre
std::vector<LWOOBJID> targets;
for (auto i = 0u; i < count; ++i) {
LWOOBJID id;
LWOOBJID id{};
bitStream->Read(id);
if (!bitStream->Read(id)) {
Game::logger->Log("TacArcBehavior", "Unable to read id from bitStream, aborting Handle! %i", bitStream->GetNumberOfUnreadBits());
};
targets.push_back(id);
}