Skip to content

Fix critical and high severity security vulnerabilities #316

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

Closed
kwsantiago wants to merge 3 commits into from
Closed

Fix critical and high severity security vulnerabilities #316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
35b65ed
Add null pointer safety and replace assertions with runtime checks
kwsantiago Feb 12, 2026
1f8b63d
Add input validation, overflow protection, and buffer safety
kwsantiago Feb 12, 2026
e9683fd
Add network security: rate limiting, connection limits, monitor auth
kwsantiago Feb 12, 2026
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
8 changes: 6 additions & 2 deletions src/Abilities.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ static void attachSkillResults(std::vector<SkillResult> results, uint8_t* pivot)
void Abilities::useNanoSkill(CNSocket* sock, SkillData* skill, sNano& nano, std::vector<ICombatant*> affected) {

Player* plr = PlayerManager::getPlayer(sock);
if (plr == nullptr) return;
ICombatant* combatant = dynamic_cast<ICombatant*>(plr);

int boost = 0;
Expand Down Expand Up @@ -401,8 +402,11 @@ void Abilities::useNPCSkill(EntityRef npc, int skillID, std::vector<ICombatant*>
static std::vector<ICombatant*> entityRefsToCombatants(std::vector<EntityRef> refs) {
std::vector<ICombatant*> combatants;
for(EntityRef ref : refs) {
if(ref.kind == EntityKind::PLAYER)
combatants.push_back(dynamic_cast<ICombatant*>(PlayerManager::getPlayer(ref.sock)));
if(ref.kind == EntityKind::PLAYER) {
Player* p = PlayerManager::getPlayer(ref.sock);
if (p != nullptr)
combatants.push_back(dynamic_cast<ICombatant*>(p));
}
else if(ref.kind == EntityKind::COMBAT_NPC || ref.kind == EntityKind::MOB)
combatants.push_back(dynamic_cast<ICombatant*>(ref.getEntity()));
}
Expand Down
13 changes: 13 additions & 0 deletions src/Buddies.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ static bool playerHasBuddyWithID(Player* plr, int buddyID) {
// Refresh buddy list
void Buddies::sendBuddyList(CNSocket* sock) {
Player* plr = PlayerManager::getPlayer(sock);
if (plr == nullptr) return;
int buddyCnt = Database::getNumBuddies(plr);

if (!validOutVarPacket(sizeof(sP_FE2CL_REP_PC_BUDDYLIST_INFO_SUCC), buddyCnt, sizeof(sBuddyBaseInfo))) {
Expand Down Expand Up @@ -85,6 +86,7 @@ static void requestBuddy(CNSocket* sock, CNPacketData* data) {
auto req = (sP_CL2FE_REQ_REQUEST_MAKE_BUDDY*)data->buf;

Player* plr = PlayerManager::getPlayer(sock);
if (plr == nullptr) return;
Player* otherPlr = PlayerManager::getPlayerFromID(req->iBuddyID);

if (otherPlr == nullptr)
Expand Down Expand Up @@ -123,13 +125,15 @@ static void reqBuddyByName(CNSocket* sock, CNPacketData* data) {
auto pkt = (sP_CL2FE_REQ_PC_FIND_NAME_MAKE_BUDDY*)data->buf;
Player* plrReq = PlayerManager::getPlayer(sock);

if (plrReq == nullptr) return;
INITSTRUCT(sP_FE2CL_REP_PC_FIND_NAME_MAKE_BUDDY_SUCC, resp);

CNSocket* otherSock = PlayerManager::getSockFromName(AUTOU16TOU8(pkt->szFirstName), AUTOU16TOU8(pkt->szLastName));
if (otherSock == nullptr)
return; // no player found

Player *otherPlr = PlayerManager::getPlayer(otherSock);
if (otherPlr == nullptr) return;
if (playerHasBuddyWithID(plrReq, otherPlr->iID))
return;

Expand All @@ -145,6 +149,7 @@ static void reqBuddyByName(CNSocket* sock, CNPacketData* data) {
static void reqAcceptBuddy(CNSocket* sock, CNPacketData* data) {
auto req = (sP_CL2FE_REQ_ACCEPT_MAKE_BUDDY*)data->buf;
Player* plr = PlayerManager::getPlayer(sock);
if (plr == nullptr) return;
Player* otherPlr = PlayerManager::getPlayerFromID(req->iBuddyID);

if (otherPlr == nullptr)
Expand Down Expand Up @@ -213,6 +218,7 @@ static void reqFindNameBuddyAccept(CNSocket* sock, CNPacketData* data) {

Player* plrReq = PlayerManager::getPlayer(sock);

if (plrReq == nullptr) return;
INITSTRUCT(sP_FE2CL_REP_ACCEPT_MAKE_BUDDY_SUCC, resp);

Player* otherPlr = PlayerManager::getPlayerFromID(pkt->iBuddyPCUID);
Expand Down Expand Up @@ -278,6 +284,7 @@ static void reqFindNameBuddyAccept(CNSocket* sock, CNPacketData* data) {
static void reqPktGetBuddyState(CNSocket* sock, CNPacketData* data) {
Player* plr = PlayerManager::getPlayer(sock);

if (plr == nullptr) return;
INITSTRUCT(sP_FE2CL_REP_GET_BUDDY_STATE_SUCC, resp);

for (int slot = 0; slot < 50; slot++) {
Expand All @@ -293,6 +300,7 @@ static void reqBuddyBlock(CNSocket* sock, CNPacketData* data) {
auto pkt = (sP_CL2FE_REQ_SET_BUDDY_BLOCK*)data->buf;
Player* plr = PlayerManager::getPlayer(sock);

if (plr == nullptr) return;
// sanity checks
if (pkt->iBuddySlot < 0 || pkt->iBuddySlot >= 50 || plr->buddyIDs[pkt->iBuddySlot] != pkt->iBuddyPCUID)
return;
Expand All @@ -317,6 +325,7 @@ static void reqBuddyBlock(CNSocket* sock, CNPacketData* data) {
if (otherSock == nullptr)
return; // other player isn't online, no broadcast needed
Player* otherPlr = PlayerManager::getPlayer(otherSock);
if (otherPlr == nullptr) return;
// search for the slot with the requesting player's ID
otherResp.iBuddyPCUID = plr->PCStyle.iPC_UID;
for (int i = 0; i < 50; i++) {
Expand All @@ -336,6 +345,7 @@ static void reqPlayerBlock(CNSocket* sock, CNPacketData* data) {
auto pkt = (sP_CL2FE_REQ_SET_PC_BLOCK*)data->buf;

Player* plr = PlayerManager::getPlayer(sock);
if (plr == nullptr) return;
int buddySlot = getAvailableBuddySlot(plr);
if (buddySlot == -1)
return;
Expand Down Expand Up @@ -363,6 +373,7 @@ static void reqBuddyDelete(CNSocket* sock, CNPacketData* data) {

Player* plr = PlayerManager::getPlayer(sock);

if (plr == nullptr) return;
// remove buddy on our side
INITSTRUCT(sP_FE2CL_REP_REMOVE_BUDDY_SUCC, resp);
resp.iBuddyPCUID = pkt->iBuddyPCUID;
Expand All @@ -389,6 +400,7 @@ static void reqBuddyDelete(CNSocket* sock, CNPacketData* data) {
if (otherSock == nullptr)
return; // other player isn't online, no broadcast needed
Player* otherPlr = PlayerManager::getPlayer(otherSock);
if (otherPlr == nullptr) return;
// search for the slot with the requesting player's ID
resp.iBuddyPCUID = plr->PCStyle.iPC_UID;
for (int i = 0; i < 50; i++) {
Expand All @@ -407,6 +419,7 @@ static void reqBuddyDelete(CNSocket* sock, CNPacketData* data) {
static void reqBuddyWarp(CNSocket* sock, CNPacketData* data) {
Player *plr = PlayerManager::getPlayer(sock);

if (plr == nullptr) return;
auto pkt = (sP_CL2FE_REQ_PC_BUDDY_WARP*)data->buf;

if (pkt->iSlotNum < 0 || pkt->iSlotNum >= 50)
Expand Down
3 changes: 2 additions & 1 deletion src/Buffs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ void Buffs::tickDrain(EntityRef self, Buff* buff, int mult) {
int dealt = combatant->takeDamage(buff->getLastSource(), damage);

size_t resplen = sizeof(sP_FE2CL_CHAR_TIME_BUFF_TIME_TICK) + sizeof(sSkillResult_Damage);
assert(resplen < CN_PACKET_BODY_SIZE);
if (resplen >= CN_PACKET_BODY_SIZE)
return;
uint8_t respbuf[CN_PACKET_BODY_SIZE];
memset(respbuf, 0, CN_PACKET_BODY_SIZE);

Expand Down
21 changes: 20 additions & 1 deletion src/BuiltinCommands.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ void BuiltinCommands::setSpecialState(CNSocket* sock, CNPacketData* data) {
auto setData = (sP_CL2FE_GM_REQ_PC_SPECIAL_STATE_SWITCH*)data->buf;
Player *plr = PlayerManager::getPlayer(sock);

if (plr == nullptr) return;
// HACK: work around the invisible weapon bug
if (setData->iSpecialStateFlag == CN_SPECIAL_STATE_FLAG__FULL_UI)
Items::updateEquips(sock, plr);
Expand All @@ -31,14 +32,16 @@ void BuiltinCommands::setSpecialState(CNSocket* sock, CNPacketData* data) {
}

static void setGMSpecialSwitchPlayer(CNSocket* sock, CNPacketData* data) {
if (PlayerManager::getPlayer(sock)->accountLevel > 30)
Player* plr = PlayerManager::getPlayer(sock);
if (plr == nullptr || plr->accountLevel > 30)
return;

BuiltinCommands::setSpecialState(sock, data);
}

static void gotoPlayer(CNSocket* sock, CNPacketData* data) {
Player *plr = PlayerManager::getPlayer(sock);
if (plr == nullptr) return;
if (plr->accountLevel > 50)
return;

Expand All @@ -56,6 +59,7 @@ static void gotoPlayer(CNSocket* sock, CNPacketData* data) {

static void setValuePlayer(CNSocket* sock, CNPacketData* data) {
Player *plr = PlayerManager::getPlayer(sock);
if (plr == nullptr) return;
if (plr->accountLevel > 50)
return;

Expand Down Expand Up @@ -126,6 +130,7 @@ static void setValuePlayer(CNSocket* sock, CNPacketData* data) {
static void setGMSpecialOnOff(CNSocket *sock, CNPacketData *data) {
Player *plr = PlayerManager::getPlayer(sock);

if (plr == nullptr) return;
// access check
if (plr->accountLevel > 30)
return;
Expand All @@ -140,6 +145,7 @@ static void setGMSpecialOnOff(CNSocket *sock, CNPacketData *data) {
}

Player *otherPlr = PlayerManager::getPlayer(otherSock);
if (otherPlr == nullptr) return;
if (req->iONOFF)
otherPlr->iSpecialState |= req->iSpecialStateFlag;
else
Expand All @@ -151,6 +157,7 @@ static void setGMSpecialOnOff(CNSocket *sock, CNPacketData *data) {
static void locatePlayer(CNSocket *sock, CNPacketData *data) {
Player *plr = PlayerManager::getPlayer(sock);

if (plr == nullptr) return;
// access check
if (plr->accountLevel > 30)
return;
Expand All @@ -167,6 +174,7 @@ static void locatePlayer(CNSocket *sock, CNPacketData *data) {
INITSTRUCT(sP_FE2CL_GM_REP_PC_LOCATION, resp);
Player *otherPlr = PlayerManager::getPlayer(otherSock);

if (otherPlr == nullptr) return;
resp.iTargetPC_UID = otherPlr->accountId;
resp.iTargetPC_ID = otherPlr->iID;
resp.iShardID = 0; // sharding is unsupported
Expand All @@ -186,6 +194,7 @@ static void locatePlayer(CNSocket *sock, CNPacketData *data) {
static void kickPlayer(CNSocket *sock, CNPacketData *data) {
Player *plr = PlayerManager::getPlayer(sock);

if (plr == nullptr) return;
// access check
if (plr->accountLevel > 30)
return;
Expand All @@ -201,6 +210,7 @@ static void kickPlayer(CNSocket *sock, CNPacketData *data) {

Player *otherPlr = PlayerManager::getPlayer(otherSock);

if (otherPlr == nullptr) return;
if (plr->accountLevel > otherPlr->accountLevel) {
Chat::sendServerMessage(sock, "player has higher access level");
return;
Expand All @@ -221,6 +231,7 @@ static void kickPlayer(CNSocket *sock, CNPacketData *data) {
static void warpToPlayer(CNSocket *sock, CNPacketData *data) {
Player *plr = PlayerManager::getPlayer(sock);

if (plr == nullptr) return;
// access check
if (plr->accountLevel > 30)
return;
Expand All @@ -240,6 +251,7 @@ static void warpToPlayer(CNSocket *sock, CNPacketData *data) {
static void teleportPlayer(CNSocket *sock, CNPacketData *data) {
Player *plr = PlayerManager::getPlayer(sock);

if (plr == nullptr) return;
// access check
if (plr->accountLevel > 30)
return;
Expand Down Expand Up @@ -279,11 +291,13 @@ static void teleportPlayer(CNSocket *sock, CNPacketData *data) {
return;
}
goalPlr = PlayerManager::getPlayer(goalSock);
if (goalPlr == nullptr) return;

PlayerManager::sendPlayerTo(targetSock, goalPlr->x, goalPlr->y, goalPlr->z, goalPlr->instanceID);
break;
case eCN_GM_TeleportType::Unstick:
targetPlr = PlayerManager::getPlayer(targetSock);
if (targetPlr == nullptr) return;

PlayerManager::sendPlayerTo(targetSock, targetPlr->x - unstickRange/2 + Rand::rand(unstickRange),
targetPlr->y - unstickRange/2 + Rand::rand(unstickRange), targetPlr->z + 80);
Expand All @@ -295,6 +309,7 @@ static void itemGMGiveHandler(CNSocket* sock, CNPacketData* data) {
auto itemreq = (sP_CL2FE_REQ_PC_GIVE_ITEM*)data->buf;
Player* plr = PlayerManager::getPlayer(sock);

if (plr == nullptr) return;
if (plr->accountLevel > 50) {
// TODO: send fail packet
return;
Expand All @@ -311,6 +326,9 @@ static void itemGMGiveHandler(CNSocket* sock, CNPacketData* data) {
return;
}

if (itemreq->iSlotNum < 0 || itemreq->iSlotNum >= AINVEN_COUNT)
return;

if (itemreq->Item.iType == 10) {
// item is vehicle, set expiration date
// set time limit: current time + 7days
Expand Down Expand Up @@ -352,6 +370,7 @@ static void nanoGMGiveHandler(CNSocket* sock, CNPacketData* data) {
auto nano = (sP_CL2FE_REQ_PC_GIVE_NANO*)data->buf;
Player *plr = PlayerManager::getPlayer(sock);

if (plr == nullptr) return;
if (plr->accountLevel > 50)
return;

Expand Down
Loading