From da8dde981847facc28777a5fdf39b918d5e31a74 Mon Sep 17 00:00:00 2001
From: dongresource <dongresource@protonmail.com>
Date: Thu, 4 Mar 2021 19:48:02 +0100
Subject: [PATCH] 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.
---
 src/CNProtocol.cpp | 39 ++++++++++++++++++---------------------
 src/MobManager.cpp | 20 ++++++++++----------
 2 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/src/CNProtocol.cpp b/src/CNProtocol.cpp
index ab887e0..53c2ee5 100644
--- a/src/CNProtocol.cpp
+++ b/src/CNProtocol.cpp
@@ -118,42 +118,39 @@ 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) {
-        case SOCKETKEY_E:
-            CNSocketEncryption::encryptData((uint8_t*)body, (uint8_t*)(&EKey), bodysize);
-            break;
-        case SOCKETKEY_FE:
-            CNSocketEncryption::encryptData((uint8_t*)body, (uint8_t*)(&FEKey), bodysize);
-            break;
-        default: {
-            free(fullpkt);
-            DEBUGLOG(
-                std::cout << "[WARN]: UNSET KEYTYPE FOR SOCKET!! ABORTING SEND" << std::endl;
-            )
-            return;
-        }
+    case SOCKETKEY_E:
+        CNSocketEncryption::encryptData((uint8_t*)body, (uint8_t*)(&EKey), bodysize);
+        break;
+    case SOCKETKEY_FE:
+        CNSocketEncryption::encryptData((uint8_t*)body, (uint8_t*)(&FEKey), bodysize);
+        break;
+    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) {
diff --git a/src/MobManager.cpp b/src/MobManager.cpp
index 7d4a2eb..4e151a9 100644
--- a/src/MobManager.cpp
+++ b/src/MobManager.cpp
@@ -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];