Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added: out of bounds checks for RakNet arrays #905

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Added: out of bounds checks for RakNet arrays #905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e778d3a
Added: array out of bounds checks
uifdev Mar 29, 2024
1e8dec3
Added: array out of bounds checks
uifdev Mar 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions Server/Components/LegacyNetwork/legacy_network_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,9 @@ enum LegacyClientVersion
IPlayer* RakNetLegacyNetwork::OnPeerConnect(RakNet::RPCParameters* rpcParams, bool isNPC, StringView serial, uint32_t version, StringView versionName, uint32_t challenge, StringView name, bool isUsingOfficialClient)
{
const RakNet::PlayerID rid = rpcParams->sender;
const RakNet::PlayerIndex senderId = rpcParams->senderIndex;

if (playerFromRakIndex[rpcParams->senderIndex])
if (senderId < 0 || senderId >= PLAYER_POOL_SIZE || playerFromRakIndex[senderId])
{
// Connection already exists
return nullptr;
Expand Down Expand Up @@ -369,7 +370,7 @@ IPlayer* RakNetLegacyNetwork::OnPeerConnect(RakNet::RPCParameters* rpcParams, bo
return nullptr;
}

playerFromRakIndex[rpcParams->senderIndex] = newConnectionResult.second;
playerFromRakIndex[senderId] = newConnectionResult.second;

return newConnectionResult.second;
}
Expand Down Expand Up @@ -451,7 +452,9 @@ void RakNetLegacyNetwork::OnPlayerConnect(RakNet::RPCParameters* rpcParams, void

network->networkEventDispatcher.dispatch(&NetworkEventHandler::onPeerConnect, *newPeer);

network->playerRemoteSystem[newPeer->getID()] = remoteSystem;
auto poolID = newPeer->getID();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't make sense to me - if a new peer was just created successfully (non null) then their ID should be valid

if(poolID >= 0 && poolID < PLAYER_POOL_SIZE) network->playerRemoteSystem[poolID] = remoteSystem;

return;
}
}
Expand Down Expand Up @@ -517,15 +520,19 @@ void RakNetLegacyNetwork::OnNPCConnect(RakNet::RPCParameters* rpcParams, void* e

void RakNetLegacyNetwork::OnRakNetDisconnect(RakNet::PlayerIndex rid, PeerDisconnectReason reason)
{
if(rid < 0 || rid >= PLAYER_POOL_SIZE) return;

IPlayer* player = playerFromRakIndex[rid];
playerFromRakIndex[rid] = nullptr;

if (!player)
{
return;
}

playerFromRakIndex[rid] = nullptr;
playerRemoteSystem[player->getID()] = nullptr;
auto poolID = player->getID();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue here

if(poolID >= 0 && poolID < PLAYER_POOL_SIZE) playerRemoteSystem[poolID] = nullptr;

networkEventDispatcher.dispatch(&NetworkEventHandler::onPeerDisconnect, *player, reason);
}

Expand All @@ -535,7 +542,7 @@ void RakNetLegacyNetwork::RPCHook(RakNet::RPCParameters* rpcParams, void* extra)
RakNetLegacyNetwork* network = reinterpret_cast<RakNetLegacyNetwork*>(extra);
const RakNet::PlayerIndex senderId = rpcParams->senderIndex;

if (senderId >= network->playerFromRakIndex.size())
if (senderId < 0 || senderId >= PLAYER_POOL_SIZE)
{
return;
}
Expand Down Expand Up @@ -887,7 +894,7 @@ void RakNetLegacyNetwork::onTick(Microseconds elapsed, TimePoint now)
{
for (RakNet::Packet* pkt = rakNetServer.Receive(); pkt; pkt = rakNetServer.Receive())
{
if (pkt->playerIndex >= playerFromRakIndex.size())
if (pkt->playerIndex < 0 || pkt->playerIndex >= PLAYER_POOL_SIZE)
{
rakNetServer.DeallocatePacket(pkt);
continue;
Expand Down
5 changes: 4 additions & 1 deletion Server/Components/LegacyNetwork/legacy_network_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,10 @@ class RakNetLegacyNetwork final : public Network, public CoreEventHandler, publi

unsigned getPing(const IPlayer& peer) override
{
auto remoteSystem = playerRemoteSystem[peer.getID()];
auto poolID = peer.getID();
if(poolID < 0 || poolID >= PLAYER_POOL_SIZE) return -1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here


auto remoteSystem = playerRemoteSystem[poolID];
if (remoteSystem == nullptr)
{
return -1;
Expand Down