Do not dynamically allocate memory in CNSocket::sendPacket()

Also reorder the rapid fire check in MobManager::pcAttackNpcs(), so the
output packet validation happens immediately before the buffer is
initialized, for clarity.
This commit is contained in:
dongresource 2021-03-04 19:48:02 +01:00
parent 29dbe83a0b
commit da8dde9818
2 changed files with 28 additions and 31 deletions

View File

@ -118,19 +118,20 @@ void CNSocket::kill() {
#endif
}
// we don't own buf, TODO: queue packets up to send in step()
void CNSocket::sendPacket(void* buf, uint32_t type, size_t size) {
if (!alive)
return;
size_t bodysize = size + sizeof(uint32_t);
uint8_t* fullpkt = (uint8_t*)xmalloc(bodysize+4);
uint8_t* body = fullpkt+4;
uint8_t fullpkt[CN_PACKET_BUFFER_SIZE]; // length, type, body
uint8_t* body = fullpkt + 4; // packet without length (type, body)
size_t bodysize = size + 4;
// set packet length
memcpy(fullpkt, (void*)&bodysize, 4);
// copy packet type to the front of the buffer & then the actual buffer
memcpy(body, (void*)&type, sizeof(uint32_t));
memcpy(body+sizeof(uint32_t), buf, size);
memcpy(body, (void*)&type, 4);
memcpy(body+4, buf, size);
// encrypt the packet
switch (activeKey) {
@ -140,20 +141,16 @@ void CNSocket::sendPacket(void* buf, uint32_t type, size_t size) {
case SOCKETKEY_FE:
CNSocketEncryption::encryptData((uint8_t*)body, (uint8_t*)(&FEKey), bodysize);
break;
default: {
free(fullpkt);
default:
DEBUGLOG(
std::cout << "[WARN]: UNSET KEYTYPE FOR SOCKET!! ABORTING SEND" << std::endl;
)
return;
}
}
// send packet data!
if (alive && !sendData(fullpkt, bodysize+4))
kill();
free(fullpkt);
}
void CNSocket::setActiveKey(ACTIVEKEY key) {

View File

@ -52,16 +52,6 @@ void MobManager::pcAttackNpcs(CNSocket *sock, CNPacketData *data) {
int32_t *pktdata = (int32_t*)((uint8_t*)data->buf + sizeof(sP_CL2FE_REQ_PC_ATTACK_NPCs));
/*
* Due to the possibility of multiplication overflow (and regular buffer overflow),
* both incoming and outgoing variable-length packets must be validated, at least if
* the number of trailing structs isn't well known (ie. it's from the client).
*/
if (!validOutVarPacket(sizeof(sP_FE2CL_PC_ATTACK_NPCs_SUCC), pkt->iNPCCnt, sizeof(sAttackResult))) {
std::cout << "[WARN] bad sP_FE2CL_PC_ATTACK_NPCs_SUCC packet size\n";
return;
}
// rapid fire anti-cheat
time_t currTime = getTime();
if (currTime - plr->lastShot < plr->fireRate * 80)
@ -77,6 +67,16 @@ void MobManager::pcAttackNpcs(CNSocket *sock, CNPacketData *data) {
if (plr->suspicionRating > 10000) // kill the socket when the player is too suspicious
sock->kill();
/*
* Due to the possibility of multiplication overflow (and regular buffer overflow),
* both incoming and outgoing variable-length packets must be validated, at least if
* the number of trailing structs isn't well known (ie. it's from the client).
*/
if (!validOutVarPacket(sizeof(sP_FE2CL_PC_ATTACK_NPCs_SUCC), pkt->iNPCCnt, sizeof(sAttackResult))) {
std::cout << "[WARN] bad sP_FE2CL_PC_ATTACK_NPCs_SUCC packet size\n";
return;
}
// initialize response struct
size_t resplen = sizeof(sP_FE2CL_PC_ATTACK_NPCs_SUCC) + pkt->iNPCCnt * sizeof(sAttackResult);
uint8_t respbuf[CN_PACKET_BUFFER_SIZE];